[Clang] Prioritise built-in headers, even on musl.

[al45tair]

Clang was putting its built-in headers at the end of the search path if running on musl; this was a mistake, because it breaks libc++, as the latter tries to include the built-in header and then the #include_next in the built-in header fails.

The right solution here is to have the built-in headers remain in their usual location in the search path, and then if it's desirable to override them for musl, have them explicitly include the musl header with #include_next. This is the solution that is already in use for other platforms.

rdar://118881637

[llvmbot]

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Alastair Houghton (al45tair)

Changes

Clang was putting its built-in headers at the end of the search path if running on musl; this was a mistake, because it breaks libc++, as the latter tries to include the built-in header and then the #include_next in the built-in header fails.

The right solution here is to have the built-in headers remain in their usual location in the search path, and then if it's desirable to override them for musl, have them explicitly include the musl header with #include_next. This is the solution that is already in use for other platforms.

rdar://118881637

---

Full diff: https://github.com/llvm/llvm-project/pull/85092.diff

4 Files Affected:

• (modified) clang/lib/Basic/Targets/OSTargets.h (+2)
• (modified) clang/lib/Driver/ToolChains/Linux.cpp (+13-9)
• (modified) clang/lib/Headers/float.h (+3-7)
• (modified) clang/lib/Headers/stddef.h (+10)

diff --git a/clang/lib/Basic/Targets/OSTargets.h b/clang/lib/Basic/Targets/OSTargets.h
index 4366c1149e4053..7c5ef420757ef3 100644
--- a/clang/lib/Basic/Targets/OSTargets.h
+++ b/clang/lib/Basic/Targets/OSTargets.h
@@ -331,6 +331,8 @@ class LLVM_LIBRARY_VISIBILITY LinuxTargetInfo : public OSTargetInfo<Target> {
     } else {
         Builder.defineMacro("__gnu_linux__");
     }
+    if (Triple.isMusl())
+      Builder.defineMacro("__musl__", "1");
     if (Opts.POSIXThreads)
       Builder.defineMacro("_REENTRANT");
     if (Opts.CPlusPlus)
diff --git a/clang/lib/Driver/ToolChains/Linux.cpp b/clang/lib/Driver/ToolChains/Linux.cpp
index 6c2f23e57bce05..857cbb73240cf6 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -629,13 +629,20 @@ void Linux::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
 
   // Add 'include' in the resource directory, which is similar to
   // GCC_INCLUDE_DIR (private headers) in GCC. Note: the include directory
-  // contains some files conflicting with system /usr/include. musl systems
-  // prefer the /usr/include copies which are more relevant.
-  SmallString<128> ResourceDirInclude(D.ResourceDir);
-  llvm::sys::path::append(ResourceDirInclude, "include");
-  if (!DriverArgs.hasArg(options::OPT_nobuiltininc) &&
-      (!getTriple().isMusl() || DriverArgs.hasArg(options::OPT_nostdlibinc)))
+  // contains some files conflicting with system /usr/include.
+  //
+  // Note: the include order used to be different on musl systems because
+  // of the expectation that that users of that C library would use the
+  // C library's copies of the "built-in" headers.  This was a mistake;
+  // it's better to adapt the built-in headers to fall through in that case
+  // (using #include_next), since otherwise if they get pulled in through
+  // some other mechanism, __has_include_next(<header>) will fail and then
+  // they'll try to redefine things, which causes errors.
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+    SmallString<128> ResourceDirInclude(D.ResourceDir);
+    llvm::sys::path::append(ResourceDirInclude, "include");
     addSystemInclude(DriverArgs, CC1Args, ResourceDirInclude);
+  }
 
   if (DriverArgs.hasArg(options::OPT_nostdlibinc))
     return;
@@ -676,9 +683,6 @@ void Linux::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   addExternCSystemInclude(DriverArgs, CC1Args, concat(SysRoot, "/include"));
 
   addExternCSystemInclude(DriverArgs, CC1Args, concat(SysRoot, "/usr/include"));
-
-  if (!DriverArgs.hasArg(options::OPT_nobuiltininc) && getTriple().isMusl())
-    addSystemInclude(DriverArgs, CC1Args, ResourceDirInclude);
 }
 
 void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
diff --git a/clang/lib/Headers/float.h b/clang/lib/Headers/float.h
index 0e73bca0a2d6e4..715a4eb54b54a4 100644
--- a/clang/lib/Headers/float.h
+++ b/clang/lib/Headers/float.h
@@ -10,15 +10,11 @@
 #ifndef __CLANG_FLOAT_H
 #define __CLANG_FLOAT_H
 
-/* If we're on MinGW, fall back to the system's float.h, which might have
- * additional definitions provided for Windows.
- * For more details see http://msdn.microsoft.com/en-us/library/y0ybw9fy.aspx
- *
- * Also fall back on Darwin and AIX to allow additional definitions and
- * implementation-defined values.
+/* On various platforms, fall back to the system's float.h, which might have
+ * additional definitions and/or implementation-defined values.
  */
 #if (defined(__APPLE__) || defined(__MINGW32__) || defined(_MSC_VER) ||        \
-     defined(_AIX)) &&                                                         \
+     defined(_AIX) || defined(__musl__)) &&                                    \
     __STDC_HOSTED__ && __has_include_next(<float.h>)
 
 /* Prior to Apple's 10.7 SDK, float.h SDK header used to apply an extra level
diff --git a/clang/lib/Headers/stddef.h b/clang/lib/Headers/stddef.h
index e0ad7b8d17aff9..11e7b8a4d645fa 100644
--- a/clang/lib/Headers/stddef.h
+++ b/clang/lib/Headers/stddef.h
@@ -28,6 +28,14 @@
  * When clang modules are not enabled, the header guards can function in the
  * normal simple fashion.
  */
+
+#if defined(__musl__)
+
+// On musl systems, use the system header
+#include_next <stddef.h>
+
+#else
+
 #if !defined(__STDDEF_H) || __has_feature(modules) ||                          \
     (defined(__STDC_WANT_LIB_EXT1__) && __STDC_WANT_LIB_EXT1__ >= 1) ||        \
     defined(__need_ptrdiff_t) || defined(__need_size_t) ||                     \
@@ -121,3 +129,5 @@ __WINT_TYPE__ directly; accommodate both by requiring __need_wint_t */
 #endif /* __need_wint_t */
 
 #endif
+
+#endif /* !defined(__musl__) */

[al45tair]

Going to merge this in Apple's fork instead.