[lld/mac] Don't emit stabs entries for functions folded during ICF

[thakis]

Summary

This matches ld64, and makes dsymutil work better with lld's output.
Fixes PR54783, see there for details.

Reduces time needed to run dsymutil on Chromium Framework from 8m30s
(which is already down from 26 min with D123218) to 6m30s and removes
many lines of "could not find object file symbol for symbol" from dsymutil output
(previously: several MB of those messages, now dsymutil is completely silent).

[thakis]

make test pass on windows

[int3]

lld/MachO/Symbols.cpp

104

How do you feel about doing this work in ConcatInputSection::foldIdentical instead? We already toggle ConcatInputSection::wasCoalesced there, so this would fit in nicely.

(We could also perform the canonicalization of Defined::isec there -- I'd previously put it here because InputSections didn't use to hold references to their symbols.)

lld/MachO/Symbols.h

148

nit: "with" suggests a symmetric relationship, "into" seems more apt to me

[thakis]

lld/MachO/Symbols.cpp

104

Do you mean giving ConcatInputSection a bool for this and setting that bool instead of giving Defined a bit, and then querying defined->isec->wasIdenticalCodeFolded instead of defined->wasIdenticalCodeFolded? Or do you mean keeping the bit on Defined and make ConcatInputSection::foldIdentical walk symbols and set the bit on all the section's symbols?

[thakis]

Thanks!

lld/MachO/Symbols.cpp

104

Oh wait the former doesn't work, defined->isec is the canonical section.

So did the latter. LMK if that's not what you meant :)

[int3]

lld/MachO/Symbols.cpp

104

I did mean the latter, but you got me pondering whether the former would work for a second :p

lgtm, thanks!

lld/MachO/InputSection.cpp

