[lld/mac] With -demangle, strip leading _ from non-mangled names

[thakis]

Summary

For

void f();
int main() { f(); }

lld -demangle now produces

ld64.lld: error: undefined symbol: f
>>> referenced by path/to/main.o:(symbol main+0x8)

instead of

ld64.lld: error: undefined symbol: _f
>>> referenced by path/to/main.o:(symbol _main+0x8)

previously. (Without -demangle, it still prints _f and _main.)

This does *not* match ld64's behavior, but it does match e.g. lld/COFF's
behaviour.

This is arguably easier to understand: clang prepends symbol names with _
on macOS, so it seems friendly if the linker removes it again in its
diagnostics. It also makes the extern "C" insertion diagnostics we added
recently look more self-consistent.

---

I'm not 100% sure this is a good idea, but it might be. Maybe best to land
it for a while and see how it feels?

[int3]

seems reasonable :)

[oontvoo]

LG (also seems harmless! :) )

lld/MachO/Symbols.cpp

 #if !defined(_MSC_VER)
 static_assert(sizeof(void *) != 8 || sizeof(Defined) == 88,
               "Try to minimize Defined's size; we create many instances");
 #endif
 
 static_assert(sizeof(SymbolUnion) == sizeof(Defined),
               "Defined should be the largest Symbol kind");
 
+// Returns a symbol name for an error message.
+static std::string maybeDemangleSymbol(StringRef symName) {
+  if (config->demangle) {
+    symName.consume_front("_");
+    return demangle(symName, true);
+  }
+  return std::string(symName);
+}
+
 std::string lld::toString(const Symbol &sym) {
-  return demangle(sym.getName(), config->demangle);
+  return maybeDemangleSymbol(sym.getName());
 }
 
 std::string lld::toMachOString(const object::Archive::Symbol &b) {
-  return demangle(b.getName(), config->demangle);
+  return maybeDemangleSymbol(b.getName());
 }
 
 uint64_t Symbol::getStubVA() const { return in.stubs->getVA(stubsIndex); }
 uint64_t Symbol::getLazyPtrVA() const {
   return in.lazyPointers->getVA(stubsIndex);
 }
 uint64_t Symbol::getGotVA() const { return in.got->getVA(gotIndex); }
 uint64_t Symbol::getTlvVA() const { return in.tlvPointers->getVA(gotIndex); }

lld/test/MachO/invalid/duplicate-symbol.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t-dup.o
-# RUN: not %lld -dylib -o /dev/null %t-dup.o %t.o 2>&1 | FileCheck %s -DNAME=_ZN1a1bL3fooE -DFILE_1=%t-dup.o -DFILE_2=%t.o
-# RUN: not %lld -dylib -o /dev/null %t.o %t.o 2>&1 | FileCheck %s -DNAME=_ZN1a1bL3fooE -DFILE_1=%t.o -DFILE_2=%t.o
+# RUN: not %lld -dylib -o /dev/null %t-dup.o %t.o 2>&1 | FileCheck %s -DNAME=__ZN1a1bL3fooE -DFILE_1=%t-dup.o -DFILE_2=%t.o
+# RUN: not %lld -dylib -o /dev/null %t.o %t.o 2>&1 | FileCheck %s -DNAME=__ZN1a1bL3fooE -DFILE_1=%t.o -DFILE_2=%t.o
 
 # RUN: not %lld -dylib -demangle -o /dev/null %t-dup.o %t.o 2>&1 | FileCheck %s -DNAME="a::b::foo" -DFILE_1=%t-dup.o -DFILE_2=%t.o
 # RUN: not %lld -dylib -demangle -o /dev/null %t.o %t.o 2>&1 | FileCheck %s -DNAME="a::b::foo" -DFILE_1=%t.o -DFILE_2=%t.o
 
 # CHECK:      error: duplicate symbol: [[NAME]]
 # CHECK-NEXT: >>> defined in [[FILE_1]]
 # CHECK-NEXT: >>> defined in [[FILE_2]]
 
 .text
-.global _ZN1a1bL3fooE
-_ZN1a1bL3fooE:
+.global __ZN1a1bL3fooE
+__ZN1a1bL3fooE:
   ret

lld/test/MachO/invalid/undefined-symbol.s

 # REQUIRES: x86
 # RUN: rm -rf %t; split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/main.s -o %t/main.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/foo.s -o %t/foo.o
 # RUN: llvm-ar crs %t/foo.a %t/foo.o
 # RUN: not %lld --icf=all -o /dev/null %t/main.o 2>&1 | \
 # RUN:     FileCheck %s -DSYM=__ZN3foo3barEi -DLOC='%t/main.o:(symbol _main+0x1)'
 # RUN: not %lld -demangle --icf=all -o /dev/null %t/main.o 2>&1 | \
