demangle undefined symbol names in error message

[kledzik]

Summary

The darwin linker has the -demangle option which directs it to demangle C++ (and soon Swift) mangled symbol names. Long term we need some Diagnostics object for formatting errors and warnings. But for now we have the Core linker just writing messages to llvm::errs(). So, to enable demangling, I changed the Resolver to call a LinkingContext method on the symbol name.

To make this more interesting, the demangling code is done via __cxa_demangle() which is part of the C++ ABI, which is only supported on some platforms, so I had to conditionalize the code with the config generated HAVE_CXXABI_H.

[ruiu]

LGTM with nits

include/lld/Core/LinkingContext.h

274

I'd name demangle rather than demangleSymbol because it's shorter and yet conveys enough information.

include/lld/ReaderWriter/MachOLinkingContext.h

100

And rename doDemangle, enableDemangle or something like that.

[t.p.northover]

Hi Nick,

The code looks reasonable to me. One comment on the tests though...

Cheers.

Tim.

test/mach-o/demangle.yaml

33–34

Any chance you could add 1 or 2 weird symbols here too (non-C++ and perhaps one without any underscore)? Since this is used for diagnostics when things have gone wrong, it's fairly important it won't crash and burn then.

[kledzik]

committed in r218718

include/lld/Core/LinkingContext.h

   /// \returns true if there is an error with the current settings.
   bool validate(raw_ostream &diagnostics);
 
+  /// Formats symbol name for use in error messages.
+  virtual std::string demangleSymbol(StringRef symbolName) const {
+    return symbolName;
+  }
+
   /// @}
   /// \name Methods used by Driver::link()
   /// @{

include/lld/ReaderWriter/MachOLinkingContext.h

 
   void addPasses(PassManager &pm) override;
   bool validateImpl(raw_ostream &diagnostics) override;
+  std::string demangleSymbol(StringRef symbolName) const override;
+
   bool createImplicitFiles(std::vector<std::unique_ptr<File>> &) const override;
 
   uint32_t getCPUType() const;

 
   bool keepPrivateExterns() const { return _keepPrivateExterns; }
   void setKeepPrivateExterns(bool v) { _keepPrivateExterns = v; }
+  bool demangle() const { return _demangle; }
+  void setDemangle(bool d) { _demangle = d; }
+
 
   bool minOS(StringRef mac, StringRef iOS) const;
   void setDoNothing(bool value) { _doNothing = value; }

   bool _printAtoms;
   bool _testingFileUsage;
   bool _keepPrivateExterns;
+  bool _demangle;
   StringRef _bundleLoader;
   mutable std::unique_ptr<mach_o::ArchHandler> _archHandler;
   mutable std::unique_ptr<Writer> _writer;

