lld-link: Only print demangled symbol names by default

[thakis]

Summary

This makes lld-link's output a bit more concise. Since most developers can't read mangled names, this should make the output a bit easier to understand as well. It also makes lld-link's output consistent with ld.lld's output.

(link.exe prints both demangled and mangled names; lld-link used to match link.exe output but now no longer does.)

For people working on toolchains, add a /demangle:no flag that makes lld-link print the mangled name instead of the demangled name. (If desired, people could pipe that through demumble -b to get the old behavior of both demangled and mangled output.)

[ruiu]

It looks like always showing both mangled and demangled names makes the output a bit too large. I'm a little skeptical how useful it is. Where did you find it useful?

[smeenai]

In D58132#1395055, @ruiu wrote:

It looks like always showing both mangled and demangled names makes the output a bit too large. I'm a little skeptical how useful it is. Where did you find it useful?

I'm in favor of at least showing the mangled name. Part of the issue is that multiple symbols can have the same demangling (as mentioned in this commit's message), and the mangled name is also useful when you're trying to examine object files for references or definitions, etc.

[thakis]

In D58132#1395055, @ruiu wrote:

It looks like always showing both mangled and demangled names makes the output a bit too large. I'm a little skeptical how useful it is. Where did you find it useful?

I found the error message in comment 0 of https://bugs.chromium.org/p/chromium/issues/detail?id=930883 a bit difficult to read and wished I had the mangled symbol too. I'm used to the lld/coff output, I suppose. (It'd be nice if they were consistent in their output too.) The mangled name is usually shorter than the demangled one, so maybe it's not so bad. I agree output length is a concern though. I figured it's early in 9.0, we could land this and see if we like it, and if we don't, then remove it again (in both elf and coff). But in the end it's your call.

[pcc]

My personal opinion: 99% of the time I reckon users will want to see the demangled name (it's only us toolchain folks who might more often prefer the mangled names), and there's already a flag --no-demangle which can be used to suppress demangling if necessary. So I think I'd prefer making the COFF linker more like the ELF linker (i.e. don't show mangled names without a flag).

[smeenai]

In D58132#1396724, @pcc wrote:

My personal opinion: 99% of the time I reckon users will want to see the demangled name (it's only us toolchain folks who might more often prefer the mangled names), and there's already a flag --no-demangle which can be used to suppress demangling if necessary. So I think I'd prefer making the COFF linker more like the ELF linker (i.e. don't show mangled names without a flag).

I'd be happy with this (just demangled names being the default, as long as there's some way to either switch to or display mangled names in addition).

[thakis]

In D58132#1396724, @pcc wrote:

My personal opinion: 99% of the time I reckon users will want to see the demangled name (it's only us toolchain folks who might more often prefer the mangled names), and there's already a flag --no-demangle which can be used to suppress demangling if necessary. So I think I'd prefer making the COFF linker more like the ELF linker (i.e. don't show mangled names without a flag).

Makes sense.

I kind of like the "demangled" (mangled) output, but I suppose I can just make that a feature in demumble.

Does lld-link have a --no-demangle flag already? If not, any opinions on how it should be spelled?

[ruiu]

Boolean flags usually have the form /foo[:no] in MSVC linker, so I'd propose /demangle and /demangle:no for lld/coff.

[thakis]

Change lld-link instead.

[ruiu]

LGTM

lld/COFF/Symbols.cpp

19

Since this file is under COFF directory, it is more consistent to add using namespace lld::coff here instead of adding coff:: to Config.

lld/COFF/Config.h

