[WebAssembly] [lld] Exclude COMDAT sections

[dschuff]

Summary

Allow exclusion/discarding of custom sections with COMDAT groups.
It piggybacks on the existing COMDAT-handling code, but applies to custom sections as well.

[dschuff]

• add tests

[dschuff]

• update. sections seem to work, functions fishy

[sbc100]

lgtm... Aside from the broken test.

lld/wasm/InputFiles.cpp

439

Remove debugging?

498

But the assert above shows that all section symbols are by definition local binding. You can't ever reference section symbol from another object file.. they are always internal to given object.

[dschuff]

• rebase, remove debug print and don't test functions

[dschuff]

I found that comdat functions seem to be getting double-included (i.e. not properly excluded) when using --no-gc-sections (independently of whether the object originates from llc or from assembly). I removed the checks that test functions and made them only test the section behavior. We can fix functions in a follow-up.

lld/test/wasm/Inputs/comdat1.s

+        .section .text.foo,"G",@,foo,comdat
+        .weak foo
+        .type foo,@function
+foo:
+        .functype foo () -> ()
+        return
+        end_function
+
+        .section .debug_foo,"G",@,foo,comdat
+        .int32 1
+        .section .debug_foo,"G",@,duplicate,comdat
+        .int64 123

lld/test/wasm/Inputs/comdat2.s

+        .section .text.foo,"G",@,foo,comdat
+        .weak foo
+        .type foo,@function
+foo:
+        .functype foo () -> ()
+        return
+        end_function
+
+        .section .debug_foo,"G",@,foo,comdat
+        .int32 2
+        .section .debug_foo,"G",@,duplicate,comdat
+        .int64 234

lld/test/wasm/comdat-sections.s

+# RUN: llvm-mc -triple=wasm32 -filetype=obj %p/Inputs/comdat1.s -o %t1.o
+# RUN: llvm-mc -triple=wasm32 -filetype=obj %p/Inputs/comdat2.s -o %t2.o
+# RUN: llvm-mc -triple=wasm32 -filetype=obj %s -o %t.o
+# RUN: wasm-ld --no-gc-sections -o %t.wasm %t.o %t1.o %t2.o
+# RUN: obj2yaml %t.wasm | FileCheck %s
+
+
+        .globl  _start
+        .type  _start,@function
+_start:
+        .functype _start () -> ()
+        call foo
+        end_function
+
+        .functype foo () -> ()
+
+# CHECK:  - Type:            CODE
+# CHECK:    Functions:
+# CHECK:      - Index:           0
+# CHECK:        Locals:          []
+# CHECK:        Body:            0B
+# CHECK:      - Index:           1
+# CHECK:        Locals:          []
+# CHECK:        Body:            1082808080000B
+
+# OOOOPS! we have 2 copies of foo. why?
+# CHECK:      - Index:           2
+# CHECK:        Locals:          []
+# CHECK:        Body:            0F0B
+# CHECK:      - Index:           3
+# CHECK:        Locals:          []
+# CHECK:        Body:            0F0B
+
+# Check that we got 1 copy of each of the .debug_foo sections from the 2 object
+# files, and that they came from the same object.
+# CHECK:  - Type:            CUSTOM
+# CHECK:    Name:            .debug_foo
+# CHECK:    Payload:         010000007B00000000000000
+# CHECK:  - Type:            CUSTOM
+# CHECK:    Name:            name
+# CHECK:    FunctionNames:
+# CHECK:      - Index:           0
+# CHECK:        Name:            __wasm_call_ctors
+# CHECK:      - Index:           1
+# CHECK:        Name:            _start
+# CHECK:      - Index:           2
+# CHECK:        Name:            foo
+# CHECK:      - Index:           3
+# CHECK:        Name:            foo

lld/wasm/InputChunks.h

 public:
   InputSection(const WasmSection &s, ObjFile *f)
       : InputChunk(f, InputChunk::Section), section(s), tombstoneValue(getTombstoneForSection(s.Name)) {
     assert(section.Type == llvm::wasm::WASM_SEC_CUSTOM);
   }
 
   StringRef getName() const override { return section.Name; }
   StringRef getDebugName() const override { return StringRef(); }
-  uint32_t getComdat() const override { return UINT32_MAX; }
+  uint32_t getComdat() const override { return section.Comdat; }
 
 protected:
   ArrayRef<uint8_t> data() const override { return section.Content; }
 
   // Offset within the input section.  This is only zero since this chunk
   // type represents an entire input section, not part of one.
   uint32_t getInputSectionOffset() const override { return 0; }
   uint64_t getTombstone() const override { return tombstoneValue; }

lld/wasm/InputFiles.cpp

else
       fatal(toString(this) + ": invalid table elements");
     for (size_t index = 0; index < seg.Functions.size(); index++) {
       auto functionIndex = seg.Functions[index];
       tableEntriesRel[functionIndex] = index;
       tableEntries[functionIndex] = offset + index;
     }
   }
 
