[LLD] [COFF] Omit automatically imported symbols from the symbol table

[mstorsjo]

Summary

For these symbols, they actually point to the symbol's IAT entry instead, which obviously is different from the symbol itself (which is imported from a different module and doesn't exist in this one).

Omitting this symbol helps gdb inspect automatically imported symbols, see https://sourceware.org/bugzilla/show_bug.cgi?id=24574 for discussion on the matter.

Surprisingly, those extra symbols don't seem to be an issue for gdb when the sources have been built with clang, only with gcc. The actual logic in gdb that this depends on still is unknown, but omitting these symbols from the symbol table is the right thing to do in any case.

While this isn't a normal bugfix, it fixes one issue reported to me by a user, and the fix is very straightforward, so maybe it still could qualify for the release branch @hans?

[ruiu]

LGTM

[hans]

While this isn't a normal bugfix, it fixes one issue reported to me by a user, and the fix is very straightforward, so maybe it still could qualify for the release branch @hans?

Yes, I'm okay with merging once it's been in trunk a little while.

[hans]

In D65727#1614322, @hans wrote:

While this isn't a normal bugfix, it fixes one issue reported to me by a user, and the fix is very straightforward, so maybe it still could qualify for the release branch @hans?

Yes, I'm okay with merging once it's been in trunk a little while.

Merged in r367986.

lld/trunk/COFF/Writer.cpp

if (!os)
       return None;
 
     sym.Value = def->getRVA() - os->getRVA();
     sym.SectionNumber = os->sectionIndex;
     break;
   }
   }
 
+  // Symbols that are runtime pseudo relocations don't point to the actual
+  // symbol data itself (as they are imported), but points to the IAT entry
+  // instead. Avoid emitting them to the symbol table, as they can confuse
+  // debuggers.
+  if (def->isRuntimePseudoReloc)
+    return None;
+
   StringRef name = def->getName();
   if (name.size() > COFF::NameSize) {
     sym.Name.Offset.Zeroes = 0;
     sym.Name.Offset.Offset = addEntryToStringTable(name);
   } else {
     memset(sym.Name.ShortName, 0, COFF::NameSize);
     memcpy(sym.Name.ShortName, name.data(), name.size());
   }

lld/trunk/test/COFF/autoimport-gnu-implib.s

 # REQUIRES: x86
 
 # RUN: llvm-mc -triple=x86_64-windows-gnu %p/Inputs/gnu-implib-head.s -filetype=obj -o %t-dabcdh.o
 # RUN: llvm-mc -triple=x86_64-windows-gnu %p/Inputs/gnu-implib-data.s -filetype=obj -o %t-dabcds00000.o
 # RUN: llvm-mc -triple=x86_64-windows-gnu %p/Inputs/gnu-implib-tail.s -filetype=obj -o %t-dabcdt.o
 # RUN: rm -f %t-implib.a
 # RUN: llvm-ar rcs %t-implib.a %t-dabcdh.o %t-dabcds00000.o %t-dabcdt.o
 
 # RUN: llvm-mc -triple=x86_64-windows-gnu %s -filetype=obj -o %t.obj
-# RUN: lld-link -lldmingw -out:%t.exe -entry:main %t.obj %t-implib.a -verbose
+# RUN: lld-link -lldmingw -debug:symtab -out:%t.exe -entry:main %t.obj %t-implib.a -verbose
 
 # RUN: llvm-readobj --coff-imports %t.exe | FileCheck -check-prefix=IMPORTS %s
+# RUN: llvm-nm %t.exe | FileCheck -check-prefix=SYMBOLS %s
 
 # IMPORTS: Import {
 # IMPORTS-NEXT: Name: foo.dll
 # IMPORTS-NEXT: ImportLookupTableRVA:
 # IMPORTS-NEXT: ImportAddressTableRVA:
 # IMPORTS-NEXT: Symbol: data (0)
 # IMPORTS-NEXT: }
 
+# Check that the automatically imported symbol "data" is not listed in
+# the symbol table.
+# SYMBOLS-NOT: {{ }}data
+
     .global main
     .text
 main:
     movl data(%rip), %eax
     ret
     .data

lld/trunk/test/COFF/autoimport-x86.s

 # REQUIRES: x86
 
 # RUN: echo -e ".global variable\n.global DllMainCRTStartup\n.text\nDllMainCRTStartup:\nret\n.data\nvariable:\n.long 42" > %t-lib.s
 # RUN: llvm-mc -triple=x86_64-windows-gnu %t-lib.s -filetype=obj -o %t-lib.obj
 # RUN: lld-link -out:%t-lib.dll -dll -entry:DllMainCRTStartup %t-lib.obj -lldmingw -implib:%t-lib.lib
 
 # RUN: llvm-mc -triple=x86_64-windows-gnu %s -filetype=obj -o %t.obj
-# RUN: lld-link -lldmingw -out:%t.exe -entry:main %t.obj %t-lib.lib -verbose
+# RUN: lld-link -lldmingw -debug:symtab -out:%t.exe -entry:main %t.obj %t-lib.lib -verbose
 
 # RUN: llvm-readobj --coff-imports %t.exe | FileCheck -check-prefix=IMPORTS %s
 # RUN: llvm-objdump -d %t.exe | FileCheck -check-prefix=DISASM %s
 # RUN: llvm-objdump -s %t.exe | FileCheck -check-prefix=CONTENTS %s
+# RUN: llvm-nm %t.exe | FileCheck -check-prefix=SYMBOLS %s
 
 # IMPORTS: Import {
 # IMPORTS-NEXT: Name: autoimport-x86.s.tmp-lib.dll
 # IMPORTS-NEXT: ImportLookupTableRVA: 0x2070
 # IMPORTS-NEXT: ImportAddressTableRVA: 0x2080
 # IMPORTS-NEXT: Symbol: variable (0)
 # IMPORTS-NEXT: }
 
 # DISASM: Disassembly of section .text:
 # DISASM-EMPTY:
-# DISASM: .text:
+# DISASM: main:
 # Relative offset at 0x1002 pointing at the IAT at 0x2080.
 # DISASM: 140001000:      8b 05 7a 10 00 00       movl    4218(%rip), %eax
 # DISASM: 140001006:      c3      retq
 
 # Runtime pseudo reloc list header consisting of 0x0, 0x0, 0x1.
 # First runtime pseudo reloc, with import from 0x2080,
 # applied at 0x1002, with a size of 32 bits.
 # Second runtime pseudo reloc, with import from 0x2080,
 # applied at 0x3000, with a size of 64 bits.
 # CONTENTS: Contents of section .rdata:
 # CONTENTS:  140002000 00000000 00000000 01000000 80200000
 # CONTENTS:  140002010 02100000 20000000 80200000 00300000
 # CONTENTS:  140002020 40000000
 # ptr: pointing at the IAT RVA at 0x2080
 # relocs: pointing at the runtime pseudo reloc list at
 # 0x2000 - 0x2024.
 # CONTENTS: Contents of section .data:
 # CONTENTS:  140003000 80200040 01000000 00200040 01000000
 # CONTENTS:  140003010 24200040 01000000
 
+# Check that the automatically imported symbol "variable" is not listed in
+# the symbol table.
+# SYMBOLS-NOT: variable
+
     .global main
     .text
 main:
     movl variable(%rip), %eax
     ret
     .data
 ptr:
     .quad variable
 relocs:
     .quad __RUNTIME_PSEUDO_RELOC_LIST__
     .quad __RUNTIME_PSEUDO_RELOC_LIST_END__