-# RUN:     FileCheck %s -DSYM='foo::bar(int)' -DLOC='%t/main.o:(symbol _main+0x1)'
+# RUN:     FileCheck %s -DSYM='foo::bar(int)' -DLOC='%t/main.o:(symbol main+0x1)'
 # RUN: not %lld -o /dev/null %t/main.o %t/foo.a 2>&1 | \
 # RUN:     FileCheck %s -DSYM=_bar -DLOC='%t/foo.a(foo.o):(symbol __ZN3foo3barEi+0x1)'
 # RUN: not %lld -demangle -o /dev/null %t/main.o %t/foo.a 2>&1 | \
-# RUN:     FileCheck %s -DSYM=_bar -DLOC='%t/foo.a(foo.o):(symbol foo::bar(int)+0x1)'
+# RUN:     FileCheck %s -DSYM=bar -DLOC='%t/foo.a(foo.o):(symbol foo::bar(int)+0x1)'
 # RUN: not %lld -o /dev/null %t/main.o -force_load %t/foo.a 2>&1 | \
 # RUN:     FileCheck %s -DSYM=_bar -DLOC='%t/foo.a(foo.o):(symbol __ZN3foo3barEi+0x1)'
 # RUN: not %lld -demangle -o /dev/null %t/main.o -force_load %t/foo.a 2>&1 | \
-# RUN:     FileCheck %s -DSYM=_bar -DLOC='%t/foo.a(foo.o):(symbol foo::bar(int)+0x1)'
+# RUN:     FileCheck %s -DSYM=bar -DLOC='%t/foo.a(foo.o):(symbol foo::bar(int)+0x1)'
 # CHECK: error: undefined symbol: [[SYM]]
 # CHECK-NEXT: >>> referenced by [[LOC]]
 
 #--- foo.s
 .globl __ZN3foo3barEi
 .text
 __ZN3foo3barEi:
   callq _bar

lld/test/MachO/undef-suggest-extern-c.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos %s -o %t.o
 
 ## The reference is mangled while the definition is not, suggest a missing
 ## extern "C".
 # RUN: echo 'call __Z3fooi' | llvm-mc -filetype=obj -triple=x86_64-apple-macos - -o %t1.o
 # RUN: not %lld %t.o %t1.o -demangle -o /dev/null 2>&1 | FileCheck %s
 
 # CHECK:      error: undefined symbol: foo(int)
 # CHECK-NEXT: >>> referenced by {{.*}}
-# CHECK-NEXT: >>> did you mean: extern "C" _foo
+# CHECK-NEXT: >>> did you mean: extern "C" foo
 
 ## Don't suggest for nested names like F::foo() and foo::foo().
 # RUN: echo 'call __ZN1F3fooEv; call __ZN3fooC1Ev' | llvm-mc -filetype=obj -triple=x86_64-apple-macos - -o %t2.o
 # RUN: not ld.lld %t.o %t2.o -o /dev/null 2>&1 | FileCheck /dev/null --implicit-check-not='did you mean'
 
 .globl _start, _foo
 _start:
 _foo:

lld/test/MachO/undef-suggest-extern-c2.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos %s -o %t.o
 
 ## The definition is mangled while the reference is not, suggest an arbitrary
 ## C++ overload.
 # RUN: echo '.globl __Z3fooi; __Z3fooi:' | llvm-mc -filetype=obj -triple=x86_64-apple-macos - -o %t1.o
 # RUN: not %lld %t.o %t1.o -demangle -o /dev/null 2>&1 | FileCheck %s
 
 ## Check that we can suggest a local definition.
 # RUN: echo '__Z3fooi: call _foo' | llvm-mc -filetype=obj -triple=x86_64-apple-macos - -o %t2.o
 # RUN: not %lld %t2.o -demangle -o /dev/null 2>&1 | FileCheck %s
 
-# CHECK:      error: undefined symbol: _foo
+# CHECK:      error: undefined symbol: foo
 # CHECK-NEXT: >>> referenced by {{.*}}
 # CHECK-NEXT: >>> did you mean to declare foo(int) as extern "C"?
 
 ## Don't suggest nested names whose base name is "foo", e.g. F::foo().
 # RUN: echo '.globl __ZN1F3fooEv; __ZN1F3fooEv:' | llvm-mc -filetype=obj -triple=x86_64-apple-macos - -o %t3.o
 # RUN: not %lld %t.o %t3.o -o /dev/null 2>&1 | FileCheck /dev/null --implicit-check-not='did you mean'
 
 call _foo