struct Configuration {
   llvm::COFF::MachineTypes Machine = IMAGE_FILE_MACHINE_UNKNOWN;
   size_t Wordsize;
   bool Verbose = false;
   WindowsSubsystem Subsystem = llvm::COFF::IMAGE_SUBSYSTEM_UNKNOWN;
   Symbol *Entry = nullptr;
   bool NoEntry = false;
   std::string OutputFile;
   std::string ImportName;
+  bool Demangle = true;
   bool DoGC = true;
   bool DoICF = true;
   bool TailMerge;
   bool Relocatable = true;
   bool ForceMultiple = false;
   bool ForceUnresolved = false;
   bool Debug = false;
   bool DebugDwarf = false;

lld/COFF/Driver.cpp

void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   // Handle /debug
   DebugKind Debug = parseDebugKind(Args);
   if (Debug == DebugKind::Full || Debug == DebugKind::Dwarf ||
       Debug == DebugKind::GHash) {
     Config->Debug = true;
     Config->Incremental = true;
   }
 
+  // Handle /demangle
+  Config->Demangle = Args.hasFlag(OPT_demangle, OPT_demangle_no);
+
   // Handle /debugtype
   Config->DebugTypes = parseDebugTypes(Args);
 
   // Handle /pdb
   bool ShouldCreatePDB =
       (Debug == DebugKind::Full || Debug == DebugKind::GHash);
   if (ShouldCreatePDB) {
     if (auto *Arg = Args.getLastArg(OPT_pdb))

lld/COFF/Options.td

defm tsaware  : B<"tsaware",
                   "Create non-Terminal Server aware executable">;
 
 def help : F<"help">;
 def help_q : Flag<["/?", "-?"], "">, Alias<help>;
 
 // LLD extensions
 def exclude_all_symbols : F<"exclude-all-symbols">;
 def export_all_symbols : F<"export-all-symbols">;
+defm demangle : B<"demangle",
+    "Demangle symbols in output (default)",
+    "Do not demangle symbols in output">;
 def kill_at : F<"kill-at">;
 def lldmingw : F<"lldmingw">;
 def output_def : Joined<["/", "-"], "output-def:">;
 def pdb_source_path : P<"pdbsourcepath",
                         "Base path used to make relative source file path absolute in PDB">;
 def rsp_quoting : Joined<["--"], "rsp-quoting=">,
   HelpText<"Quoting style for response files, 'windows' (default) or 'posix'">;
 def dash_dash_version : Flag<["--"], "version">,
   HelpText<"Print version information">;
 defm threads: B<"threads",
     "Run the linker multi-threaded (default)",
     "Do not run the linker multi-threaded">;
 
 // Flags for debugging
 def lldmap : F<"lldmap">;
 def lldmap_file : Joined<["/", "-"], "lldmap:">;
 def show_timing : F<"time">;
 
 //==============================================================================
 // The flags below do nothing. They are defined only for link.exe compatibility.
 //==============================================================================
 
 class QF<string name> : Joined<["/", "-", "-?"], name#":">;
 
-multiclass QB<string name> {
-  def "" : F<name>;
-  def _no : F<name#":no">;
-}
-
 def ignoreidl : F<"ignoreidl">;
 def nologo : F<"nologo">;
 def throwingnew : F<"throwingnew">;
 def editandcontinue : F<"editandcontinue">;
 def fastfail : F<"fastfail">;
 
 def delay : QF<"delay">;
 def errorreport : QF<"errorreport">;
 def idlout : QF<"idlout">;
 def maxilksize : QF<"maxilksize">;
 def tlbid : QF<"tlbid">;
 def tlbout : QF<"tlbout">;
 def verbose_all : QF<"verbose">;
 def guardsym : QF<"guardsym">;

lld/COFF/Symbols.cpp

 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Memory.h"
 #include "lld/Common/Strings.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 using namespace llvm::object;
 
 // Returns a symbol name for an error message.
 std::string lld::toString(coff::Symbol &B) {
+  if (coff::Config->Demangle)
     if (Optional<std::string> S = lld::demangleMSVC(B.getName()))
-    return ("\"" + *S + "\" (" + B.getName() + ")").str();
+      return *S;
   return B.getName();
 }
 
 namespace lld {
 namespace coff {
 
 StringRef Symbol::getName() {
   // COFF symbol names are read lazily for a performance reason.

lld/test/COFF/undefined-symbol-cv.s

 # REQUIRES: x86
 # RUN: llvm-mc -triple=x86_64-windows-msvc -filetype=obj -o %t.obj %s
 # RUN: not lld-link /out:%t.exe %t.obj 2>&1 | FileCheck %s
 
-# CHECK: error: undefined symbol: "int __cdecl foo(void)" (?foo@@YAHXZ)
+# CHECK: error: undefined symbol: int __cdecl foo(void)
 # CHECK-NEXT: >>> referenced by file1.cpp:1
 # CHECK-NEXT: >>>               {{.*}}.obj:(main)
 # CHECK-NEXT: >>> referenced by file1.cpp:2
 # CHECK-NEXT: >>>               {{.*}}.obj:(main)
 # CHECK-EMPTY:
-# CHECK-NEXT: error: undefined symbol: "int __cdecl bar(void)" (?bar@@YAHXZ)
+# CHECK-NEXT: error: undefined symbol: int __cdecl bar(void)
 # CHECK-NEXT: >>> referenced by file2.cpp:3
 # CHECK-NEXT: >>>               {{.*}}.obj:(main)
 # CHECK-NEXT: >>> referenced by file1.cpp:4
 # CHECK-NEXT: >>>               {{.*}}.obj:(f1)
 # CHECK-EMPTY:
-# CHECK-NEXT: error: undefined symbol: "int __cdecl baz(void)" (?baz@@YAHXZ)
+# CHECK-NEXT: error: undefined symbol: int __cdecl baz(void)
 # CHECK-NEXT: >>> referenced by file1.cpp:5
 # CHECK-NEXT: >>>               {{.*}}.obj:(f2)
 
   .cv_file  1 "file1.cpp" "EDA15C78BB573E49E685D8549286F33C" 1
   .cv_file  2 "file2.cpp" "EDA15C78BB573E49E685D8549286F33D" 1
 
         .section        .text,"xr",one_only,main
 .globl main

lld/test/COFF/undefined-symbol.s

 # REQUIRES: x86
 # RUN: llvm-mc -triple=x86_64-windows-msvc -filetype=obj -o %t.obj %s
 # RUN: not lld-link /out:%t.exe %t.obj 2>&1 | FileCheck %s
+# RUN: not lld-link /out:%t.exe /demangle %t.obj 2>&1 | FileCheck %s
+# RUN: not lld-link /out:%t.exe /demangle:no %t.obj 2>&1 | FileCheck --check-prefix=NODEMANGLE %s
 
-# CHECK: error: undefined symbol: "int __cdecl foo(void)" (?foo@@YAHXZ)
+# NODEMANGLE: error: undefined symbol: ?foo@@YAHXZ
+# NODEMANGLE: error: undefined symbol: ?bar@@YAHXZ
+# NODEMANGLE: error: undefined symbol: __imp_?baz@@YAHXZ
+
+# CHECK: error: undefined symbol: int __cdecl foo(void)
 # CHECK-NEXT: >>> referenced by {{.*}}.obj:(main)
 # CHECK-NEXT: >>> referenced by {{.*}}.obj:(main)
 # CHECK-EMPTY:
-# CHECK-NEXT: error: undefined symbol: "int __cdecl bar(void)" (?bar@@YAHXZ)
+# CHECK-NEXT: error: undefined symbol: int __cdecl bar(void)
 # CHECK-NEXT: >>> referenced by {{.*}}.obj:(main)
 # CHECK-NEXT: >>> referenced by {{.*}}.obj:(f1)
 # CHECK-EMPTY:
-# CHECK-NEXT: error: undefined symbol: "__declspec(dllimport) int __cdecl baz(void)" (__imp_?baz@@YAHXZ)
+# CHECK-NEXT: error: undefined symbol: __declspec(dllimport) int __cdecl baz(void)
 # CHECK-NEXT: >>> referenced by {{.*}}.obj:(f2)
 
         .section        .text,"xr",one_only,main
 .globl main
 main:
   call  "?foo@@YAHXZ"
   call  "?foo@@YAHXZ"
   call  "?bar@@YAHXZ"