Add Hurd toolchain support to Clang

[sthibaul]

The Hurd::Hurd constructor would actually need to do the same gcc inclusion path detection as on Linux, but let's leave this aside for now, this commit is enough for a build without libc++.

[kristina]

lib/Driver/ToolChains/Clang.cpp

533–534

Please keep line lengths to 80 columns at most, it's one of the few hard rules outlined in the developer policy.

[kristina]

A few style naming/comments.

lib/Driver/ToolChains/Hurd.cpp

37

Does this need a switch? Wouldn't an if be sufficient?

107

Variable names should be capitalized.

126

Default should generally be the first case, if present.

[sthibaul]

I commented one of them, and will fix the rest.

lib/Driver/ToolChains/Hurd.cpp

37

An if would work, yes, it's just to make it easily extensible for future cases, just like the getMultiarchTriple function in Linux.cpp

[krytarowski]

lib/Driver/ToolChains/Hurd.cpp

137

Is this some hurd standard or personal taste?

[sthibaul]

lib/Driver/ToolChains/Hurd.cpp

137

I copied this from the Linux.cpp file.

Actually it happens to be a standard in the pure GNU system which does not define a /usr. Debian GNU/Hurd eventually migrated to having a real /usr just like other Debian ports to keep things coherent, but the GNU system is supposed to have prefix=/

[sthibaul]

In this version, the Driver introduces the "-hurd-" part

[sthibaul]

This includes version finding the gcc Hurd triplet (i[3456]-gnu) in the Gcc detector.

[kristina]