return (toString(getFile()) + ":(" + getName() + "+0x" +
       .str();
 }
 
 void ConcatInputSection::foldIdentical(ConcatInputSection *copy) {
   align = std::max(align, copy->align);
   copy->live = false;
   copy->wasCoalesced = true;
   copy->replacement = this;
+  for (auto &copySym : copy->symbols)
+    copySym->wasIdenticalCodeFolded = true;
 
   // Merge the sorted vectors of symbols together.
   auto it = symbols.begin();
   for (auto copyIt = copy->symbols.begin(); copyIt != copy->symbols.end();) {
     if (it == symbols.end()) {
       symbols.push_back(*copyIt++);
       it = symbols.end();
     } else if ((*it)->value > (*copyIt)->value) {

lld/MachO/Symbols.h

public:
 
   // Place the bitfields first so that they can get placed in the tail padding
   // of the parent class, on platforms which support it.
   bool overridesWeakDef : 1;
   // Whether this symbol should appear in the output binary's export trie.
   bool privateExtern : 1;
   // Whether this symbol should appear in the output symbol table.
   bool includeInSymtab : 1;
+  // Whether this symbol was folded into a different symbol during ICF.
+  bool wasIdenticalCodeFolded : 1;
   // Only relevant when compiling for Thumb-supporting arm32 archs.
   bool thumb : 1;
   // Symbols marked referencedDynamically won't be removed from the output's
   // symbol table by tools like strip. In theory, this could be set on arbitrary
   // symbols in input object files. In practice, it's used solely for the
   // synthetic __mh_execute_header symbol.
   // This is information for the static linker, and it's also written to the
   // output file's symbol table for tools running later (such as `strip`).

lld/MachO/Symbols.cpp

 
 Defined::Defined(StringRefZ name, InputFile *file, InputSection *isec,
                  uint64_t value, uint64_t size, bool isWeakDef, bool isExternal,
                  bool isPrivateExtern, bool isThumb,
                  bool isReferencedDynamically, bool noDeadStrip,
                  bool canOverrideWeakDef, bool isWeakDefCanBeHidden,
                  bool interposable)
     : Symbol(DefinedKind, name, file), overridesWeakDef(canOverrideWeakDef),
-      privateExtern(isPrivateExtern), includeInSymtab(true), thumb(isThumb),
+      privateExtern(isPrivateExtern), includeInSymtab(true),
+      wasIdenticalCodeFolded(false), thumb(isThumb),
       referencedDynamically(isReferencedDynamically), noDeadStrip(noDeadStrip),
       interposable(interposable), weakDefCanBeHidden(isWeakDefCanBeHidden),
       weakDef(isWeakDef), external(isExternal), isec(isec), value(value),
       size(size) {
   if (isec) {
     isec->symbols.push_back(this);
     // Maintain sorted order.
     for (auto it = isec->symbols.rbegin(), rend = isec->symbols.rend();

 void Defined::canonicalize() {
   if (unwindEntry)
     unwindEntry = unwindEntry->canonical();
   if (isec)
     isec = isec->canonical();
 }
 
 uint64_t DylibSymbol::getVA() const {
   return isInStubs() ? getStubVA() : Symbol::getVA();
 }
 
 void LazyArchive::fetchArchiveMember() { getFile()->fetch(sym); }

lld/MachO/SyntheticSections.cpp

void SymtabSection::emitStabs() {
   for (const SymtabEntry &entry :
        concat<SymtabEntry>(localSymbols, externalSymbols)) {
     Symbol *sym = entry.sym;
     assert(sym->isLive() &&
            "dead symbols should not be in localSymbols, externalSymbols");
     if (auto *defined = dyn_cast<Defined>(sym)) {
       if (defined->isAbsolute())
         continue;
+
+      // Constant-folded symbols go in the executable's symbol table, but don't
+      // get a stabs entry.
+      if (defined->wasIdenticalCodeFolded)
+        continue;
+
       InputSection *isec = defined->isec;
       ObjFile *file = dyn_cast_or_null<ObjFile>(isec->getFile());
       if (!file || !file->compileUnit)
         continue;
       symbolsNeedingStabs.push_back(defined);
     }
   }
 

lld/test/MachO/stabs-icf.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
+
+# RUN: %lld -lSystem --icf=all %t.o -o %t
+# RUN: dsymutil -s %t | FileCheck %s -DDIR=%t -DSRC_PATH=%t.o
+
+# This should include no N_FUN entry for _bar2 (which is ICF'd into _bar),
+# but it does include a SECT EXT entry.
+# CHECK:      (N_SO         ) 00      0000   0000000000000000   '/tmp{{[/\\]}}test.cpp'
+# CHECK-NEXT: (N_OSO        ) 03      0001   {{.*}} '[[SRC_PATH]]'
+# CHECK-NEXT: (N_FUN        ) 01      0000   [[#%.16x,MAIN:]]   '_main'
+# CHECK-NEXT: (N_FUN        ) 00      0000   000000000000000b{{$}}
+# CHECK-NEXT: (N_FUN        ) 01      0000   [[#%.16x,BAR:]]    '_bar'
+# CHECK-NEXT: (N_FUN        ) 00      0000   0000000000000001{{$}}
+# CHECK-NEXT: (N_SO         ) 01      0000   0000000000000000{{$}}
+# CHECK-DAG:  (     SECT EXT) 01      0000   [[#MAIN]]           '_main'
+# CHECK-DAG:  (     SECT EXT) 01      0000   [[#BAR]]           '_bar'
+# CHECK-DAG:  (     SECT EXT) 01      0000   [[#BAR]]          '_bar2'
+# CHECK-DAG:  (       {{.*}}) {{[0-9]+}}                 0010   {{[0-9a-f]+}}      '__mh_execute_header'
+# CHECK-DAG:  (       {{.*}}) {{[0-9]+}}                 0100   0000000000000000   'dyld_stub_binder'
+# CHECK-EMPTY:
+
+.text
+.globl _bar, _bar2, _main
+
+.subsections_via_symbols
+
+_bar:
+  ret
+
+_bar2:
+  ret
+
+_main:
+Lfunc_begin0:
+  call _bar
+  call _bar2
+  ret
+Lfunc_end0:
+
+.section  __DWARF,__debug_str,regular,debug
+  .asciz  "test.cpp"             ## string offset=0
+  .asciz  "/tmp"                 ## string offset=9
+.section  __DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+  .byte  1                       ## Abbreviation Code
+  .byte  17                      ## DW_TAG_compile_unit
+  .byte  1                       ## DW_CHILDREN_yes
+  .byte  3                       ## DW_AT_name
+  .byte  14                      ## DW_FORM_strp
+  .byte  27                      ## DW_AT_comp_dir
+  .byte  14                      ## DW_FORM_strp
+  .byte  17                      ## DW_AT_low_pc
+  .byte  1                       ## DW_FORM_addr
+  .byte  18                      ## DW_AT_high_pc
+  .byte  6                       ## DW_FORM_data4
+  .byte  0                       ## EOM(1)
+.section  __DWARF,__debug_info,regular,debug
+.set Lset0, Ldebug_info_end0-Ldebug_info_start0 ## Length of Unit
+  .long  Lset0
+Ldebug_info_start0:
+  .short  4                       ## DWARF version number
+.set Lset1, Lsection_abbrev-Lsection_abbrev ## Offset Into Abbrev. Section
+  .long  Lset1
+  .byte  8                       ## Address Size (in bytes)
+  .byte  1                       ## Abbrev [1] 0xb:0x48 DW_TAG_compile_unit
+  .long  0                       ## DW_AT_name
+  .long  9                       ## DW_AT_comp_dir
+  .quad  Lfunc_begin0            ## DW_AT_low_pc
+.set Lset3, Lfunc_end0-Lfunc_begin0     ## DW_AT_high_pc
+  .long  Lset3
+  .byte  0                       ## End Of Children Mark
+Ldebug_info_end0: