Fix DTrace build with SystemTap and GCC defaults

[f4z4on]

We are experiencing an issue when building PHP with DTrace enabled with SystemTap (see #11847).† SystemTap, unlike Oracle DTrace, Open DTrace or their forks, relies on both external C compiler and external preprocessor. We have recently aligned both of these with compiler and preprocessor we use to build regular C source code (see #11643).‡ We set CPP environment variable to the value of our CPP macro which defaults to C preprocessor detected by configure script. Similarly, we set CC environment variable to the value of our CC macro which defaults to C compiler detected by configure script. C compiler flags from CFLAGS_CLEAN macro have already been in place since versions 5.4.20 and 5.5.4 from 2013-09-18.

We had modified all dtrace invocations in the same way to make it look consistent. However, SystemTap dtrace needs C preprocessor only when it generates header files (-h option) with preprocessing (-C option). Similarly, it needs C compiler only when it generates object files with probe definitions (-G option). External C preprocessor use via the -C option is actually common to dtrace implementations other than SystemTap even for actions other than generating header files. However, customization of C preprocessor differs widely between SystemTap and the rest of DTrace ecosystem (for example, no other implementation uses environment variables). Therefore, we have recently stopped setting all these environment variables when invoking dtrace. We set only what is necessary for particular action. We hope it makes this fix simpler and clearer without introducing additional complexity to our build system.

The cause of our issue is the fact that GCC preprocessor, when invoked via gcc binary, detects the language for preprocessing from source file extensions. Because DTrace probes and providers for statically-defined tracing are in files with .d extension, GCC detection fails because it probably thinks it is processing proper D programing language source code (somewhat different from what DTrace uses) which has no C-style preprocessing macros. People have noticed, because configure script on systems with GCC sets CPP make macro to gcc -E. Therefore practically everyone, who does not override C preprocessor to cpp, who is building PHP with DTrace enabled on Linux with SystemTap, experiences this issue.

We force C language when passing C preprocessor as set by the user or detected by the configure script. D programs for DTrace commonly use C language preprocessor. This includes statically-defined tracing probes which can contain the same #pragma directives known from C and similar languages.

We use -x option from GCC to force C. Other compilers suites and their preprocessors may recognize this option too (most notably Clang). Many probably do not. However, this issue is specific to SystemTap, which is itself specific to Linux and has to be build using the same compiler as Linux kernel, which only supports building using GCC-compatible compilers. This limitation of using only GCC-compatible compiler suite does not apply to applications with statically-defined tracing. We can definitely imagine someone building PHP with DTrace enabled using SystemTap and wanting to use compiler suite or preprocessor which is different from what they used to build SystemTap itself or their Linux kernel and which does not recognize the -x option. However, we do not think it is reasonable to accommodate for such use case. Properly solving this would require implementing non-trivial m4 macros, which is probably not warranted given this affects 8 lines of make commands… Moreover, such macros probably belong to a project like GNU Autoconf rather than to PHP.

This has no effect on Oracle DTrace, Open DTrace or any of their forks because they do not allow customization of preprocessor nor compiler via environment variables. They simply ignore CPP, CC, and CFLAGS environment variables, so it does not matter what values we set there.

[f4z4on]

This is an alternative to #11853 by @petk showing what I’ve described and explained in my comments there. I suggest continuing the discussion back at original issue #11847.

[petk]

This looks ok, yes. I've checked it (only quickly though) with few preprocessors and it seems to be building fine now.

However, the CPP will be still default cpp used at the other 3 dtrace invocations. This is ok? I assume that there it doesn't matter so much and it is more important to set the CC variable to be synced with the rest of the php-src building.

And the -x option is supported widely across the preprocessors? It seems to be specific to the GNU and other preprocessors just use similar set of options to the GNU set. 🤔

Edit: Ah, yes, there is explanation in the comments regarding the -x option. Ok...

Edit 2: It is possible though to check if preprocessor supports certain set of flags though with AC_PREPROC_IFELSE:
https://github.com/autoconf-archive/autoconf-archive/blob/master/m4/ax_check_preproc_flag.m4
(if it will be needed)

[petk]

Maybe adding this to check for the -xc CPP flag anyway would be also kind of good just to be sure here (otherwise, yes, probably -xc would work on wide variety of preprocessors here):

AC_CACHE_CHECK([whether _AC_LANG preprocessor accepts -xc],php_cv_c_preproc_x_flag_ok,[
save_CPPFLAGS=$CPPFLAGS
CPPFLAGS="$CPPFLAGS -xc"
AC_PREPROC_IFELSE(
  [AC_LANG_PROGRAM()],
  [php_cv_c_preproc_x_flag_ok=yes],
  [php_cv_c_preproc_x_flag_ok=no])
CPPFLAGS=$save_CPPFLAGS
])
AS_IF([test "$php_cv_c_preproc_x_flag_ok" = "yes"],[DTRACE_CPP_FLAGS=" -xc"])

And then in the first changed line in the PR:

 $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)$DTRACE_CPP_FLAGS" dtrace -h -C -s $ac_srcdir[$]ac_provsrc -o \$[]@.bak && \$(SED) -e 's,PHP_,DTRACE_,g' \$[]@.bak > \$[]@
 
 \$(PHP_DTRACE_OBJS): $ac_bdir[$]ac_hdrobj