Alright, once this is fully reviewed, if accepted, I can land the LLVMSupport change and add your new target and close the stack. In the meantime, adding some more reviewers who have more experience with Clang CR than me and who can hopefully weigh in some opinions. (Sorry about the delays, looks like I'll be the one seeing addition of this target through, not landed the LLVMSupport change yet since I want to make sure this set of patches is good, otherwise the LLVMSupport change on its own holds little weight)

[kristina]

Added first batch of comments regarding the patch. Some style, some semantics-related.

lib/Basic/Targets/OSTargets.h

283

__MACH__ and __HURD__ seem appropriate? Apple Mach (Darwin/XNU) uses __MACH__ with __APPLE__, Hurd should probably follow a similar convention, I don't think there are many places aside from XNU build where __MACH__ is used on its own.

lib/Driver/ToolChains/Hurd.cpp

73

No space here.

79

I'm not quite sure I like this. Also early return should be for the "bad" case, not for the good case, at least IMO, this is not a huge issue but I'll see what others say. I think this may just be subjective.

85

Move semantics? Or is this guaranteed elision (I'm not sure)?

119

Until needed I wouldn't use an array here, also drop const.

[kristina]

lib/Driver/Driver.cpp

392

Same as previous comment regarding this type of function.

[kristina]

Phab lost this inline comment for some reason, but please leave some comment regarding why that part in Driver.cpp does what it does (summarize the conclusion of the discussion in D54378).

lib/Driver/Driver.cpp

416

Please comment the reasoning behind this (ie. the discussion in D54378) for any other maintainers.

[kristina]

The other lost comment was regarding the functions where you're using strcpy instead of idiomatic LLVM and also creating unnecessary temporary std::string instances on the stack.

[kristina]

Commented on that particular idiom, there's two instances where it's used, aside from variable naming issues (pos should be Pos) it's very non idiomatic as far as rest of LLVM code goes, don't pass string literals around as const char*, prefer StringRef instead, that would also save you from needing to resort to using strlen later (sorry for previous comment, I didn't mean strcpy).

lib/Driver/Driver.cpp

400

Please avoid that kind of code and avoid strlen, you should pass things as StringRef as that's the general way of doing things unless you have a good reason for doing so otherwise. This entire function/part that uses it should be rewritten, I especially don't like the temporary std::string on the stack. It may be worth considering SmallString which is a variation of SmallVector or just manipulating the StringRef. You very certainly don't need strlen however, StringRef provides the needed operators, same goes for using std::string::find, just use StringRef instead.

[kristina]

lib/Driver/Driver.cpp

418

Reference to a local variable?

[kristina]

lib/Driver/Driver.cpp

418

Hm, actually this is fine I guess, just avoid strlen and pass literals as StringRefs. I would make sure the triple actually matches before you construct a local though, otherwise you're forcing doing it for every target, which in majority of the cases isn't going to be Hurd. I would still use SmallString instead of std::string but that's more of a nitpick.

[kristina]

Also, this needs unit tests and lit/FileCheck tests.

[rjmccall]

lib/Driver/ToolChains/Hurd.cpp

85

If you're talking about the initialization of SysRoot, yes, initializing a local variable from a temporary is a guaranteed copy-elision, and adding std::move will actually inhibit the optimization.

[kristina]

As discussed in #hurd, a few additional comments. The Hurd codepath should first check if the triple is eligible, ie. minimizing any cost to the driver invocation for most targets, which are not going to be Hurd. In fact I would wrap it in LLVM_UNLIKELY but that's just a suggestion. Once you confirmed that the triple in question is the one you're looking for (from the suffix), you can alter the existing triple. The std::string should be avoided here since even a zero length SmallVector will guarantee not allocating anything unless used. This may be worth factoring out into a separate function entirely, and another important point is avoiding any sort of unneeded overhead unless you've confirmed that it's the Hurd triple.

In general, I've looked over it again and I'd ask to get rid of switches that can be replaced with if statements. If there's a need later, you can add it later. For now, there's no need so I would avoid it. Switch is not generally an easy construct for humans to parse. Avoid arrays with 1 element until needed as well please. You can always introduce further changes as they're needed, however, for now, I'm strongly against adding "clutter" because it may be useful in the future.

I'll end this comment with this: In general when structuring your code, the performance penalty for other targets when the conditions that can be easily tested are not met should pretty much be close to nonexistent. I would suggest keeping that in mind when submitting revisions.

[sthibaul]

lib/Basic/Targets/OSTargets.h

283

There is actually no __HURD__ macro, it's the __GNU__ macro which has that role. __MACH__ should however be there too indeed, as well as __gnu_hurd__ similarly to Linux' __gnu_linux__.

lib/Driver/ToolChains/Hurd.cpp

79

Well, this is inspired from clang/lib/Driver/ToolChains/Linux.cpp, which additionally has some gcc tests, which I'll include in a later patch. That argues for using this way of doing the test since that is how it will be in the end.

[sthibaul]

In general when structuring your code, the performance penalty for other targets when the conditions that can be easily tested are not met should pretty much be close to nonexistent. I would suggest keeping that in mind when submitting revisions.

I know, as discussed on IRC I just hadn't managed to find a way to achieve it in this case :)

But thanks to the IRC discussion I'll be able to do it.

[sthibaul]

I believe this version addresses all the comments.
I could run this with check-all on a linux-amd64 box.

[kristina]

I'll re-review when I'm up, from a quick glance it looks much better but I'll have to patch it over my fork and try out a few things (Mostly x86_64 Linux and Darwin test suites). I think the test is lacking a bit, there's a lot of stuff that isn't covered, and there's a lack of negative tests (ie. when invalid input is supplied and matches the triple suffix). Feel free to run tests though, may find something before me, I'm a bit too tired to reconfigure my buildbot to do what I want right now, so I'll leave it until when I'm up. So yeah I may be being a bit nitpicky but I think tests could cover a little bit more.

It would also be great to get at least one other Clang reviewer to sign off on this. I can sign off on this myself once I test it, but I feel like getting another set of eyes would be good. But yeah if this is all good and someone else can also skim through this and sign off on it, I can commit the stack when I'm up and when I've done some stuff. If you can, try to build/test with a recent Clang as that usually brings out some benign warnings one may miss if using an older SDK.

But very good job in general, this seems a lot better and streamlined than the previous revision.

So that said to other Clang reviewers: Gentle ping, would really love a second set of eyes though, though it can wait until Monday at worst since I'd imagine a lot of people are off right now.

Thank you.

[sthibaul]

I have added a few checks (the ld.so dynamic linker specification, the ../lib32 paths, and /usr/lib/i386-gnu)

About negative tests, what kind of invalid input are you thinking about?

[kristina]

Ping. Would like someone else to co-review this, everything looks fine aside from tests (I think a portion of this could use a short unit test with regards to creating the actual target instance). Also your FileCheck test is only for invoking clang -cc1 from the looks, can you add coverage for static linker invocations (as driver is also generally used to invoke the linker, unless that's explicitly not the case on Hurd)?

[aaron.ballman]

I don't know enough about hurd to review those portions of it, but I have some general comments on the patch.

lib/Basic/Targets/OSTargets.h

279

Missing full stop at the end of the comment.

lib/Driver/ToolChains/Hurd.cpp

66–67

Formatting looks off here as well.

75

What GCC logic above?

we currently -> we are currently

121

This doesn't look unreachable to me?

159

Spurious newline.

162

Can elide braces.

lib/Driver/ToolChains/Hurd.h

23–24

Formatting looks off here; did clang-format produce this?

32–34

Why are these virtual functions?

getDynamicLinker() is implemented but never called in this patch -- is it needed?

36

This appears to be entirely unused.

[sthibaul]

I'll update the diff according to the comments.

lib/Driver/ToolChains/Hurd.cpp

75

That was from Linux.cpp. I have not yet included the gcc pieces above, so the comment doesn't make sense yet indeed.

121

For now it is, we only have x86 architecture for the Hurd. This is like Linux::getDynamicLinker which uses llvm_unreachable in the default case.

lib/Driver/ToolChains/Hurd.h

32–34

I don't know the rationale, I am just reusing the same principle as in Linux.{h,cpp}.

getDynamicLinker is used by Gnu.cpp's ConstructJob.

36

Again, it is used by Gnu.cpp's ConstructJob

[sthibaul]

(there still needs to be work on the test part)

[sthibaul]

I have added static and shared library tests

a short unit test with regards to creating the actual target instance)

I'm not sure what you mean? The tests create an actual binary for the target.

[kristina]

Alright, will patch it into my fork in a bit and try it out, if everything looks good, I'll land the stack. (Again huge sorry about this taking some time, have lots on my hands at the moment)

[kristina]

Your tests don't pass:

********************
Testing Time: 21.51s
********************
Failing Tests (1):
    Clang :: Driver/hurd.c

  Expected Passes    : 470
  Expected Failures  : 3
  Unsupported Tests  : 71
  Unexpected Failures: 1

Seems to be an issue with this particular path:

/q/src/llvm-tainted-8.0/tools/clang/test/Driver/hurd.c:9:11: error: CHECK: expected string not found in input
// CHECK: "-internal-externc-isystem" "[[SYSROOT]]/usr/include/i386-gnu"

[sthibaul]

I don't have this issue with make check-all in clang/. Just to make sure: are the .keep files of the patch getting created? Otherwise the i386-gnu/ directories will not get created and then it's not surprising that clang doesn't add them to the research paths.

[kristina]

Hm, yes you're right, the files aren't in the diff that Phab gives me, this is annoying. I wonder if it's stripping them from the diff for some reason, can you upload the raw diff?

[kristina]

Actually nevermind, I'll just create them myself, seems like a strange bug.

[kristina]

Yes everything passes now, my bad for not noticing.

lib/Basic/Targets.cpp

case llvm::Triple::x86:
     case llvm::Triple::Haiku:
       return new HaikuX86_32TargetInfo(Triple, Opts);
     case llvm::Triple::RTEMS:
       return new RTEMSX86_32TargetInfo(Triple, Opts);
     case llvm::Triple::NaCl:
       return new NaClTargetInfo<X86_32TargetInfo>(Triple, Opts);
     case llvm::Triple::ELFIAMCU:
       return new MCUX86_32TargetInfo(Triple, Opts);
+    case llvm::Triple::Hurd:
+      return new HurdTargetInfo<X86_32TargetInfo>(Triple, Opts);
     default:
       return new X86_32TargetInfo(Triple, Opts);
     }
 
   case llvm::Triple::x86_64:
     if (Triple.isOSDarwin() || Triple.isOSBinFormatMachO())
       return new DarwinX86_64TargetInfo(Triple, Opts);
 

lib/Basic/Targets/OSTargets.h

HaikuTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
     this->SizeType = TargetInfo::UnsignedLong;
     this->IntPtrType = TargetInfo::SignedLong;
     this->PtrDiffType = TargetInfo::SignedLong;
     this->ProcessIDType = TargetInfo::SignedLong;
     this->TLSSupported = false;
   }
 };
 
