Set DTrace preprocessor via the DTRACE_CPP variable by petk · Pull Request #11853 · php/php-src

petk · 2023-08-01T22:54:22Z

The way DTrace generates files depend on the given preprocessor. Without passing the CPP environment variable dtrace Python script uses the cpp by default.

For systems where the CPP is set to something else, the generated files won't be created the same way, so instead the DTrace preprocessor can be overriden manually with the new DTRACE_CPP variable. For example:

./configure DTRACE_CPP="gcc -E -xc" --enable-dtrace

Fixes GH-11847

The way DTrace generates files depend on the given preprocessor. Without passing the CPP environment variable dtrace Python script uses the cpp by default. For systems where the CPP is set to something else, the generated files won't be created the same way, so instead the DTrace preprocessor can be overriden manually with the new DTRACE_CPP variable. For example: ./configure DTRACE_CPP="gcc -E -xc" --enable-dtrace Fixes phpGH-11847

remicollet · 2023-08-02T05:21:23Z

For consistency, shouldn't we also have a DTRACE_CC ?

I will try to test it later (not much available, you know... Holidays..)

f4z4on · 2023-08-02T11:00:03Z

Wow 🤯 That is definitely the issue. Spaces are alright… I’ll describe my thinking here (you PHP implementation folks all probably know this already 😅).

I was not aware of the difference between cpp and gcc -E. It’s probably the .d file extension which confuses gcc executable while cpp probably defaults to C. Proper D language does not have a preprocessor. However, DTrace uses #pragma directives (that is C preprocessing macro) for statically-defined traces (like PHP does) and scripts. Therefore -xc is correct, I guess. SystemTap’s dtrace defaults to cpp, so that issue wasn't visible until I started meddling with PHP cross compilation with DTrace enabled which requires different compiler executables altogether.

Now to possible solution…

I don’t think DTrace needs its own preprocessor. If so, people should be able to override it when invoking both ./configure and make. Much like CC, CFLAGS and so on. Good build systems following ./configure && make && make check paradigm do that. All in all, I still think there should not be a separate macro for users to set.

There may be a macro derived from CPP macro, similar to what CFLAGS and CFLAGS_CLEAN are doing. If someone really really wanted to override that, they still could, but I’d say that is a very obscure use case.

Or, and I prefer this one, we may simply force -xc after CPP when invoking dtrace -h -C on non-C files (that is files with .d extension). There is only one such invocation (two if you count oci8 extension), and it is that make rule I cherry-picked in my previous comment. The other invocations do not use preprocessing. So, perhaps even ditching CPP from their invocations to be more deliberate about it. Similarly ditching CC and CFLAGS from other dtrace -h -C invocations. This is SystemTap-specific behavior. The -C option is as well, and that is the option enabling C preprocessor use. SystemTap is Linux-only. Others ignore all of this… It just works for everyone… Anyone stumbling upon this code in years to come would see there is some complicated story right away because dtrace invocations differ. There is no silver bullet that would not introduce more complexity (i.e., additional m4 macros, macro dependencies, …) and I believe that’s something everyone should strive for to avoid unless necessary.

diff --git a/build/php.m4 b/build/php.m4
index 921a78eb78..73dd1e8e27 100644
--- a/build/php.m4
+++ b/build/php.m4
@@ -2389,7 +2389,7 @@ dnl overwritten (Bug 61268).
 $abs_srcdir/$ac_provsrc:;
 
 $ac_bdir[$]ac_hdrobj: $abs_srcdir/$ac_provsrc
-	CPP="\$(CPP)" CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -h -C -s $ac_srcdir[$]ac_provsrc -o \$[]@.bak && \$(SED) -e 's,PHP_,DTRACE_,g' \$[]@.bak > \$[]@
+	CPP="\$(CPP) -xc" dtrace -h -C -s $ac_srcdir[$]ac_provsrc -o \$[]@.bak && \$(SED) -e 's,PHP_,DTRACE_,g' \$[]@.bak > \$[]@
 
 \$(PHP_DTRACE_OBJS): $ac_bdir[$]ac_hdrobj
 
@@ -2409,12 +2409,12 @@ EOF
 $ac_bdir[$]ac_provsrc.lo: \$(PHP_DTRACE_OBJS)
 	echo "[#] Generated by Makefile for libtool" > \$[]@
 	@test -d "$dtrace_lib_dir" || mkdir $dtrace_lib_dir