Patch

diff --git a/build/php.m4 b/build/php.m4
index 921a78eb78..26b564309e 100644
--- a/build/php.m4
+++ b/build/php.m4
@@ -2381,6 +2381,17 @@ dnl DTrace objects.
     ;;
   esac
 
+AC_CACHE_CHECK([whether _AC_LANG preprocessor accepts -xc],php_cv_c_preproc_x_flag_ok,[
+save_CPPFLAGS=$CPPFLAGS
+CPPFLAGS="$CPPFLAGS -xc"
+AC_PREPROC_IFELSE(
+  [AC_LANG_PROGRAM()],
+  [php_cv_c_preproc_x_flag_ok=yes],
+  [php_cv_c_preproc_x_flag_ok=no])
+CPPFLAGS=$save_CPPFLAGS
+])
+AS_IF([test "$php_cv_c_preproc_x_flag_ok" = "yes"],[DTRACE_CPP_FLAGS=" -xc"])
+
 dnl Generate Makefile.objects entries. The empty $ac_provsrc command stops an
 dnl implicit circular dependency in GNU Make which causes the .d file to be
 dnl overwritten (Bug 61268).
@@ -2389,7 +2400,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)$DTRACE_CPP_FLAGS" dtrace -h -C -s $ac_srcdir[$]ac_provsrc -o \$[]@.bak && \$(SED) -e 's,PHP_,DTRACE_,g' \$[]@.bak > \$[]@
 
 \$(PHP_DTRACE_OBJS): $ac_bdir[$]ac_hdrobj

[f4z4on]

However, the CPP will be still default cpp used at the other 3 dtrace invocations. This is ok? I assume that there it doesn't matter so much and it is more important to set the CC variable to be synced with the rest of the php-src building.

Other dtrace invocations do not use preprocessor. SystemTap dtrace uses proprocessor only when generating header files (-h option) and explicitly told to do so (-C option)—both of these conditions have to be met. External preprocessor is not used to when generating object files (-G option). No other DTrace implementations does this. The fact we set CC, CPP, and CFLAGS environment variables is only to cater to SystemTap. The -C options is proprietary to SystemTap implementation.

And the -x option is supported widely across the preprocessors? It seems to be specific to the GNU and other preprocessors just use similar set of options to the GNU set.

No it is not. But it does not matter. Other operating system and DTrace implementation do not care. The fact we set CC, CPP, and CFLAGS environment variables is only to cater to SystemTap. The -C options is proprietary to SystemTap implementation. SystemTap is closely tied to Linux kernel. It has to be compiled with the same compiler suite as Linux kernel. And Linux kernel can be compiled only with GCC-compatible compilers. It is okay to use GCC-style option to force C language because SystemTap dtrace expects C preprocessor.

I can definitely imagine someone building PHP with DTrace enabled using SystemTap and wanting to use compiler suite or preprocessor which is different from what they used to build SystemTap itself or their Linux kernel and which does not recognize -x option. However, I don't think it is reasonable to accommodate for such use case. Especially not by introducing another variable, macro etc. unless properly abstracted. Properly solving this would require implementing non-trivial m4 macros, which I don't believe is not warranted given this affects 2 lines of make commands…

The problem here isn't C preprocessor. It's SystemTap’s dtrace implementation. Clean build system implementation would detect dtrace flavor and prepare proper invocation pretty much like it does for C compilers. In such implementation, there may be some C preprocessor detection or whatever for SystemTap flavor. But it may be more than 10 years from last time I had written Autoconf macros and I don't have time for that. Frankly, I don't believe it's anyone's good use of time anyway. This affects practically no one. Every line of code, every if, loop and variable are costs.

Why no one? This would be for people who compile PHP on Linux with SystemTap with DTrace enabled using different compiler suite than the one used to compile their kernel and SystemTap and that different compiler suite or specifically its preprocessor does not recognize GCC-style -x option. I'm pretty, if such people exist, they have a lot of other problems, and they know how to customize build systems.

