clang+lld: Improve clang+ld.darwinnew.lld interaction, pass -demangle

This patch:
- adds an ld64.lld.darwinnew symlink for lld, to go with f2710d4,
  so that `clang -fuse-ld=lld.darwinnew` can be used to test new
  Mach-O lld while it's in bring-up. (The expectation is that we'll
  remove this again once new Mach-O lld is the defauld and only Mach-O
  lld.)
- lets the clang driver know if the linker is lld (currently
  only triggered if `-fuse-ld=lld` or `-fuse-ld=lld.darwinnew` is
  passed). Currently only used for the next point, but could be used
  to implement other features that need close coordination between
  compiler and linker, e.g. having a diag for calling `clang++` instead
  of `clang` when link errors are caused by a missing C++ stdlib.
- lets the clang driver pass `-demangle` to Mach-O lld (both old and
  new), in addition to ld64
- implements -demangle for new Mach-O lld
- changes demangleItanium() to accept _Z, __Z, ___Z, ____Z prefixes
  (and updates one test added in D68014). Mach-O has an extra
  underscore for symbols, and the three (or, on Mach-O, four)
  underscores are used for block names.

Differential Revision: https://reviews.llvm.org/D91884

Author: nico

clang/include/clang/Driver/ToolChain.h

@@ -327,7 +327,11 @@ class ToolChain {
 
   /// Returns the linker path, respecting the -fuse-ld= argument to determine
   /// the linker suffix or name.
-  std::string GetLinkerPath() const;
+  /// If LinkerIsLLD is non-nullptr, it is set to true if the returned linker
+  /// is LLD. If it's set, it can be assumed that the linker is LLD built
+  /// at the same revision as clang, and clang can make assumptions about
+  /// LLD's supported flags, error output, etc.
+  std::string GetLinkerPath(bool *LinkerIsLLD = nullptr) const;
 
   /// Returns the linker path for emitting a static library.
   std::string GetStaticLibToolPath() const;

clang/lib/Driver/ToolChain.cpp

@@ -548,7 +548,10 @@ std::string ToolChain::GetProgramPath(const char *Name) const {
   return D.GetProgramPath(Name, *this);
 }
 
-std::string ToolChain::GetLinkerPath() const {
+std::string ToolChain::GetLinkerPath(bool *LinkerIsLLD) const {
+  if (LinkerIsLLD)
+    *LinkerIsLLD = false;
+
   // Get -fuse-ld= first to prevent -Wunused-command-line-argument. -fuse-ld= is
   // considered as the linker flavor, e.g. "bfd", "gold", or "lld".
   const Arg* A = Args.getLastArg(options::OPT_fuse_ld_EQ);
@@ -599,8 +602,12 @@ std::string ToolChain::GetLinkerPath() const {
     LinkerName.append(UseLinker);
 
     std::string LinkerPath(GetProgramPath(LinkerName.c_str()));
-    if (llvm::sys::fs::can_execute(LinkerPath))
+    if (llvm::sys::fs::can_execute(LinkerPath)) {
+      if (LinkerIsLLD)
+        // FIXME: Remove lld.darwinnew here once it's the only MachO lld.
+        *LinkerIsLLD = UseLinker == "lld" || UseLinker == "lld.darwinnew";
       return LinkerPath;
+    }
   }
 
   if (A)

clang/lib/Driver/ToolChains/Darwin.cpp

@@ -204,15 +204,18 @@ static bool shouldLinkerNotDedup(bool IsLinkerOnlyAction, const ArgList &Args) {
 void darwin::Linker::AddLinkArgs(Compilation &C, const ArgList &Args,
                                  ArgStringList &CmdArgs,
                                  const InputInfoList &Inputs,
-                                 unsigned Version[5]) const {
+                                 unsigned Version[5], bool LinkerIsLLD) const {
   const Driver &D = getToolChain().getDriver();
   const toolchains::MachO &MachOTC = getMachOToolChain();
 
   // Newer linkers support -demangle. Pass it if supported and not disabled by
   // the user.
-  if (Version[0] >= 100 && !Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
+  if ((Version[0] >= 100 || LinkerIsLLD) &&
+      !Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
     CmdArgs.push_back("-demangle");
 
+  // FIXME: Pass most of the flags below that check Version if LinkerIsLLD too.
+
   if (Args.hasArg(options::OPT_rdynamic) && Version[0] >= 137)
     CmdArgs.push_back("-export_dynamic");
 
@@ -533,9 +536,13 @@ void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
           << A->getAsString(Args);
   }
 
+  bool LinkerIsLLD = false;
+  const char *Exec =
+      Args.MakeArgString(getToolChain().GetLinkerPath(&LinkerIsLLD));
+
   // I'm not sure why this particular decomposition exists in gcc, but
   // we follow suite for ease of comparison.
-  AddLinkArgs(C, Args, CmdArgs, Inputs, Version);
+  AddLinkArgs(C, Args, CmdArgs, Inputs, Version, LinkerIsLLD);
 
   if (willEmitRemarks(Args) &&
       checkRemarksOptions(getToolChain().getDriver(), Args,
@@ -693,7 +700,6 @@ void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                        "-filelist"};
   }
 
-  const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
   std::unique_ptr<Command> Cmd = std::make_unique<Command>(
       JA, *this, ResponseSupport, Exec, CmdArgs, Inputs, Output);
   Cmd->setInputFileList(std::move(InputFileList));

clang/lib/Driver/ToolChains/Darwin.h

@@ -63,7 +63,8 @@ class LLVM_LIBRARY_VISIBILITY Linker : public MachOTool {
   bool NeedsTempPath(const InputInfoList &Inputs) const;
   void AddLinkArgs(Compilation &C, const llvm::opt::ArgList &Args,
                    llvm::opt::ArgStringList &CmdArgs,
-                   const InputInfoList &Inputs, unsigned Version[5]) const;
+                   const InputInfoList &Inputs, unsigned Version[5],
+                   bool LinkerIsLLD) const;
 
 public:
   Linker(const ToolChain &TC) : MachOTool("darwin::Linker", "linker", TC) {}

clang/test/Driver/darwin-ld-demangle-lld.c

@@ -0,0 +1,6 @@
+// With -fuse-ld=lld, -demangle is always passed to the linker on Darwin.
+
+// RUN: %clang -### -fuse-ld=lld %s 2>&1 | FileCheck %s
+// FIXME: Remove ld.darwinnew once it's the default (and only) mach-o lld.
+// RUN: %clang -### -fuse-ld=lld.darwinnew %s 2>&1 | FileCheck %s
+// CHECK: -demangle

lld/Common/Strings.cpp

@@ -21,12 +21,11 @@ using namespace lld;
 
 // Returns the demangled C++ symbol name for name.
 std::string lld::demangleItanium(StringRef name) {
-  // itaniumDemangle can be used to demangle strings other than symbol
-  // names which do not necessarily start with "_Z". Name can be
-  // either a C or C++ symbol. Don't call demangle if the name
-  // does not look like a C++ symbol name to avoid getting unexpected
-  // result for a C symbol that happens to match a mangled type name.
-  if (!name.startswith("_Z"))
+  // demangleItanium() can be called for all symbols. Only demangle C++ symbols,
+  // to avoid getting unexpected result for a C symbol that happens to match a
+  // mangled type name such as "Pi" (which would demangle to "int*").
+  if (!name.startswith("_Z") && !name.startswith("__Z") &&
+      !name.startswith("___Z") && !name.startswith("____Z"))
     return std::string(name);
 
   return demangle(std::string(name));

lld/MachO/Config.h

@@ -43,6 +43,7 @@ struct Configuration {
   uint32_t headerPad;
   llvm::StringRef installName;
   llvm::StringRef outputFile;
+  bool demangle = false;
   llvm::MachO::Architecture arch;
   PlatformInfo platform;
   llvm::MachO::HeaderFileType outputType;

lld/MachO/Driver.cpp

@@ -584,6 +584,7 @@ bool macho::link(llvm::ArrayRef<const char *> argsArr, bool canExitEarly,
   config->runtimePaths = args::getStrings(args, OPT_rpath);
   config->allLoad = args.hasArg(OPT_all_load);
   config->forceLoadObjC = args.hasArg(OPT_ObjC);
+  config->demangle = args.hasArg(OPT_demangle);
 
   if (const opt::Arg *arg = args.getLastArg(OPT_static, OPT_dynamic))
     config->staticLink = (arg->getOption().getID() == OPT_static);

lld/MachO/Options.td

@@ -1107,9 +1107,7 @@ def debug_snapshot : Flag<["-"], "debug_snapshot">,
      Flags<[HelpHidden]>,
      Group<grp_undocumented>;
 def demangle : Flag<["-"], "demangle">,
-     HelpText<"This option is undocumented in ld64">,
-     Flags<[HelpHidden]>,
-     Group<grp_undocumented>;
+     HelpText<"Demangle symbol names in diagnostics">;
 def dyld_env : Flag<["-"], "dyld_env">,
      HelpText<"This option is undocumented in ld64">,
      Flags<[HelpHidden]>,

lld/MachO/Symbols.cpp

@@ -33,8 +33,8 @@ void LazySymbol::fetchArchiveMember() { file->fetch(sym); }
 
 // Returns a symbol for an error message.
 std::string lld::toString(const Symbol &sym) {
-  if (Optional<std::string> s = demangleItanium(sym.getName()))
-    return *s;
+  if (config->demangle)
+    return demangleItanium(sym.getName());
   return std::string(sym.getName());
 }
 

lld/MachO/Writer.cpp

@@ -379,7 +379,7 @@ void Writer::scanRelocations() {
     for (Reloc &r : isec->relocs) {
       if (auto *s = r.referent.dyn_cast<lld::macho::Symbol *>()) {
         if (isa<Undefined>(s))
-          error("undefined symbol " + s->getName() + ", referenced from " +
+          error("undefined symbol " + toString(*s) + ", referenced from " +
                 sys::path::filename(isec->file->getName()));
         else
           target->prepareSymbolRelocation(s, isec, r);

lld/test/ELF/undef.s

@@ -27,9 +27,7 @@
 # CHECK-NEXT: >>>               {{.*}}:(.text+0x15)
 # CHECK-NEXT: >>> the vtable symbol may be undefined because the class is missing its key function (see https://lld.llvm.org/missingkeyfunction)
 
-# Check that this symbol isn't demangled
-
-# CHECK:      error: undefined symbol: __Z3fooi
+# CHECK:      error: undefined symbol: foo(int)
 # CHECK-NEXT: >>> referenced by undef.s
 # CHECK-NEXT: >>>               {{.*}}:(.text+0x1A)
 

lld/test/MachO/demangle.s

@@ -0,0 +1,15 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+
+# RUN: not %lld %t.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: not %lld -demangle %t.o -o /dev/null 2>&1 | \
+# RUN:     FileCheck --check-prefix=DEMANGLE %s
+
+# CHECK: undefined symbol __Z1fv
+# DEMANGLE: undefined symbol f()
+
+.globl _main
+_main:
+  callq __Z1fv
+  ret

lld/test/MachO/silent-ignore.test

@@ -1,5 +1,4 @@
 RUN: %lld -v \
-RUN:   -demangle \
 RUN:   -dynamic \
 RUN:   -no_deduplicate \
 RUN:   -lto_library /lib/foo \

lld/tools/lld/CMakeLists.txt

@@ -24,7 +24,8 @@ install(TARGETS lld
   RUNTIME DESTINATION bin)
 
 if(NOT LLD_SYMLINKS_TO_CREATE)
-  set(LLD_SYMLINKS_TO_CREATE lld-link ld.lld ld64.lld wasm-ld)
+  set(LLD_SYMLINKS_TO_CREATE
+      lld-link ld.lld ld64.lld ld64.darwinnew.lld wasm-ld)
 endif()
 
 foreach(link ${LLD_SYMLINKS_TO_CREATE})

llvm/utils/gn/secondary/lld/tools/lld/BUILD.gn

@@ -4,6 +4,7 @@ symlinks = [
   "lld-link",
   "ld.lld",
   "ld64.lld",
+  "ld64.darwinnew.lld",
   "wasm-ld",
 ]
 foreach(target, symlinks) {