-	if CPP="\$(CPP)" CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o $dtrace_d_obj -s $abs_srcdir/$ac_provsrc $dtrace_lib_objs 2> /dev/null && test -f "$dtrace_d_obj"; then [\\]
+	if CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o $dtrace_d_obj -s $abs_srcdir/$ac_provsrc $dtrace_lib_objs 2> /dev/null && test -f "$dtrace_d_obj"; then [\\]
 	  echo "pic_object=['].libs/$dtrace_prov_name[']" >> \$[]@ [;\\]
 	else [\\]
 	  echo "pic_object='none'" >> \$[]@ [;\\]
 	fi
-	if CPP="\$(CPP)" CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o $ac_bdir[$]ac_provsrc.o -s $abs_srcdir/$ac_provsrc $dtrace_nolib_objs 2> /dev/null && test -f "$ac_bdir[$]ac_provsrc.o"; then [\\]
+	if CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o $ac_bdir[$]ac_provsrc.o -s $abs_srcdir/$ac_provsrc $dtrace_nolib_objs 2> /dev/null && test -f "$ac_bdir[$]ac_provsrc.o"; then [\\]
 	  echo "non_pic_object=[']$dtrace_prov_name[']" >> \$[]@ [;\\]
 	else [\\]
 	  echo "non_pic_object='none'" >> \$[]@ [;\\]
@@ -2426,7 +2426,7 @@ EOF
   *)
 cat>>Makefile.objects<<EOF
 $ac_bdir[$]ac_provsrc.o: \$(PHP_DTRACE_OBJS)
-	CPP="\$(CPP)" CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o \$[]@ -s $abs_srcdir/$ac_provsrc $dtrace_objs
+	CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o \$[]@ -s $abs_srcdir/$ac_provsrc $dtrace_objs
 
 EOF
     ;;
diff --git a/ext/oci8/config.m4 b/ext/oci8/config.m4
index 883e272fb1..6fcc6d9eab 100644
--- a/ext/oci8/config.m4
+++ b/ext/oci8/config.m4
@@ -124,7 +124,7 @@ dnl overwritten (Bug 61268).
 PHP_EXT_SRCDIR([oci8])/$ac_provsrc:;
 
 $ac_bdir[$]ac_hdrobj: $ac_srcdir[$]ac_provsrc
-	CPP="\$(CPP)" CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -h -C -s $ac_srcdir[$]ac_provsrc -o \$[]@.bak && \$(SED) -e 's,PHPOCI_,DTRACE_,g' \$[]@.bak > \$[]@
+	CPP="\$(CPP) -xc" dtrace -h -C -s $ac_srcdir[$]ac_provsrc -o \$[]@.bak && \$(SED) -e 's,PHPOCI_,DTRACE_,g' \$[]@.bak > \$[]@
 
 \$(OCI8_DTRACE_OBJS): $ac_bdir[$]ac_hdrobj
 
@@ -145,12 +145,12 @@ EOF
 $ac_bdir[$]ac_provsrc.lo: \$(OCI8_DTRACE_OBJS)
 	echo "[#] Generated by Makefile for libtool" > \$[]@
 	@test -d "$dtrace_lib_dir" || mkdir $dtrace_lib_dir
-	if CPP="\$(CPP)" CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o $dtrace_d_obj -s $ac_srcdir[$]ac_provsrc $dtrace_oci8_lib_objs 2> /dev/null && test -f "$dtrace_d_obj"; then [\\]
+	if CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o $dtrace_d_obj -s $ac_srcdir[$]ac_provsrc $dtrace_oci8_lib_objs 2> /dev/null && test -f "$dtrace_d_obj"; then [\\]
 	  echo "pic_object=['].libs/$dtrace_prov_name[']" >> \$[]@ [;\\]
 	else [\\]
 	  echo "pic_object='none'" >> \$[]@ [;\\]
 	fi
-	if CPP="\$(CPP)" CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o $ac_bdir[$]ac_provsrc.o -s $ac_srcdir[$]ac_provsrc $dtrace_nolib_objs 2> /dev/null && test -f "$ac_bdir[$]ac_provsrc.o"; then [\\]
+	if CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o $ac_bdir[$]ac_provsrc.o -s $ac_srcdir[$]ac_provsrc $dtrace_nolib_objs 2> /dev/null && test -f "$ac_bdir[$]ac_provsrc.o"; then [\\]
 	  echo "non_pic_object=[']$dtrace_prov_name[']" >> \$[]@ [;\\]
 	else [\\]
 	  echo "non_pic_object='none'" >> \$[]@ [;\\]