From another point, I was apparently the first person in a decade who cared enough about cross-compilation on Linux and DTrace to look into it. And that's orders of magnitude more common use case. I suspect many Linux distribution disable DTrace in PHP on some machine architectures because of lack of cross-compilation and even their maintainers and communities didn't care enough. And the issue is really there since introduction of DTrace to PHP…

In summary, I don't agree with any proposed modifications. I see your point, but I believe they don’t go the necessary length to actually bring the value you are probably looking for. On the contrary, I believe they introduce bloat at best and mislead reader of the code that there's some sophistication around DTrace while there’s none.

Last, oversimplified recap: CPP environment variable is only for SystemTap dtrace. SystemTap means GGC.

[f4z4on]

Given what has been said so far, I actually see only one other possible change I would be willing to put my name under. That is removal of CPP environment variable. Leave only CC and CFLAGS when generating object files with probes (-G option).

By the way, use of these environment variables is undocumented SystemTap behavior anyway. But CC is necessary allow cross-compilation. As far as I know, those #pragma directives which can be in D source code are machine-independent, so custom CPP does not have to be there because the default does the job. At least no one has complained about it since the introduction of DTrace to PHP.

I have chosen to still include CPP because I think it is more consistent and honest. We set CC and CFLAGS for dtrace -G, so let’s set CPP for dtrace -h -C as well. But if setting CPP causes problems and my solution does not people happy, I believe this is the second best approach.

[f4z4on]

What I'm not willing to put my name under is additional code which hides intentions, is half-baked or introduces unnecessary indirection or bloat. If anyone feels neither this PR as-is is good, nor the removal CPP from dtrace invocation is good, feel free to close this issue. No hard feelings, really. But I don’t want be part of that work and my name connected to it…

[Girgias]

Other than @petk I don't think any of us are very familiar with the build system soooooooooo

[petk]

@f4z4on ok, then let's do that what is in the PR. If the need comes to change it further, we can adapt it later on more. No, worries. Because DTrace should kind of target more architectures later on also. Using DTrace on Windows, for example, seems to be possible, so I'm guessing DTrace goal is to be kind of ubiquitous at some point (just as a theoretic example).

Well, build system is just like a configuration. It needs to change and adapt as the code changes so it compiles and runs properly. Yes, exactly. The build system needs to prepare the invocation of DTrace. That's why that additional check was suggested - preparing the DTrace invocation step by step. And user input kind of can't be blindly trusted it is correct. :)

[f4z4on]

I appreciate your understanding! We may never fully agree on this, because we are looking at this from very different perspectives. Text is not good for this.

I'll try one more time though.

I think you're seeing the -xc argument and want to move it elsewhere because that’s how things are done in GNU Build System.

I’m seeing CPP and know it’s there just for SystemTap because no other DTrace implementation uses it and will not use it (they are philosophically very different)… Using SystemTap without GCC makes no sense, so I prefer to place GCC-style option in there to keep things simple because it’s CPP that’s the issue, not -xc. And that CPP will realistically always point to GCC preprocessor.

Because DTrace should kind of target more architectures later on also. Using DTrace on Windows, for example, seems to be possible, so I'm guessing DTrace goal is to be kind of ubiquitous at some point (just as a theoretic example).

That’s actually kind of my point. DTrace on Windows is (already) based on Open DTrace and follows referential behavior (that is as described in Oracle documentation)… If Microsoft introduces support for statically-defined tracing (SDT), I bet it’s going to work the same. Every other DTrace implementation follows what I would call, a referential implementation—Oracle DTrace. DTrace on macOS is still a fork from Oracle I think. I’m not sure what the situation is on BSDs nowadays. One thing is clear though, everyone is shifting to Open DTrace. For the purpose of the issue at hand, there’s no difference between Open DTrace, Oracle DTrace, nor any of their forks. The only outlier here is Linux with SystemTap. If someone on Linux integrated Open DTrace, all would be fine as-is. It is unlikely given the rise of eBPF, I’m just saying… It’s actually more likely Linux is going to drift away even further in case Red Hat finds another shiny new idea philosophically closer to eBPF and abandons SystemTap and its SDT compatibility with DTrace.

As far as I'm concerned, CPP, CC, and CFLAGS environment variables along with -C option when invoking dtrace are proprietary options for SystemTap. Moreover, the environment variables are not even documented, they are not validated in any way in dtrace utility, …

Given this, someone could say: “OK, there are more DTrace implementations much like there are more compiler suites. Let’s create Autoconf macros to properly detect them…” And I would be all for that. However, that should be contributed to Autoconf or SystemTap or some other place, not to PHP… Somewhere in those macros, GNU Build System magicians can think of interoperable way of invoking C preprocessor… I think focusing on -xc is not even it.