+// Hurd target
+template <typename Target>
+class LLVM_LIBRARY_VISIBILITY HurdTargetInfo : public OSTargetInfo<Target> {
+protected:
+  void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
+                    MacroBuilder &Builder) const override {
+    // Linux defines; list based off of gcc output
+    DefineStd(Builder, "unix", Opts);
+    Builder.defineMacro("__GNU__");
+    Builder.defineMacro("__GLIBC__");
+    Builder.defineMacro("__ELF__");
+    if (Opts.POSIXThreads)
+      Builder.defineMacro("_REENTRANT");
+    if (Opts.CPlusPlus)
+      Builder.defineMacro("_GNU_SOURCE");
+  }
+public:
+  HurdTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
+      : OSTargetInfo<Target>(Triple, Opts) {}
+};
+
 // Minix Target
 template <typename Target>
 class LLVM_LIBRARY_VISIBILITY MinixTargetInfo : public OSTargetInfo<Target> {
 protected:
   void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
                     MacroBuilder &Builder) const override {
     // Minix defines
 

lib/Driver/CMakeLists.txt

add_clang_library(clangDriver
   ToolChains/Darwin.cpp
   ToolChains/DragonFly.cpp
   ToolChains/FreeBSD.cpp
   ToolChains/Fuchsia.cpp
   ToolChains/Gnu.cpp
   ToolChains/Haiku.cpp
   ToolChains/HIP.cpp
   ToolChains/Hexagon.cpp
+  ToolChains/Hurd.cpp
   ToolChains/Linux.cpp
   ToolChains/MipsLinux.cpp
   ToolChains/MinGW.cpp
   ToolChains/Minix.cpp
   ToolChains/MSVC.cpp
   ToolChains/Myriad.cpp
   ToolChains/NaCl.cpp
   ToolChains/NetBSD.cpp

lib/Driver/Driver.cpp

 #include "ToolChains/Darwin.h"
 #include "ToolChains/DragonFly.h"
 #include "ToolChains/FreeBSD.h"
 #include "ToolChains/Fuchsia.h"
 #include "ToolChains/Gnu.h"
 #include "ToolChains/HIP.h"
 #include "ToolChains/Haiku.h"
 #include "ToolChains/Hexagon.h"
+#include "ToolChains/Hurd.h"
 #include "ToolChains/Lanai.h"
 #include "ToolChains/Linux.h"
 #include "ToolChains/MSVC.h"
 #include "ToolChains/MinGW.h"
 #include "ToolChains/Minix.h"
 #include "ToolChains/MipsLinux.h"
 #include "ToolChains/Myriad.h"
 #include "ToolChains/NaCl.h"
DAL->AddJoinedArg(0, Opts->getOption(options::OPT_mlinker_version_EQ),
                       HOST_LINK_VERSION);
     DAL->getLastArg(options::OPT_mlinker_version_EQ)->claim();
   }
 #endif
 
   return DAL;
 }
 