lib/Core/Resolver.cpp

       foundUndefines = true;
       if (_context.printRemainingUndefines()) {
         llvm::errs() << "Undefined symbol: " << undefAtom->file().path()
-                     << ": " << undefAtom->name() << "\n";
+                     << ": " << _context.demangleSymbol(undefAtom->name())
+                     << "\n";
       }
     }
     if (foundUndefines) {

lib/Driver/DarwinLdDriver.cpp

   if (parsedArgs->getLastArg(OPT_t))
     ctx.setLogInputFiles(true);
 
+  // Handle -demangle option.
+  if (parsedArgs->getLastArg(OPT_demangle))
+    ctx.setDemangle(true);
+
   // Handle -keep_private_externs
   if (parsedArgs->getLastArg(OPT_keep_private_externs)) {
     ctx.setKeepPrivateExterns(true);

lib/Driver/DarwinLdOptions.td

      HelpText<"alignment for segment/section">;
 def image_base : Separate<["-"], "image_base">;
 def seg1addr : Separate<["-"], "seg1addr">, Alias<image_base>;
+def demangle : Flag<["-"], "demangle">,
+     HelpText<"Demangles symbol names in errors and warnings">;
 
 def t : Flag<["-"], "t">,
      HelpText<"Print the names of the input files as ld processes them">;

lib/ReaderWriter/MachO/MachOLinkingContext.cpp

 
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Triple.h"
+#include "llvm/Config/config.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MachO.h"

 
 #include <algorithm>
 
+#if HAVE_CXXABI_H
+#include <cxxabi.h>
+#endif
+
 using lld::mach_o::ArchHandler;
 using lld::mach_o::MachODylibFile;
 using namespace llvm::MachO;

       _osMinVersion(0), _pageZeroSize(0), _pageSize(4096), _baseAddress(0),
       _compatibilityVersion(0), _currentVersion(0), _deadStrippableDylib(false),
       _printAtoms(false), _testingFileUsage(false), _keepPrivateExterns(false),
-      _archHandler(nullptr), _exportMode(ExportMode::globals) {}
+      _demangle(false), _archHandler(nullptr),
+      _exportMode(ExportMode::globals) {}
 
 MachOLinkingContext::~MachOLinkingContext() {}
 

   llvm_unreachable("_exportMode unknown enum value");
 }
 
+std::string MachOLinkingContext::demangleSymbol(StringRef symbolName) const {
+  // Only try to demangle symbols if -demangle on command line
+  if (!_demangle)
+    return symbolName;
+
+  // Only try to demangle symbols that look like C++ symbols
+  if (!symbolName.startswith("__Z"))
+    return symbolName;
+
+#if HAVE_CXXABI_H
+  SmallString<256> symBuff;
+  StringRef nullTermSym = Twine(symbolName).toNullTerminatedStringRef(symBuff);
+  // Mach-O has extra leading underscore that needs to be removed.
+  const char *cstr = nullTermSym.data() + 1;
+  int status;
+  char *demangled = abi::__cxa_demangle(cstr, nullptr, nullptr, &status);
+  if (demangled != NULL) {
+    std::string result(demangled);
+    // __cxa_demangle() always uses a malloc'ed buffer to return the result.
+    free(demangled);
+    return result;
+  }
+#endif
+
+  return symbolName;
+}
+
 
 } // end namespace lld

test/mach-o/demangle.yaml

+# REQUIRES: system-linker-mach-o
+#
+# RUN: not lld -flavor darwin -arch x86_64 -macosx_version_min 10.8 %s  \
+# RUN:     -dylib -o %t %p/Inputs/libSystem.yaml  2> %t.err
+# RUN: FileCheck %s < %t.err
+#
+# RUN: not lld -flavor darwin -arch x86_64 -macosx_version_min 10.8 %s \
+# RUN:     -dylib -o %t %p/Inputs/libSystem.yaml -demangle 2> %t.err2
+# RUN: FileCheck %s --check-prefix=DCHECK < %t.err2
+#
+# Test -demangle option works on undefined symbol errors.
+#
+
+--- !mach-o
+arch:            x86_64
+file-type:       MH_OBJECT
+flags:           [ MH_SUBSECTIONS_VIA_SYMBOLS ]
+sections:
+  - segment:         __TEXT
+    section:         __text
+    type:            S_REGULAR
+    attributes:      [ S_ATTR_PURE_INSTRUCTIONS, S_ATTR_SOME_INSTRUCTIONS ]
+    address:         0x0000000000000000
+    content:         [ 0x55, 0x48, 0x89, 0xE5, 0xBF, 0x64, 0x00, 0x00,
+                       0x00, 0x5D, 0xE9, 0x00, 0x00, 0x00, 0x00 ]
+    relocations:
+      - offset:          0x0000000B
+        type:            X86_64_RELOC_BRANCH
+        length:          2
+        pc-rel:          true
+        extern:          true
+        symbol:          1
+global-symbols:
+  - name:            __Z1xv
+    type:            N_SECT
+    scope:           [ N_EXT ]
+    sect:            1
+    value:           0x0000000000000000
+undefined-symbols:
+  - name:            __Znam
+    type:            N_UNDF
+    scope:           [ N_EXT ]
+    value:           0x0000000000000000
+
+...
+
+# CHECK:  __Znam
+# DCHECK: operator new[](unsigned long)