@petk, even your suggestion to add new Autoconf macro which could kind of be okay. But it is not a proper way to do things with GNU Build System. That Autoconf macro should be linked to Automake macro, so that there’s a new resulting Make macro. Still not that much of a code, but it’s another variable, which is another cost, another cognitive load, etc. Even if you thought about it as a compromise, I don't think it’s worth it given the oversimplification of CPP is only for SystemTap and SystemTap means GCC.

[f4z4on]

I’m sorry. The -C option is not specific to SystemTap. It's actually in all of them. I scratched my mentions of this in comments above. It does not change anything else though.

If anything, this realization shows me the need for some sort of Autoconf macros for dtrace. I was living under the impression it is not possible to cross-compile PHP with DTrace enabled with anything else than SystemTap. I thought the other implementations do not allow changing target architecture, because they do not allow any compiler customization. But they do. They also allow preprocessor customization. Sadly, it is quite different from SystemTap. This is not our issue here as the scope of the problem is not PHP and should be solved elsewhere. Again, it does not change anything I said here before. It just shows this is much more complex issue…

To recap: My original issue from #11643 was that I could not cross-compile on Linux with SystemTap. We solved it by introducing additional SystemTap-specific environment variables. This broke the defaults on Linux with SystemTap as described in #11847. I propose we fix it just by fixing SystemTap on Linux (which practically also implies GCC) and do nothing else. Because anything else that has been proposed so far does not even scratch the complexity of the issue and IMHO does not move in the right direction.

I hope I may find some time to properly address dtrace invocation this later this year across the relevant projects. No promises though…

[petk]

Allrighty then. Sounds good. I understand. Thanks for the explanations, @f4z4on

The preprocessor is otherwise also checked at the beginning of configure run (using the AC_PROG_CPP), so that calms me a bit more if something exotic will be inserted into CPP variable. :)

I've checked this on few build cases and looks ok to me.

I'm adding also @remicollet to reviewers to add it to the master branch for the next PHP-8.3 release then.

[f4z4on]

Sorry for the close… A have changed commit messages based on the discussion above and some realizations, rebased the commits, but accidentally pushed origin master HEAD first, which closed the PR. Nevertheless, code changes are the same, I’ve modified just the messages and rebased them.

[petk]

@f4z4on what if we also remove the CPP variable entirely from the header invocation? This would work ok with chosen compiler? Because on Debian where cc -E is used, the -xc option doesn't work (it is configured differently via configure) and some other options should be then appended. It would be quite challenging to discover the proper -xc for every case out there otherwise.

[f4z4on]

@petk Strange… Out of curiosity, are you talking about typical Debian installation with build-essential (or its dependencies installed)? We’re building PHP on Debian, so I’m wondering why we haven’t seen this issue.

Nevertheless, it is a real issue and your proposal would work. SystemTap and other DTrace implementations default to cpp. I’ve been actually thinking this is the best low-effort approach to resolve the issue for some time, especially after some research and some time away from the issue. And that was even before you stumbled upon your issue.

I wasn’t in a rush to change anything as I’m quite busy with other stuff and @remicollet seems under a barrage of other issue too. But I’ll change the PR and rebase it shortly…

[bukka]

I have read through all of this and agree that custom $(CPP) should not be used. If it used only by SystemTap and requires cpp, then I don't really see a point to make it customizable. What was the reason to introduce it in the first place? Was it to allow some other custom flags for cpp?

[bukka]

I think if it really needs some customization, then a new variable as proposed by @petk in other PR, should be used instead as this is a special case where extra -x parameter for selecting language needs to be supported.

[f4z4on]

Because it is effectively a different approach I opened a new PR #12083.

[f4z4on]

What was the reason to introduce it in the first place? Was it to allow some other custom flags for cpp?

Consistency and, in a way, premature optimization (a.k.a. the root of all evil 😅). I found out dtrace invocation is the only blocker for cross-compilation of PHP on and for Linux with SystemTap. I found out SystemTap dtrace utility recognizes not just CFLAGS (introduce to PHP build ages ago) but CPP and CC as well. Although I needed only CC for cross-compiling I thought what harm would do using the same CPP Autoconf / configure script / Makefile / user gives us since I’m already meddling with DTrace. Turns out it does some harm… 😞

[petk]

@f4z4on it is Debian Bullseye. I've checked using the Docker image debian:bullseye-slim and installed gcc g++ from default APT packages.

Otherwise, there is this recommendation to do something like this in such cases:

/* zend_drace_d.c */
#include "zend_dtrace.d"

But it also didn't work with the cc -E in my case (probably I've only too quickly checked it and could be done but let's go with the new PR, yes). 🤔