Make sys/mips/include/stdarg.h compatible with clang's builtin headers

[arichardson]

Summary

This is required when building with an external clang.

I have created https://reviews.llvm.org/D44604 to try and upstream the clang stdarg.h changes I made for CHERI clang. However, it may be possible that the clang <stdarg.h> is included before <machine/stdarg.h> so we should check before defining va_start, etc.

[brooks]

LGTM

[imp]

A better read on 'why' they aren't defined would be good to know. Is this a freebsd specific extension to our clang? Are other architectures affected?

[arichardson]

In D14248#298772, @imp wrote:

A better read on 'why' they aren't defined would be good to know. Is this a freebsd specific extension to our clang? Are other architectures affected?

As far as I can tell the FreeBSD clang install does not include the stddef.h, stdarg.h, etc from the clang sources but instead uses the files in /usr/include.
I am building clang using the CMakeLists.txt from LLVM so it installs those files.
The clang builtin headers already define these macros and in some (most?) cases are included before the machine/stdarg.h header so we get macro redefined warnings (-Werror) here.

[imp]

In D14248#300057, @arichardson wrote:

In D14248#298772, @imp wrote:

A better read on 'why' they aren't defined would be good to know. Is this a freebsd specific extension to our clang? Are other architectures affected?

As far as I can tell the FreeBSD clang install does not include the stddef.h, stdarg.h, etc from the clang sources but instead uses the files in /usr/include.
I am building clang using the CMakeLists.txt from LLVM so it installs those files.
The clang builtin headers already define these macros and in some (most?) cases are included before the machine/stdarg.h header so we get macro redefined warnings (-Werror) here.