@@ -162,7 +162,7 @@ EOF
     AC_MSG_WARN([OCI8 extension: OCI8 DTrace support is not confirmed on this platform])
 cat>>Makefile.objects<<EOF
 $ac_bdir[$]ac_provsrc.o: \$(OCI8_DTRACE_OBJS)
-	CPP="\$(CPP)" CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o \$[]@ -s $ac_srcdir[$]ac_provsrc $dtrace_oci8_objs
+	CC="\$(CC)" CFLAGS="\$(CFLAGS_CLEAN)" dtrace -G -o \$[]@ -s $ac_srcdir[$]ac_provsrc $dtrace_oci8_objs
 
 EOF
     ;;

But… I’m talking here as a user of PHP build system who builds PHP regularly, including cross-compiling. There may be other consideration I am not aware of. From the outside, I prefer a simple solution which works and if there is a need for something obscure, people needing it are expected to understand how build systems work and how to modify them — there is no need to cater to them at the expense of introducing bloat. I believe a use case of using completely separate CPP just for dtrace utility from SystemTap on Linux is such an obscure case. These are just C macros, it’s just that we have found some C preprocessor invocations which do not default to C when processing source files with .d extension… Luckily on Linux with SystemTap, there’s a safe and broadly-implemented way of forcing it via that -xc option.

f4z4on · 2023-08-02T11:00:25Z

Another option is to stop setting CPP. As I mentioned above, SystemTap’s dtrace defaults to cpp and that just works… If I'm not mistaken, header files generated by dtrace are architecture-independent. I think I tested this, when I was working on #11643 because I originally wanted to only add CC. Then I thought that CPP is part of the suit along with CC, so if we’re passing along customized CC, we should also respect customized CPP. But as far as I can tell, it’s not necessary. I don't like this option though.

I still prefer appending -xc to CC and setting only the necessary environment variables for SystemTap’s dtrace based on what we want from dtrace in that particular invocation.

f4z4on · 2023-08-02T15:25:56Z

I’ve submitted alternative pull request #11858 based on my comments here… I suggest continuing the discussion back at original issue #11847.

petk · 2023-08-02T16:21:49Z

@f4z4on awesome, thanks. I'll check it out.

petk · 2023-08-02T21:13:58Z

I'm closing this PR to have a solution #11858...

github-actions Bot added Category: Build System Extension: oci8 labels Aug 1, 2023

petk force-pushed the patch-dtrace branch from caba3fd to 76ad89f Compare August 1, 2023 22:55

petk force-pushed the patch-dtrace branch from 76ad89f to 0ae3021 Compare August 1, 2023 23:13

f4z4on mentioned this pull request Aug 2, 2023

Fix DTrace build with SystemTap and GCC defaults #11858

Closed

f4z4on mentioned this pull request Aug 2, 2023

DTrace enabled build is broken #11847

Closed

petk closed this Aug 2, 2023

petk deleted the patch-dtrace branch August 7, 2023 09:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Set DTrace preprocessor via the DTRACE_CPP variable#11853

Set DTrace preprocessor via the DTRACE_CPP variable#11853
petk wants to merge 1 commit into
php:masterfrom
petk:patch-dtrace

petk commented Aug 1, 2023 •

edited

Loading

Uh oh!

remicollet commented Aug 2, 2023

Uh oh!

f4z4on commented Aug 2, 2023 •

edited

Loading

Uh oh!

f4z4on commented Aug 2, 2023 •

edited

Loading

Uh oh!

f4z4on commented Aug 2, 2023 •

edited

Loading

Uh oh!

petk commented Aug 2, 2023

Uh oh!

petk commented Aug 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

petk commented Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

remicollet commented Aug 2, 2023

Uh oh!

f4z4on commented Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

f4z4on commented Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

f4z4on commented Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petk commented Aug 2, 2023

Uh oh!

petk commented Aug 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

petk commented Aug 1, 2023 •

edited

Loading

f4z4on commented Aug 2, 2023 •

edited

Loading

f4z4on commented Aug 2, 2023 •

edited

Loading

f4z4on commented Aug 2, 2023 •

edited

Loading