[Driver] Prioritize SYSROOT/usr/include over RESOURCE_DIR/include on linux-musl

[MaskRay]

Summary

On a musl-based Linux distribution, stdalign.h stdarg.h stdbool.h stddef.h stdint.h stdnoreturn.h are expected to be provided by musl (/usr/include), instead of RESOURCE_DIR/include.
Reorder RESOURCE_DIR/include to fix the search order problem.
(Currently musl doesn't provide stdatomic.h. stdatomic.h is still found in RESOURCE_DIR/include.)

gcc on musl has a similar search order:

/usr/lib/gcc/x86_64-alpine-linux-musl/8.3.0/../../../../include/c++/8.3.0
/usr/lib/gcc/x86_64-alpine-linux-musl/8.3.0/../../../../include/c++/8.3.0/x86_64-alpine-linux-musl
/usr/lib/gcc/x86_64-alpine-linux-musl/8.3.0/../../../../include/c++/8.3.0/backward
/usr/local/include
/usr/include/fortify
/usr/include
/usr/lib/gcc/x86_64-alpine-linux-musl/8.3.0/include

This is different from a glibc-based distribution where RESOURCE_DIR/include is placed before SYSROOT/usr/include.

[dalias]

Just chiming in to confirm that the requested change is correct for musl. musl does not support use/mixing of compiler-provided std headers with its headers, and intentionally has no mechanism for communicating with such headers as to which types have already been defined or still need to be defined. If the current include order, with clang's headers before the libc ones, works in some situations, it's only by accident.

My understanding is that most if not all of the BSDs also work this way; GCC has their libc header dirs ordered before the GCC include dir. So it probably makes sense to do the same change for them.

[phosek]

This looks reasonable to me; I'd prefer to clean up Clang's internal headers to be compatible with both FreeBSD's and musl's standard headers as suggested in http://lists.llvm.org/pipermail/llvm-dev/2019-August/134353.html, but I'm not sure how feasible or doable that is.

[dalias]

It's not just infeasible; it's impossible by design. musl's headers do not support any contract with compiler-provided versions of the std headers, and won't. Attempting to use them in place of the libc ones is unsupported usage and will periodically break.

[phosek]

LGTM

[raj.khem]

Can we bring this to 9.x release branch as well please ?

[raj.khem]

one issue I now see is that when we have some thing like

static const struct {

        const wchar_t *name;
        int (*func)(EditLine *, int, const wchar_t **);
} cmds[] = {
        { L"bind",              map_bind        },
        { L"echotc",            terminal_echotc },
        { L"edit",              el_editmode     },
        { L"history",           hist_command    },
        { L"telltc",            terminal_telltc },
        { L"settc",             terminal_settc  },
        { L"setty",             tty_stty        },
        { NULL,                 NULL            }
};

clang complains

warning: incompatible pointer types initializing 'const wchar_t *' (aka 'const long *') with an expression of type 'int [5]' [-Wincompatible-pointer-types]

{ L"bind",              map_bind        },
  ^~~~~~~

it seems L"..." prefix is treated as int where as in musl wchar_t is of long int type

[dalias]

That's a separate issue of clang having a slight types-ABI mismatch with some 32-bit archs whose original ABIs used long instead of int for wchar_t. The wrong header order makes it quickly apparent, but these are independent; wrong header order is still wrong and will break (other) things even without this type mismatch. Backport of the fix would be much appreciated.

[MaskRay]

@hans Can this be put on your list of things for 9.0.1? I searched for [meta] 9.0.1 on bugs.llvm.org but can't find a release blocker bug so I have to bother you directly...

[hans]

In D65699#1662936, @MaskRay wrote:

@hans Can this be put on your list of things for 9.0.1? I searched for [meta] 9.0.1 on bugs.llvm.org but can't find a release blocker bug so I have to bother you directly...

Right, the bug hasn't been filed yet. I've put it on my local list. Thanks!

[raj.khem]

In D65699#1662915, @dalias wrote:

That's a separate issue of clang having a slight types-ABI mismatch with some 32-bit archs whose original ABIs used long instead of int for wchar_t. The wrong header order makes it quickly apparent, but these are independent; wrong header order is still wrong and will break (other) things even without this type mismatch. Backport of the fix would be much appreciated.

@dalias I agree its an issue that comes to fore due to this change and was latent, this however now fails to build libedit, which is a prerequisite for building clang itself, so its kind of pesky problem now.

cfe/trunk/lib/Driver/ToolChains/Linux.cpp

void Linux::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
   std::string SysRoot = computeSysRoot();
 
   if (DriverArgs.hasArg(clang::driver::options::OPT_nostdinc))
     return;
 
   if (!DriverArgs.hasArg(options::OPT_nostdlibinc))
     addSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/local/include");
 
-  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
-    SmallString<128> P(D.ResourceDir);
-    llvm::sys::path::append(P, "include");
-    addSystemInclude(DriverArgs, CC1Args, P);
-  }
+  SmallString<128> ResourceDirInclude(D.ResourceDir);
+  llvm::sys::path::append(ResourceDirInclude, "include");
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc) &&
+      (!getTriple().isMusl() || DriverArgs.hasArg(options::OPT_nostdlibinc)))
+    addSystemInclude(DriverArgs, CC1Args, ResourceDirInclude);
 
   if (DriverArgs.hasArg(options::OPT_nostdlibinc))
     return;
 
   // Check for configure-time C include directories.
   StringRef CIncludeDirs(C_INCLUDE_DIRS);
   if (CIncludeDirs != "") {
     SmallVector<StringRef, 5> dirs;
if (getTriple().getOS() == llvm::Triple::RTEMS)
     return;
 
   // Add an include of '/include' directly. This isn't provided by default by
   // system GCCs, but is often used with cross-compiling GCCs, and harmless to
   // add even when Clang is acting as-if it were a system compiler.
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/include");
 
   addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + "/usr/include");
