Fix compilation with upstream clang builtin headers

[arichardson]

Summary

By using -nobuiltininc and adding the clang builtin headers resource dir to
the end of the compiler header search path, we can still find headers such
as immintrin.h but find the FreeBSD version of stddef.h/stdarg.h/.. first.

This currently only works if the -print-resource-dir flag is supported by
clang which was introduced with clang 5.0.

Test Plan

build worked for me with upstream clang. Would be nice to fix
clang/FreeBSD headers to be compatible instead, but for now this works.

[arichardson]

Fix inverted condition

[arichardson]

Remove debug .info

[arichardson]

Fix typo in path

[jhb]

Do we know what incompatibilities there are currently? For external GCC we are always using the compiler-provided headers and then fixing any issues we run into in the code base.

[bdrewery]

share/mk/bsd.compiler.mk

202–204 ↗

(On Diff #47615)

Please also add COMPILER_RESOURCE_DIR to the _TOOLCHAIN_METADATA_VARS list in Makefile.inc1 or this lookup will execute in parts of the build where it should already be known/cached.

[arichardson]

In D17002#365778, @jhb wrote:

Do we know what incompatibilities there are currently? For external GCC we are always using the compiler-provided headers and then fixing any issues we run into in the code base.

There were a few issues last time I tried:

• stddef.h: Clang uses _WCHAR_T instead of _WCHAR_T_DECLARED (and similar for size_t and ptrdiff_t, etc.) so we get typedef redefinitions because the clang builtin headers are parsed before the ones in $WORLDTMP.
• stdarg.h: uses _VA_LIST instead of _VA_LIST_DECLARED so some headers that check for the define instead of including stdarg.h break.
• stdint.h: uses macro redefinitions without #undef before, but for some reason is not treated as a system header so I got -Werror failures.
• some other errors where I don't remember the details.

[arichardson]

In D17002#365858, @arichardson wrote:

• stdint.h: uses macro redefinitions without #undef before, but for some reason is not treated as a system header so I got -Werror failures.

Turns out we are compiling some files with -Wsystem-headers so that explains the errors. I can submit a patch to clang to fix those warnings but until then something like this will be needed.

We might even be able to fix this without parsing the compiler resource dir. If we add -isystem ${WORLDTMP}/usr/include it should come before the builtins instead after in the search path.
I'll try that approach since it will avoid another fork() + more complexity in bsd.compiler.mk.

[arichardson]

In D17002#366002, @arichardson wrote:

In D17002#365858, @arichardson wrote:

• stdint.h: uses macro redefinitions without #undef before, but for some reason is not treated as a system header so I got -Werror failures.

Turns out we are compiling some files with -Wsystem-headers so that explains the errors. I can submit a patch to clang to fix those warnings but until then something like this will be needed.

We might even be able to fix this without parsing the compiler resource dir. If we add -isystem ${WORLDTMP}/usr/include it should come before the builtins instead after in the search path.
I'll try that approach since it will avoid another fork() + more complexity in bsd.compiler.mk.

I just tried this approach and it works for C but breaks all C++ files:

.if ${MK_CLANG_BOOTSTRAP} == "no" && !defined(BOOTSTRAPPING) && !empty(SYSROOT)
CFLAGS+= -isystem ${SYSROOT}/usr/include
.endif

This changes the #include search to

#include <...> search starts here:
 /exports/users/alr48/sources/cheribsd/contrib/cheri-libunwind/include
 /exports/users/alr48/sources/cheribsd/lib/libgcc_eh
 /home/alr48/obj/build/cheribsd-obj-128/exports/users/alr48/sources/cheribsd/mips.mips64/tmp/usr/include
 /home/alr48/cheri/output/sdk/lib/clang/7.0.0/include

This means stddef.h is now taken from FreeBSD instead of the clang headers and everything compiles fine.

However, when compiling a C++ file we get the following:

#include <...> search starts here:
 /exports/users/alr48/sources/cheribsd/contrib/cheri-libunwind/include
 /exports/users/alr48/sources/cheribsd/lib/libgcc_eh
 /home/alr48/obj/build/cheribsd-obj-128/exports/users/alr48/sources/cheribsd/mips.mips64/tmp/usr/include
 /home/alr48/obj/build/cheribsd-obj-128/exports/users/alr48/sources/cheribsd/mips.mips64/tmp/usr/include/c++/v1
 /home/alr48/cheri/output/sdk/lib/clang/7.0.0/include
End of search list.
In file included from /exports/users/alr48/sources/cheribsd/contrib/cheri-libunwind/src/libunwind.cpp:18:
In file included from /home/alr48/obj/build/cheribsd-obj-128/exports/users/alr48/sources/cheribsd/mips.mips64/tmp/usr/include/c++/v1/new:89:
In file included from /home/alr48/obj/build/cheribsd-obj-128/exports/users/alr48/sources/cheribsd/mips.mips64/tmp/usr/include/c++/v1/exception:81:
In file included from /home/alr48/obj/build/cheribsd-obj-128/exports/users/alr48/sources/cheribsd/mips.mips64/tmp/usr/include/c++/v1/cstddef:44:
/home/alr48/cheri/output/sdk/lib/clang/7.0.0/include/stddef.h:124:9: error: 'offsetof' macro redefined [-Werror,-Wmacro-redefined]
#define offsetof(t, d) __builtin_offsetof(t, d)
        ^
/home/alr48/obj/build/cheribsd-obj-128/exports/users/alr48/sources/cheribsd/mips.mips64/tmp/usr/include/stddef.h:75:9: note: previous definition is here
#define offsetof(type, field)   __offsetof(type, field)
        ^
1 error generated.

This is because cstddef uses #include_next and then finds stddef.h in the clang builtin includes directory.
I'll see if I can get this to work without using -idirafter for the resource dir, but it will probably be even more hacky.

[arichardson]

Since I can't come up with a better solution update this patch

• Add to TOOLCHAIN_METADATA_VARS
• Fix typo in comment

[arichardson]

ping?

[arichardson]

@bdrewery is this okay to commit now?

[arichardson]

ping

[bdrewery]

What case does this fix exactly?

Also is there any chance that SYSTEM_COMPILER is a problem here? Does /usr/bin/cc need to have a certain directory path being used that matches the bootstrap compiler? Said another way, are we planning to change the directory used here such that /usr/bin/cc may do the wrong thing? SYSTEM_COMPILER depends on /usr/bin/cc matching the behavior of what is to be built as the bootstrap compiler. --sysroot is always added to accomplish that.

[arichardson]

This change is required to build with an unmodified upstream LLVM since the default upstream clang builtin headers are incompatible with the ones in /usr/bin/include (see my earlier comments in this review).

--sysroot only affects the normal include paths, but does not change the search path for compiler-provided headers.
SYSTEM_COMPILER compiler should not make any difference here and will work as expected with /usr/bin/cc:

/usr/bin/cc -print-resource-dir
/usr/lib/clang/6.0.1

/usr/lib/clang/6.0.1/include contains all the necessary compiler-builtin headers (but we don't install the stddef.h/etc. headers that are incompatible)

[brooks]

In D17002#489608, @arichardson wrote:

This change is required to build with an unmodified upstream LLVM since the default upstream clang builtin headers are incompatible with the ones in /usr/bin/include (see my earlier comments in this review).

--sysroot only affects the normal include paths, but does not change the search path for compiler-provided headers.
SYSTEM_COMPILER compiler should not make any difference here and will work as expected with /usr/bin/cc:

/usr/bin/cc -print-resource-dir
/usr/lib/clang/6.0.1

/usr/lib/clang/6.0.1/include contains all the necessary compiler-builtin headers (but we don't install the stddef.h/etc. headers that are incompatible)

All the devel/llvm* ports work around this issue by not installing a subset of headers (requiring a patch to a somewhat frequently changing file). It would be nice to not have to do that and this change allows us to avoid needing to make that change (once it's merged to all supported releases).

[jhb]

Does this work with GCC? Seems no:

> mips-unknown-freebsd12.1-gcc -print-resource-dir
mips-unknown-freebsd12.1-gcc: error: unrecognized command line option '-print-resource-dir'; did you mean '-print-search-dirs'?
mips-unknown-freebsd12.1-gcc: fatal error: no input files
compilation terminated.

[arichardson]

In D17002#489675, @jhb wrote:

Does this work with GCC? Seems no:

> mips-unknown-freebsd12.1-gcc -print-resource-dir
mips-unknown-freebsd12.1-gcc: error: unrecognized command line option '-print-resource-dir'; did you mean '-print-search-dirs'?
mips-unknown-freebsd12.1-gcc: fatal error: no input files
compilation terminated.

That should be fine since the GCC provided stddef.h supports every macro ever defined by any stddef.h implementation so we don't need to skip it.
The ${COMPILER_RESOURCE_DIR} != "unknown" check should ensure that nothing changes for GCC

https://github.com/gcc-mirror/gcc/blob/master/gcc/ginclude/stddef.h#L243

[arichardson]

Ping? Can we merge this or do I have to convince upstream LLVM to add checks for the FreeBSD _FOO_DECLARED macros to their stddef.h/stdarg.h? I'm not sure how willing they would be to make that change though.

[dim]

In D17002#506769, @arichardson wrote:

Ping? Can we merge this or do I have to convince upstream LLVM to add checks for the FreeBSD _FOO_DECLARED macros to their stddef.h/stdarg.h? I'm not sure how willing they would be to make that change though.

This should be solved upstream, once and for all. There is a reason we don't install those headers, the port shouldn't install them either.

[arichardson]

In D17002#526349, @dim wrote:

In D17002#506769, @arichardson wrote:

Ping? Can we merge this or do I have to convince upstream LLVM to add checks for the FreeBSD _FOO_DECLARED macros to their stddef.h/stdarg.h? I'm not sure how willing they would be to make that change though.

This should be solved upstream, once and for all. There is a reason we don't install those headers, the port shouldn't install them either.

I think they should be installed. It allows e.g. building binaries for a bare metal target. Fixing it in upstream llvm would be ideal but we could also change the FreeBSD headers to check for the llvm macros?