[PATCH] D34158: For Linux/gnu compatibility, preinclude <stdc-predef.h> if the file is available

Wed Aug 9 15:20:53 PDT 2017

jyknight added a comment.

Just to restate: the ideal outcome of this discussion for me would be to resolve things such that _ALL_ libc implementations will feel comfortable using this technique to provide the C11-required predefined macros.

I'd love for linux, freebsd, macos, solaris, etc etc libc to all conform to the C standard in this regards, and do so in a common way, without the need to encode information about each libc version into the compiler. I _really_ don't think that scales well.

So I take your comments from FreeBSD's point of view seriously, and would very much like to understand and hopefully resolve them.

In https://reviews.llvm.org/D34158#837130, @joerg wrote:

> In https://reviews.llvm.org/D34158#836026, @jyknight wrote:
>
> > In https://reviews.llvm.org/D34158#827178, @joerg wrote:
> >
> > > (2) It adds magic behavior that can make debugging more difficult. Partially preprocessed sources for example could be compiled with plain -c before, now they need a different command line.
> >
> >
> > If this is a problem, making it be Linux-only does _nothing_ to solve it. But I don't actually see how this is a substantively new problem? Compiling with plain -c before
> >  would get #defines for those predefined macros that the compiler sets, even though you may not have wanted those. Is this fundamentally different?
>
>
> It makes it a linux-only problem. As such, it is something *I* only care about secondary. A typical use case I care about a lot is pulling the crash report sources from my (NetBSD) build machine,
>  extracting the original command line to rerun the normal compilation with -save-temps. I don't necessarily have the (same) system headers on the machine I use for debugging and that's exactly
>  the kind of use case this change breaks. All other predefined macros are driven by the target triple and remain stable.

"it's Linux only so I don't care if it's broken." is still not very helpful. :)

But I do think understand what you're saying now, so thanks for the elaboration.

Firstly, let's consider a "clang foo.i" or "clang -x cpp-output foo.c" compilation. In that case, it *clearly* should not be including the predef file. I think the patch as it stands may not do this properly. A test needs to be added for this to this patch, and perhaps the behavior needs to be fixed as well.

(Sidenote: clang doesn't support preprocessed input properly, but that's another bug, and we certainly ought not make it worse. Check out e.g. "int main() { return __GNUC__; }". it should report that __GNUC__ is undeclared, but instead compiles a program that returns 4.)

But, that's not the case you're talking about above -- you're not talking about compiling preprocessed output, you're talking about taking output that comes from -frewrite-includes.

Let me recap the scenario:

1. Start with a source file foo.c, with this content:

#include <stdlib.h>
#pragma clang __debug parser_crash

2. Run "clang foo.c". It crashes, and dumps a /tmp/foo-XXX.c and a /tmp/foo-XXX.sh script.

The .c file is generated via -frewrite-includes, so it's _not_ already preprocessed, it simply has all includes pulled into a single file. It also _doesn't_ insert the compiler-predefined macros at the top, but it _will_ include the content of this stdc-predef.h file.

OK, so then...
The generated script invokes a -cc1 command line, with all the include arguments stripped out of the command. (TO FIX: We should be stripping the new arg as well: add "-fsystem-include-if-exists" argument to the list of include things in the skipArgs() function in lib/Driver/Job.cpp). Even without that change, it's actually already fine, as there is no include path specified in which to find the file -- but it's cleaner to strip it, so let's do that. The reproducer script will thus run correctly, and not include the file.

Now, the "/tmp/foo-XXX.sh" also has a line labeled "Driver args: " with the original command-line on it. If I understand correctly, you then like to take this simpler Driver command-line, and edit it manually: add -save-temps, and change the input filename  to the "/tmp/foo-XXX.c" file, and run that, instead of actually invoking the reproducer foo-XXX.sh.

Since stdc-predef.h is included automatically, it will now be present twice -- first, it will read the one from your system's /usr/include, and then the copy inlined into the /tmp/foo-XXX.c file. That's not what you desired. You wanted nothing from your /usr/include to be used.

The fix for the end-user here is easy: you can add -nostdinc which will suppress all the default include paths, and thus it will not find this predef file from your system include dir.

I'll note that you'd also have had an issue if the original driver command-line had a "-include" option in it, which you would have needed to edit out manually as well. (But I understand that is less common.)

Have I correctly described the situation? I guess my feeling here is that this is somewhat of an edge-case, and that the workaround (-nostdinc) is a sufficient fix.

>>> (3) It seems to me that the GNU userland (and maybe Windows) is the exception to a well integrated tool chain. Most other platforms have a single canonical
>>>  libc, libm and libpthread implementation and can as such directly define all the relevant macros directly in the driver.
>> 
>> I don't think this is accurate. There's many platforms out there, and for almost none of them do we have exact knowledge of the features of the libc encoded
>>  into the compiler. I'd note that not only do you need this for every (OS, libc) combination, you'd need it for every (OS, libc-VERSION) combination.
> 
> Not really. The feature set is rarely changing and generally will have only a cut-off version.

Yes, but this is information that the compiler has no real need to know, and that for many platforms would be difficult and problematic to coordinate.

https://reviews.llvm.org/D34158