+
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc) && getTriple().isMusl())
+    addSystemInclude(DriverArgs, CC1Args, ResourceDirInclude);
 }
 
 static std::string DetectLibcxxIncludePath(llvm::vfs::FileSystem &vfs,
                                            StringRef base) {
   std::error_code EC;
   int MaxVersion = 0;
   std::string MaxVersionString = "";
   for (llvm::vfs::directory_iterator LI = vfs.dir_begin(base, EC), LE;

cfe/trunk/test/Driver/linux-musl-header-search.cpp

+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
+// RUN:     -target x86_64-linux-musl -stdlib=libc++ \
+// RUN:     -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin \
+// RUN:     -resource-dir=%S/Inputs/resource_dir \
+// RUN:     --sysroot=%S/Inputs/basic_linux_libcxx_tree --gcc-toolchain= \
+// RUN:   | FileCheck --check-prefix=CHECK-X86-64-LIBCXX %s
+
+// RESOURCE_DIR/include comes after /usr/include on linux-musl.
+// This is different from a glibc-based distribution.
+// CHECK-X86-64-LIBCXX: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-X86-64-LIBCXX: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-X86-64-LIBCXX: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+// CHECK-X86-64-LIBCXX: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
+// CHECK-X86-64-LIBCXX: "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
+// CHECK-X86-64-LIBCXX: "-internal-isystem" "[[RESOURCE_DIR]]{{/|\\\\}}include"
+
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only -nobuiltininc 2>&1 \
+// RUN:     -target x86_64-linux-musl \
+// RUN:     -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin \
+// RUN:     -resource-dir=%S/Inputs/resource_dir \
+// RUN:     --sysroot=%S/Inputs/basic_linux_libcxx_tree --gcc-toolchain= \
+// RUN:   | FileCheck --check-prefix=CHECK-NOBUILTININC %s
+
+// CHECK-NOBUILTININC: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-NOBUILTININC-NOT: "-internal-isystem" "[[RESOURCE_DIR]]{{/|\\\\}}include"
+
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only -nostdlibinc 2>&1 \
+// RUN:     -target x86_64-linux-musl \
+// RUN:     -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin \
+// RUN:     -resource-dir=%S/Inputs/resource_dir \
+// RUN:     --sysroot=%S/Inputs/basic_linux_libcxx_tree --gcc-toolchain= \
+// RUN:   | FileCheck --check-prefix=CHECK-NOSTDLIBINC %s
+
+// CHECK-NOSTDLIBINC: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-NOSTDLIBINC-NOT: "-internal-externc-isystem"
+// CHECK-NOSTDLIBINC-NOT: "-internal-isystem"
+// CHECK-NOSTDLIBINC: "-internal-isystem" "[[RESOURCE_DIR]]{{/|\\\\}}include"
+// CHECK-NOSTDLIBINC-NOT: "-internal-externc-isystem"
+// CHECK-NOSTDLIBINC-NOT: "-internal-isystem"