+  ArrayRef<StringRef> comdats = wasmObj->linkingData().Comdats;
+  for (StringRef comdat : comdats) {
+    bool isNew = ignoreComdats || symtab->addComdat(comdat);
+    keptComdats.push_back(isNew);
+  }
+
   uint32_t sectionIndex = 0;
 
   // Bool for each symbol, true if called directly.  This allows us to implement
   // a weaker form of signature checking where undefined functions that are not
   // called directly (i.e. only address taken) don't have to match the defined
   // function's signature.  We cannot do this for directly called functions
   // because those signatures are checked at validation times.
   // See https://bugs.llvm.org/show_bug.cgi?id=40412
   std::vector<bool> isCalledDirectly(wasmObj->getNumberOfSymbols(), false);
   for (const SectionRef &sec : wasmObj->sections()) {
     const WasmSection &section = wasmObj->getWasmSection(sec);
     // Wasm objects can have at most one code and one data section.
     if (section.Type == WASM_SEC_CODE) {
       assert(!codeSection);
       codeSection = &section;
     } else if (section.Type == WASM_SEC_DATA) {
       assert(!dataSection);
       dataSection = &section;
     } else if (section.Type == WASM_SEC_CUSTOM) {
-      customSections.emplace_back(make<InputSection>(section, this));
+      auto *customSec = make<InputSection>(section, this);
+      customSec->discarded = isExcludedByComdat(customSec);
+      customSections.emplace_back(customSec);
       customSections.back()->setRelocations(section.Relocations);
       customSectionsByIndex[sectionIndex] = customSections.back();
     }
     sectionIndex++;
     // Scans relocations to determine if a function symbol is called directly.
     for (const WasmRelocation &reloc : section.Relocations)
       if (reloc.Type == R_WASM_FUNCTION_INDEX_LEB)
         isCalledDirectly[reloc.Index] = true;
   }
 
   typeMap.resize(getWasmObj()->types().size());
   typeIsUsed.resize(getWasmObj()->types().size(), false);
 
-  ArrayRef<StringRef> comdats = wasmObj->linkingData().Comdats;
-  for (StringRef comdat : comdats) {
-    bool isNew = ignoreComdats || symtab->addComdat(comdat);
-    keptComdats.push_back(isNew);
-  }
 
   // Populate `Segments`.
   for (const WasmSegment &s : wasmObj->dataSegments()) {
     auto* seg = make<InputSegment>(s, this);
     seg->discarded = isExcludedByComdat(seg);
     segments.emplace_back(seg);
   }
   setRelocs(segments, dataSection);
if (wasmSym.isDefined()) {
       }
     }
     size_t idx = symbols.size();
     symbols.push_back(createUndefined(wasmSym, isCalledDirectly[idx]));
   }
 }
 
 bool ObjFile::isExcludedByComdat(InputChunk *chunk) const {
+  llvm::errs() << "Checking " << chunk->getName() << "\n";
   uint32_t c = chunk->getComdat();
   if (c == UINT32_MAX)
     return false;
+  llvm::errs() << "InputChunk kind " << chunk->kind() << " name "
+               << chunk->getName() << " comdat " << c << " excluded "
+               << !keptComdats[c] << "\n";
   return !keptComdats[c];
 }
 
 FunctionSymbol *ObjFile::getFunctionSymbol(uint32_t index) const {
   return cast<FunctionSymbol>(symbols[index]);
 }
 
 GlobalSymbol *ObjFile::getGlobalSymbol(uint32_t index) const {
InputGlobal *global =
         globals[sym.Info.ElementIndex - wasmObj->getNumImportedGlobals()];
     if (sym.isBindingLocal())
       return make<DefinedGlobal>(name, flags, this, global);
     return symtab->addDefinedGlobal(name, flags, this, global);
   }
   case WASM_SYMBOL_TYPE_SECTION: {
     InputSection *section = customSectionsByIndex[sym.Info.ElementIndex];
     assert(sym.isBindingLocal());
+    // Need to return null if discarded here? data and func only do that when
+    // binding is not local.
+    if (section->discarded)
+      return nullptr;
     return make<SectionSymbol>(flags, section, this);
   }
   case WASM_SYMBOL_TYPE_EVENT: {
     InputEvent *event =
         events[sym.Info.ElementIndex - wasmObj->getNumImportedEvents()];
     if (sym.isBindingLocal())
       return make<DefinedEvent>(name, flags, this, event);
     return symtab->addDefinedEvent(name, flags, this, event);

lld/wasm/OutputSections.cpp

 
 void CustomSection::finalizeContents() {
   raw_string_ostream os(nameData);
   encodeULEB128(name.size(), os);
   os << name;
   os.flush();
 
   for (InputSection *section : inputSections) {
+    assert(!section->discarded);
     section->outputSec = this;
     section->outputOffset = payloadSize;
     payloadSize += section->getSize();
   }
 
   createHeader(payloadSize + nameData.size());
 }
 

lld/wasm/Writer.cpp

 
 } // anonymous namespace
 
 void Writer::calculateCustomSections() {
   log("calculateCustomSections");
   bool stripDebug = config->stripDebug || config->stripAll;
   for (ObjFile *file : symtab->objectFiles) {
     for (InputSection *section : file->customSections) {
+      // Exclude COMDAT sections that are not selected for inclusion
+      if (section->discarded)
+        continue;
       StringRef name = section->getName();
       // These custom sections are known the linker and synthesized rather than
       // blindly copied.
       if (name == "linking" || name == "name" || name == "producers" ||
           name == "target_features" || name.startswith("reloc."))
         continue;
       // These custom sections are generated by `clang -fembed-bitcode`.
       // These are used by the rust toolchain to ship LTO data along with