[Clang] Fix Hurd toolchain class hierarchy

[aganea]

Summary

As noted in PR45061, a two-stage ThinLTO build fails the clang/test/Driver/hurd.c test because of a static_cast here which wrongly converts an instance of clang::driver::toolchains::Hurd into that of clang::driver::toolchains::Linux. ThinLTO will later devirtualize the ToolChain.getDynamicLinker(Args); call and use Linux::getDynamicLinker() instead, causing the test to generate a wrong -dynamic-linker linker flag (/lib/ld-linux.so.2 instead of /lib/ld.so)

Ideally, and because of the static_cast mentioned above, maybe gnutools::Linker should take a const Generic_ELF & instance, not a const ToolChain &. But that causes other issues and could be done later.

[aganea]

@hans, do you think this could be included in 10.0?

[aganea]

Fix clang-formatting.

[aganea]

The patch did not make sense conceptually. Hurd is not Linux. I think now it makes more sense.

[hans]

In D75373#1898620, @aganea wrote:

@hans, do you think this could be included in 10.0?

I think it's too late in the process for this, maybe it can be merged for 10.0.1.

[kristina]

In D75373#1900237, @aganea wrote:

The patch did not make sense conceptually. Hurd is not Linux. I think now it makes more sense.

Yes this seems to make more sense now. LGTM.

clang/lib/Driver/ToolChains/Gnu.h

 public:
   Generic_ELF(const Driver &D, const llvm::Triple &Triple,
               const llvm::opt::ArgList &Args)
       : Generic_GCC(D, Triple, Args) {}
 
   void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
                              llvm::opt::ArgStringList &CC1Args,
                              Action::OffloadKind DeviceOffloadKind) const override;
+
+  virtual std::string getDynamicLinker(const llvm::opt::ArgList &Args) const {
+    return {};
+  }
+
+  virtual void addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const {}
 };
 
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_GNU_H

clang/lib/Driver/ToolChains/Gnu.cpp

case llvm::Triple::x86_64:
     if (T.getEnvironment() == llvm::Triple::GNUX32)
       return "elf32_x86_64";
     return "elf_x86_64";
   default:
     return nullptr;
   }
 }
 
-static bool getPIE(const ArgList &Args, const toolchains::Linux &ToolChain) {
+static bool getPIE(const ArgList &Args, const ToolChain &TC) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
       Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
     return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
                            options::OPT_nopie);
   if (!A)
-    return ToolChain.isPIEDefault();
+    return TC.isPIEDefault();
   return A->getOption().matches(options::OPT_pie);
 }
 
-static bool getStaticPIE(const ArgList &Args,
-                         const toolchains::Linux &ToolChain) {
+static bool getStaticPIE(const ArgList &Args, const ToolChain &TC) {
   bool HasStaticPIE = Args.hasArg(options::OPT_static_pie);
   // -no-pie is an alias for -nopie. So, handling -nopie takes care of
   // -no-pie as well.
   if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
-    const Driver &D = ToolChain.getDriver();
+    const Driver &D = TC.getDriver();
     const llvm::opt::OptTable &Opts = D.getOpts();
     const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
     const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
     D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
   }
   return HasStaticPIE;
 }
 
 static bool getStatic(const ArgList &Args) {
   return Args.hasArg(options::OPT_static) &&
       !Args.hasArg(options::OPT_static_pie);
 }
 
 void tools::gnutools::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                            const InputInfo &Output,
                                            const InputInfoList &Inputs,
                                            const ArgList &Args,
                                            const char *LinkingOutput) const {
-  const toolchains::Linux &ToolChain =
-      static_cast<const toolchains::Linux &>(getToolChain());
+  // FIXME: The Linker class constructor takes a ToolChain and not a
+  // Generic_ELF, so the static_cast might return a reference to a invalid
+  // instance (see PR45061). Ideally, the Linker constructor needs to take a
+  // Generic_ELF instead.
+  const toolchains::Generic_ELF &ToolChain =
+      static_cast<const toolchains::Generic_ELF &>(getToolChain());
   const Driver &D = ToolChain.getDriver();
 
   const llvm::Triple &Triple = getToolChain().getEffectiveTriple();
 
   const llvm::Triple::ArchType Arch = ToolChain.getArch();
   const bool isAndroid = ToolChain.getTriple().isAndroid();
   const bool IsIAMCU = ToolChain.getTriple().isOSIAMCU();
   const bool IsPIE = getPIE(Args, ToolChain);
if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
       CmdArgs.push_back("--fix-cortex-a53-843419");
   }
 
   // Android does not allow shared text relocations. Emit a warning if the
   // user's code contains any.
   if (isAndroid)
       CmdArgs.push_back("--warn-shared-textrel");
 
-  for (const auto &Opt : ToolChain.ExtraOpts)
-    CmdArgs.push_back(Opt.c_str());
+  ToolChain.addExtraOpts(CmdArgs);
 
   CmdArgs.push_back("--eh-frame-hdr");
 
   if (const char *LDMOption = getLDMOption(ToolChain.getTriple(), Args)) {
     CmdArgs.push_back("-m");
     CmdArgs.push_back(LDMOption);
   } else {
     D.Diag(diag::err_target_unknown_triple) << Triple.str();

clang/lib/Driver/ToolChains/Hurd.h

Hurd(const Driver &D, const llvm::Triple &Triple,
        const llvm::opt::ArgList &Args);
 
   bool HasNativeLLVMSupport() const override;
 
   void
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
                             llvm::opt::ArgStringList &CC1Args) const override;
 
-  virtual std::string computeSysRoot() const;
+  std::string computeSysRoot() const;
 
-  virtual std::string getDynamicLinker(const llvm::opt::ArgList &Args) const;
+  std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
+
+  void addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const override;
 
   std::vector<std::string> ExtraOpts;
 
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
 
   std::string getMultiarchTriple(const Driver &D,

clang/lib/Driver/ToolChains/Hurd.cpp

static StringRef getOSLibDir(const llvm::Triple &Triple, const ArgList &Args) {
   // 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)
+Hurd::Hurd(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
     : Generic_ELF(D, Triple, Args) {
   std::string SysRoot = computeSysRoot();
   path_list &Paths = getFilePaths();
 
   const std::string OSLibDir = std::string(getOSLibDir(Triple, Args));
   const std::string MultiarchTriple = getMultiarchTriple(D, Triple, SysRoot);
 
 #ifdef ENABLE_LINKER_BUILD_ID
void Hurd::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
 
   // 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");
 }
+
+void Hurd::addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const {
+  for (const auto &Opt : ExtraOpts)
+    CmdArgs.push_back(Opt.c_str());
+}

clang/lib/Driver/ToolChains/Linux.h

public:
   bool isPIEDefault() const override;
   bool isNoExecStackDefault() const override;
   bool IsMathErrnoDefault() const override;
   SanitizerMask getSupportedSanitizers() const override;
   void addProfileRTLibs(const llvm::opt::ArgList &Args,
                         llvm::opt::ArgStringList &CmdArgs) const override;
   virtual std::string computeSysRoot() const;
 
-  virtual std::string getDynamicLinker(const llvm::opt::ArgList &Args) const;
+  std::string getDynamicLinker(const llvm::opt::ArgList &Args) const override;
+
+  void addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const override;
 
   std::vector<std::string> ExtraOpts;
 
   llvm::DenormalMode getDefaultDenormalModeForType(
     const llvm::opt::ArgList &DriverArgs,
     Action::OffloadKind DeviceOffloadKind,
     const llvm::fltSemantics *FPType = nullptr) const override;
 

clang/lib/Driver/ToolChains/Linux.cpp

if (!DriverArgs.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles) &&
         isFastMathRuntimeAvailable(DriverArgs, Unused))
       return llvm::DenormalMode::getPreserveSign();
     return llvm::DenormalMode::getIEEE();
   }
   default:
     return llvm::DenormalMode::getIEEE();
   }
 }
+
+void Linux::addExtraOpts(llvm::opt::ArgStringList &CmdArgs) const {
+  for (const auto &Opt : ExtraOpts)
+    CmdArgs.push_back(Opt.c_str());
+}