+static void replaceString(std::string S,
+                          const char *Other,
+                          const char *Replace) {
+  size_t pos = S.find(Other);
+
+  if (pos == std::string::npos)
+    return;
+
+  S.replace(pos, strlen(Other), Replace);
+}
+
 /// Compute target triple from args.
 ///
 /// This routine provides the logic to compute a target triple from various
 /// args passed to the driver and the default triple string.
 static llvm::Triple computeTargetTriple(const Driver &D,
                                         StringRef TargetTriple,
                                         const ArgList &Args,
                                         StringRef DarwinArchName = "") {
   // FIXME: Already done in Compilation *Driver::BuildCompilation
   if (const Arg *A = Args.getLastArg(options::OPT_target))
     TargetTriple = A->getValue();
 
+  std::string T = TargetTriple;
+  replaceString(T, "-unknown-gnu", "-unknown-hurd-gnu");
+  replaceString(T, "-pc-gnu", "-pc-hurd-gnu");
+  TargetTriple = T;
+
   llvm::Triple Target(llvm::Triple::normalize(TargetTriple));
 
   // Handle Apple-specific options available here.
   if (Target.isOSBinFormatMachO()) {
     // If an explicit Darwin arch name is given, that trumps all.
     if (!DarwinArchName.empty()) {
       tools::darwin::setTripleTypeForMachOArchName(Target, DarwinArchName);
       return Target;
case llvm::Triple::Win32:
       }
       break;
     case llvm::Triple::PS4:
       TC = llvm::make_unique<toolchains::PS4CPU>(*this, Target, Args);
       break;
     case llvm::Triple::Contiki:
       TC = llvm::make_unique<toolchains::Contiki>(*this, Target, Args);
       break;
+    case llvm::Triple::Hurd:
+      TC = llvm::make_unique<toolchains::Hurd>(*this, Target, Args);
+      break;
     default:
       // Of these targets, Hexagon is the only one that might have
       // an OS of Linux, in which case it got handled above already.
       switch (Target.getArch()) {
       case llvm::Triple::tce:
         TC = llvm::make_unique<toolchains::TCEToolChain>(*this, Target, Args);
         break;
       case llvm::Triple::tcele:

lib/Driver/ToolChains/Clang.cpp

static bool useFramePointerForTargetByDefault(const ArgList &Args,
   default:
     break;
   }
 
   if (Triple.getOS() == llvm::Triple::NetBSD) {
     return !areOptimizationsEnabled(Args);
   }
 
-  if (Triple.isOSLinux() || Triple.getOS() == llvm::Triple::CloudABI) {
+  if (Triple.isOSLinux() || Triple.getOS() == llvm::Triple::CloudABI ||
+      Triple.isOSHurd()) {
     switch (Triple.getArch()) {
     // Don't use a frame pointer on linux if optimizing for certain targets.
     case llvm::Triple::mips64:
     case llvm::Triple::mips64el:
     case llvm::Triple::mips:
     case llvm::Triple::mipsel:
     case llvm::Triple::ppc:
     case llvm::Triple::ppc64:

lib/Driver/ToolChains/Gnu.cpp

bool Active;
        TargetTriple.getOS() != llvm::Triple::Solaris},
 
       // Deal with cases (on Ubuntu) where the system architecture could be i386
       // but the GCC target architecture could be (say) i686.
       // FIXME: It may be worthwhile to generalize this and look for a second
       // triple.
       {"i386-linux-gnu/gcc/" + CandidateTriple.str(), "../../..",
        (TargetArch == llvm::Triple::x86 &&
+        TargetTriple.getOS() != llvm::Triple::Solaris)},
+      {"i386-gnu/gcc/" + CandidateTriple.str(), "../../..",
+       (TargetArch == llvm::Triple::x86 &&
         TargetTriple.getOS() != llvm::Triple::Solaris)}};
 
   for (auto &Suffix : Suffixes) {
     if (!Suffix.Active)
       continue;
 
     StringRef LibSuffix = Suffix.LibSuffix;
     std::error_code EC;

lib/Driver/ToolChains/Hurd.h

+//===--- Hurd.h - Hurd ToolChain Implementations ----------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
+
+#include "Gnu.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY Hurd : public Generic_ELF {
+public:
+  Hurd(const Driver &D, const llvm::Triple &Triple,
+          const llvm::opt::ArgList &Args);
+
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+                            llvm::opt::ArgStringList &CC1Args) const override;
+
+  virtual std::string computeSysRoot() const;
+};
+
+} // end namespace toolchains
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H

lib/Driver/ToolChains/Hurd.cpp

+//===--- Hurd.cpp - Hurd ToolChain Implementations --------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Hurd.h"
+#include "CommonArgs.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Config/config.h"
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/Options.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang::driver;
+using namespace clang::driver::toolchains;
+using namespace clang;
+using namespace llvm::opt;
+
+using tools::addPathIfExists;
+
+/// Get our best guess at the multiarch triple for a target.
+///
+/// Debian-based systems are starting to use a multiarch setup where they use
+/// a target-triple directory in the library and header search paths.
+/// Unfortunately, this triple does not align with the vanilla target triple,
+/// so we provide a rough mapping here.
+static std::string getMultiarchTriple(const Driver &D,
+                                      const llvm::Triple &TargetTriple,
+                                      StringRef SysRoot) {
+  // For most architectures, just use whatever we have rather than trying to be
+  // clever.
+  switch (TargetTriple.getArch()) {
+  default:
+    break;
+
+  // We use the existence of '/lib/<triple>' as a directory to detect some
+  // common hurd triples that don't quite match the Clang triple for both
+  // 32-bit and 64-bit targets. Multiarch fixes its install triples to these
+  // regardless of what the actual target triple is.
+  case llvm::Triple::x86:
+    if (D.getVFS().exists(SysRoot + "/lib/i386-gnu"))
+      return "i386-gnu";
+    break;
+  }
+
+  return TargetTriple.str();
+}
+
+static StringRef getOSLibDir(const llvm::Triple &Triple, const ArgList &Args) {
+  // It happens that only x86 and PPC use the 'lib32' variant of oslibdir, and
+  // using that variant while targeting other architectures causes problems
+  // because the libraries are laid out in shared system roots that can't cope
+  // with a 'lib32' library search path being considered. So we only enable
+  // them when we know we may need it.
+  //
+  // FIXME: This is a bit of a hack. We should really unify this code for
+  // reasoning about oslibdir spellings with the lib dir spellings in the
+  // GCCInstallationDetector, but that is a more significant refactoring.
+
+  if (Triple.getArch() == llvm::Triple::x86)
+    return "lib32";
+
+  return Triple.isArch32Bit() ? "lib" : "lib64";
+}
+
+Hurd::Hurd(const Driver &D, const llvm::Triple &Triple,
+                 const ArgList &Args)
+    : Generic_ELF(D, Triple, Args) { }
+
+std::string Hurd::computeSysRoot() const {
+  if (!getDriver().SysRoot.empty())
+    return getDriver().SysRoot;
+
+  return std::string();
+}
+
+void Hurd::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
+                                     ArgStringList &CC1Args) const {
+  const Driver &D = getDriver();
+  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);
+  }
+
+  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;
+    CIncludeDirs.split(Dirs, ":");
+    for (StringRef Dir : Dirs) {
+      StringRef Prefix =
+          llvm::sys::path::is_absolute(Dir) ? StringRef(SysRoot) : "";
+      addExternCSystemInclude(DriverArgs, CC1Args, Prefix + Dir);
+    }
+    return;
+  }
+
+  // Lacking those, try to detect the correct set of system includes for the
+  // target triple.
+
+  const StringRef X86MultiarchIncludeDirs[] = {
+      "/usr/include/i386-gnu"};
+
+  ArrayRef<StringRef> MultiarchIncludeDirs;
+  switch (getTriple().getArch()) {
+  default:
+    break;
+  case llvm::Triple::x86:
+    MultiarchIncludeDirs = X86MultiarchIncludeDirs;
+    break;
+  }
+
+  for (StringRef Dir : MultiarchIncludeDirs) {
+    if (D.getVFS().exists(SysRoot + Dir)) {
+      addExternCSystemInclude(DriverArgs, CC1Args, SysRoot + Dir);
+      break;
+    }
+  }
+
+  // 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");
+}

lib/Frontend/InitHeaderSearch.cpp

if (CIncludeDirs != "") {
     CIncludeDirs.split(dirs, ":");
     for (StringRef dir : dirs)
       AddPath(dir, ExternCSystem, false);
     return;
   }
 
   switch (os) {
   case llvm::Triple::Linux:
+  case llvm::Triple::Hurd:
   case llvm::Triple::Solaris:
     llvm_unreachable("Include management is handled in the driver.");
 
   case llvm::Triple::CloudABI: {
     // <sysroot>/<triple>/include
     SmallString<128> P = StringRef(HSOpts.ResourceDir);
     llvm::sys::path::append(P, "../../..", triple.str(), "include");
     AddPath(P, System, false);