So is that just for MIPS, or do all architectures need this pattern? Mips is the odd-man out here. i386, amd64, arm, and arm64 use sys/_stdarg.h. The whole rest of this file is not used at all (since GCC is >= 3! three! released in 2001!

Maybe something like https://reviews.freebsd.org/D14323 is better here?

[arichardson]

D14323 looks better, I'll just somehow have to change the clang builtin headers to not define __va_copy(), etc.
I currently get the following error with that, but I can work around that.

In file included from /Users/alex/cheri/freebsd-master/lib/libcam/scsi_cmdparse.c:54:
In file included from /Users/alex/cheri/freebsd-master/sys/cam/cam_ccb.h:44:
In file included from /Users/alex/cheri/freebsd-master/sys/cam/scsi/scsi_all.h:28:
In file included from /Users/alex/devel/build/freebsd/Users/alex/cheri/freebsd-master/mips.mips64/tmp/usr/include/machine/stdarg.h:9:
/Users/alex/cheri/freebsd-master/sys/sys/_stdarg.h:46:11: error: 'va_start' macro redefined [-Werror,-Wmacro-redefined]
  #define va_start(ap, last) __builtin_va_start((ap), (last))
          ^
/Users/alex/cheri/output/sdk/lib/clang/7.0.0/include/stdarg.h:35:9: note: previous definition is here
#define va_start(ap, param) __builtin_va_start(ap, param)
        ^
In file included from /Users/alex/cheri/freebsd-master/lib/libcam/scsi_cmdparse.c:54:
In file included from /Users/alex/cheri/freebsd-master/sys/cam/cam_ccb.h:44:
In file included from /Users/alex/cheri/freebsd-master/sys/cam/scsi/scsi_all.h:28:
In file included from /Users/alex/devel/build/freebsd/Users/alex/cheri/freebsd-master/mips.mips64/tmp/usr/include/machine/stdarg.h:9:
/Users/alex/cheri/freebsd-master/sys/sys/_stdarg.h:47:11: error: 'va_arg' macro redefined [-Werror,-Wmacro-redefined]
  #define va_arg(ap, type) __builtin_va_arg((ap), type)
          ^
/Users/alex/cheri/output/sdk/lib/clang/7.0.0/include/stdarg.h:41:9: note: previous definition is here
#define va_arg(ap, type)    __builtin_va_arg(ap, type)
        ^
In file included from /Users/alex/cheri/freebsd-master/lib/libcam/scsi_cmdparse.c:54:
In file included from /Users/alex/cheri/freebsd-master/sys/cam/cam_ccb.h:44:
In file included from /Users/alex/cheri/freebsd-master/sys/cam/scsi/scsi_all.h:28:
In file included from /Users/alex/devel/build/freebsd/Users/alex/cheri/freebsd-master/mips.mips64/tmp/usr/include/machine/stdarg.h:9:
/Users/alex/cheri/freebsd-master/sys/sys/_stdarg.h:48:11: error: '__va_copy' macro redefined [-Werror,-Wmacro-redefined]
  #define __va_copy(dest, src) __builtin_va_copy((dest), (src))
          ^
/Users/alex/cheri/output/sdk/lib/clang/7.0.0/include/stdarg.h:47:9: note: previous definition is here
#define __va_copy(d,s) __builtin_va_copy(d,s)
        ^
In file included from /Users/alex/cheri/freebsd-master/lib/libcam/scsi_cmdparse.c:54:
In file included from /Users/alex/cheri/freebsd-master/sys/cam/cam_ccb.h:44:
In file included from /Users/alex/cheri/freebsd-master/sys/cam/scsi/scsi_all.h:28:
In file included from /Users/alex/devel/build/freebsd/Users/alex/cheri/freebsd-master/mips.mips64/tmp/usr/include/machine/stdarg.h:9:
/Users/alex/cheri/freebsd-master/sys/sys/_stdarg.h:50:13: error: 'va_copy' macro redefined [-Werror,-Wmacro-redefined]
    #define va_copy(dest, src) __va_copy(dest, src)
            ^
/Users/alex/cheri/output/sdk/lib/clang/7.0.0/include/stdarg.h:51:9: note: previous definition is here
#define va_copy(dest, src)  __builtin_va_copy(dest, src)
        ^
4 errors generated.
*** Error code 1

I'll try to clean up my hacks that I added to CHERI clang and see if upstream will accept them but I feel like they might be reluctant to make clang stddef/stdarg look like the GCC one (which just defines every macro every libc ever used to note that types and macros have been defined).

[imp]

I'm inclined to hold off a little on this one still... If I made the changes in other reviews, as seems likely, then we can revisit. This will give you time as well. I wonder why we've not hit this with the external toolchain work that's gone on until now.
So let's let this one cook a while longer and clear out other patches while it does.

[brooks]

In D14248#300264, @imp wrote:

I'm inclined to hold off a little on this one still... If I made the changes in other reviews, as seems likely, then we can revisit. This will give you time as well. I wonder why we've not hit this with the external toolchain work that's gone on until now.
So let's let this one cook a while longer and clear out other patches while it does.

We probably aren't hitting this with external toolchain because we don't install std*.h headers in the clang and llvm ports because they have historically caused problems.

[jhb]

So I sort of tripped over this in my work to make the external GCC toolchains less terrible so that --sysroot actually works. As part of that we started using the stdarg.h that GCC ships instead of the one from the system (which is fine). I did once get a similar build failure, but couldn't reproduce it when doing a clean build. However, I think the only way you can have a problem is if some userland code is including both <machine/stdarg.h> and <stdarg.h> (could be via a nested include of <machine/stdarg.h> by some header in sys). However, if we are doing that, that is the problem. Userland should only ever use <stdarg.h> and not <machine/stdarg.h>. If we fix those cases I don't think we will need to patch <sys/_stdarg.h> anymore. Do you have the log from your most recent build failure without stdarg.h patched? I think before when I got errors with external GCC it was in libcam (which makes sense as I think there are some CAM files shared between the kernel and userland which might have the problem of using <machine/stdarg.h> in userland).

[jhb]

My recent build tests were against the wrong tree which is why this had seemed to be fixed. It was a single header (scsi_all.h) that was using <machine/stdarg.h> in userland. Fixing that allowed my MIPS build world with external GCC to complete fine using GCC's std*.h headers in /usr/local/lib/gcc/<mumble>/include. I've uploaded that diff to D14776. Hopefully this fixes the same issue with clang when using clang's stdarg.h.

[jhb]

I think this one can be abandoned now?

[arichardson]

Should no longer be needed