[WebAssembly] Add Object and ObjectWriter support for wasm COMDAT sections

[dschuff]

Summary

Allow sections to be placed into COMDAT groups, in addition to functions and data
segments.

Also make section symbols unnamed, which allows sections with identical names
(section names are independent of their section symbols, but previously we
gave the symbols the same name as their sections, which results in collisions
when sections are identically-named).

[dschuff]

Add the rest

[sbc100]

Looks great!

llvm/test/MC/WebAssembly/comdat-sections.ll

8

Where does this name come from?

[dschuff]

This version basically just tests -fdebug-types-section end-to-end to YAML via comdats. (It probably needs more tests, suggestions are welcome)
I'm thinking it would be good to add assembly syntax so we can test just the MC and Object stuff separately from the debuginfo stuff. Not sure if that should be a separate change?

[dschuff]

llvm/test/MC/WebAssembly/comdat-sections.ll

8

The name is just the hash of the dwarf description of the type. So duplicate types will get duplicate names.

[dschuff]

• clang-format

[dschuff]

I'm going to add assembly support in a separate change. So this change is ready for review; it has the end-to-end test, and when I add assembler support I'll add tests that go from ll -> asm and asm -> YAML

[sbc100]

llvm/include/llvm/BinaryFormat/Wasm.h

334

Can you upload a PR to tooling-conventions to add this ?

[dschuff]

rebase, fix typo

llvm/include/llvm/BinaryFormat/Wasm.h

enum : unsigned {
   WASM_COMDAT_INFO = 0x7,
   WASM_SYMBOL_TABLE = 0x8,
 };
 
 // Kind codes used in the custom "linking" section in the WASM_COMDAT_INFO
 enum : unsigned {
   WASM_COMDAT_DATA = 0x0,
   WASM_COMDAT_FUNCTION = 0x1,
+  // GLOBAL, EVENT, and TABLE are in here but LLVM doesn't use them yet.
+  WASM_COMDAT_SECTION = 0x5,
 };
 
 // Kind codes used in the custom "linking" section in the WASM_SYMBOL_TABLE
 enum WasmSymbolType : unsigned {
   WASM_SYMBOL_TYPE_FUNCTION = 0x0,
   WASM_SYMBOL_TYPE_DATA = 0x1,
   WASM_SYMBOL_TYPE_GLOBAL = 0x2,
   WASM_SYMBOL_TYPE_SECTION = 0x3,

llvm/include/llvm/Object/Wasm.h

 };
 
 struct WasmSection {
   WasmSection() = default;
 
   uint32_t Type = 0;         // Section type (See below)
   uint32_t Offset = 0;       // Offset with in the file
   StringRef Name;            // Section name (User-defined sections only)
+  uint32_t Comdat = UINT32_MAX; // From the "comdat info" section
   ArrayRef<uint8_t> Content; // Section content
   std::vector<wasm::WasmRelocation> Relocations; // Relocations for this section
 };
 
 struct WasmSegment {
   uint32_t SectionOffset;
   wasm::WasmDataSegment Data;
 };

llvm/lib/MC/MCContext.cpp

MCSectionWasm *MCContext::getWasmSection(const Twine &Section, SectionKind Kind,
   auto IterBool = WasmUniquingMap.insert(
       std::make_pair(WasmSectionKey{Section.str(), Group, UniqueID}, nullptr));
   auto &Entry = *IterBool.first;
   if (!IterBool.second)
     return Entry.second;
 
   StringRef CachedName = Entry.first.SectionName;
 
-  MCSymbol *Begin = createSymbol(CachedName, false, false);
+  MCSymbol *Begin = createSymbol(CachedName, true, false);
   cast<MCSymbolWasm>(Begin)->setType(wasm::WASM_SYMBOL_TYPE_SECTION);
 
   MCSectionWasm *Result = new (WasmAllocator.Allocate())
       MCSectionWasm(CachedName, Kind, GroupSym, UniqueID, Begin);
   Entry.second = Result;
 
   auto *F = new MCDataFragment();
   Result->getFragmentList().insert(Result->begin(), F);

llvm/lib/MC/MCObjectFileInfo.cpp

 
 MCSection *MCObjectFileInfo::getDwarfComdatSection(const char *Name,
                                                    uint64_t Hash) const {
   switch (TT.getObjectFormat()) {
   case Triple::ELF:
     return Ctx->getELFSection(Name, ELF::SHT_PROGBITS, ELF::SHF_GROUP, 0,
                               utostr(Hash));
   case Triple::Wasm:
-    // FIXME: When using dwarf 5, the .debug_info section is used for type units
-    // but that section already exists, so attempting to get it as a comdate
-    // section triggers an assert.
     return Ctx->getWasmSection(Name, SectionKind::getMetadata(), utostr(Hash),
                                MCContext::GenericSectionID);
   case Triple::MachO:
   case Triple::COFF:
   case Triple::GOFF:
   case Triple::XCOFF:
   case Triple::UnknownObjectFormat:
     report_fatal_error("Cannot get DWARF comdat section for this object file "

llvm/lib/MC/WasmObjectWriter.cpp

if (Section.isWasmData()) {
         continue;
       }
       if (Name == "target_features") {
         TargetFeaturesSection =
             std::make_unique<WasmCustomSection>(Name, &Section);
         continue;
       }
 
+      // Custom sections can also belong to COMDAT groups. In this case the
+      // decriptor's "index" field is the section index (in the final object
+      // file), but that is not known until after layout, so it must be fixed up
+      // later
+      if (const MCSymbolWasm *C = Section.getGroup()) {
+        Comdats[C->getName()].emplace_back(
+            WasmComdatEntry{wasm::WASM_COMDAT_SECTION,
+                            static_cast<uint32_t>(CustomSections.size())});
+      }
+
       CustomSections.emplace_back(Name, &Section);
     }
   }
 
   // Populate WasmIndices and DataLocations for defined symbols.
   for (const MCSymbol &S : Asm.symbols()) {
     // Ignore unnamed temporary symbols, which aren't ever exported, imported,
     // or used in relocations.
if (Mode != DwoMode::DwoOnly) {
     writeExportSection(Exports);
     writeElemSection(TableElems);
     writeDataCountSection();
 
     CodeSectionIndex = writeCodeSection(Asm, Layout, Functions);
     DataSectionIndex = writeDataSection(Layout);
   }
 
-  for (auto &CustomSection : CustomSections) {
-    writeCustomSection(CustomSection, Asm, Layout);
+  // The Sections in the COMDAT list have placeholder indices (their index among
+  // custom sections, rather than among all sections). Fix them up here.
+  for (auto &Group : Comdats) {
+    for (auto &Entry : Group.second) {
+      if (Entry.Kind == wasm::WASM_COMDAT_SECTION) {
+        Entry.Index += SectionCount;
+      }
     }
+  }
+  for (auto &CustomSection : CustomSections)
+    writeCustomSection(CustomSection, Asm, Layout);
 
   if (Mode != DwoMode::DwoOnly) {
     writeLinkingMetaDataSection(SymbolInfos, InitFuncs, Comdats);
 
     writeRelocSection(CodeSectionIndex, "CODE", CodeRelocations);
     writeRelocSection(DataSectionIndex, "DATA", DataRelocations);
   }
   writeCustomRelocSections();

llvm/lib/Object/WasmObjectFile.cpp

while (EntryCount--) {
         if (!isDefinedFunctionIndex(Index))
           return make_error<GenericBinaryError>(
               "COMDAT function index out of range", object_error::parse_failed);
         if (getDefinedFunction(Index).Comdat != UINT32_MAX)
           return make_error<GenericBinaryError>("Function in two COMDATs",
                                                 object_error::parse_failed);
         getDefinedFunction(Index).Comdat = ComdatIndex;
         break;
+      case wasm::WASM_COMDAT_SECTION:
+        if (Index >= Sections.size())
+          return make_error<GenericBinaryError>(
+              "COMDAT section index out of range", object_error::parse_failed);
+        if (Sections[Index].Type != wasm::WASM_SEC_CUSTOM)
+          return make_error<GenericBinaryError>(
+              "Non-custom section in a COMDAT", object_error::parse_failed);
+        Sections[Index].Comdat = ComdatIndex;
+        break;
       }
     }
   }
   return Error::success();
 }
 
 Error WasmObjectFile::parseProducersSection(ReadContext &Ctx) {
   llvm::SmallSet<StringRef, 3> FieldsSeen;

llvm/lib/ObjectYAML/WasmYAML.cpp

void MappingTraits<WasmYAML::InitFunction>::mapping(
   IO.mapRequired("Symbol", Init.Symbol);
 }
 
 void ScalarEnumerationTraits<WasmYAML::ComdatKind>::enumeration(
     IO &IO, WasmYAML::ComdatKind &Kind) {
 #define ECase(X) IO.enumCase(Kind, #X, wasm::WASM_COMDAT_##X);
   ECase(FUNCTION);
   ECase(DATA);
+  ECase(SECTION);
 #undef ECase
 }
 
 void MappingTraits<WasmYAML::ComdatEntry>::mapping(
     IO &IO, WasmYAML::ComdatEntry &ComdatEntry) {
   IO.mapRequired("Kind", ComdatEntry.Kind);
   IO.mapRequired("Index", ComdatEntry.Index);
 }

llvm/test/DebugInfo/WebAssembly/dwarf-headers.ll

 ; RUN: llc -dwarf-version=4 -generate-type-units \
 ; RUN:     -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
 ; RUN:     | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-4
 
 ; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
 ; RUN:     -dwarf-version=4 -generate-type-units \
 ; RUN:     -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
 ; RUN:     | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-4
 ; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-4
 
-; TODO: enable testing for dwarf v5 with type units
-; (See the FIXME in MCObjectFileInfo::getDwarfComdatSection)
-; RU N: llc -dwarf-version=5 -generate-type-units \
-; RU N:     -filetype=obj -O0 -mtriple= < %s \
-; RU N:     | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
+; RUN: llc -dwarf-version=5 -generate-type-units \
+; RUN:     -filetype=obj -O0 -mtriple= < %s \
+; RUN:     | llvm-dwarfdump -v - | FileCheck %s --check-prefix=SINGLE-5
 
-; RU N: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
-; RU N:     -dwarf-version=5 -generate-type-units \
-; RU N:     -filetype=obj -O0 -mtriple= < %s \
-; RU N:     | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
-; RU N: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=%t.dwo \
+; RUN:     -dwarf-version=5 -generate-type-units \
+; RUN:     -filetype=obj -O0 -mtriple= < %s \
+; RUN:     | llvm-dwarfdump -v - | FileCheck %s --check-prefix=O-5
+; RUN: llvm-dwarfdump -v %t.dwo | FileCheck %s --check-prefix=DWO-5
 
 ; This test is derived from test/CodeGen/X86/dwarf-headers.ll
 
 ; Looking for DWARF headers to be generated correctly.
 ; There are 8 variants with 5 formats: v4 CU, v4 TU, v5 normal/partial CU,
 ; v5 skeleton/split CU, v5 normal/split TU.  Some v5 variants differ only
 ; in the unit_type code, and the skeleton/split CU differs from normal/partial
 ; by having one extra field (dwo_id).

llvm/test/MC/WebAssembly/comdat-sections.ll

+; RUN: llc -dwarf-version=4 -generate-type-units \
+; RUN:     -filetype=obj -O0 -mtriple=wasm32-unknown-unknown < %s \
+; RUN:     | obj2yaml | FileCheck %s
+
+
+; CHECK:     Comdats:
+; CHECK:      - Name:            '4721183873463917179'
+; CHECK:        Entries:
+; CHECK:          - Kind:            SECTION
+; CHECK:            Index:           3
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown-wasm"
+
+%struct.S = type { i32 }
+
+@s = global %struct.S zeroinitializer, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11}
+!llvm.ident = !{!12}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "s", scope: !2, file: !3, line: 5, type: !6, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !3, producer: "clang version 5.0.0 (trunk 295942)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5)
+!3 = !DIFile(filename: "t.cpp", directory: "/home/probinson/projects/scratch")
+!4 = !{}
+!5 = !{!0}
+!6 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "S", file: !3, line: 1, size: 32, elements: !7, identifier: "_ZTS1S")
+!7 = !{!8}
+!8 = !DIDerivedType(tag: DW_TAG_member, name: "s1", scope: !6, file: !3, line: 2, baseType: !9, size: 32)
+!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!10 = !{i32 2, !"Dwarf Version", i32 4}
+!11 = !{i32 2, !"Debug Info Version", i32 3}
+!12 = !{!"clang version 5.0.0 (trunk 295942)"}

llvm/tools/obj2yaml/wasm2yaml.cpp

for (const object::WasmSegment &Segment : Obj.dataSegments()) {
         LinkingSec->SegmentInfos.push_back(SegmentInfo);
       }
       if (Segment.Data.Comdat != UINT32_MAX) {
         LinkingSec->Comdats[Segment.Data.Comdat].Entries.emplace_back(
             WasmYAML::ComdatEntry{wasm::WASM_COMDAT_DATA, SegmentIndex});
       }
       SegmentIndex++;
     }
+    uint32_t SectionIndex = 0;
+    for (const auto &Sec : Obj.sections()) {
+      const WasmSection &WasmSec = Obj.getWasmSection(Sec);
+      if (WasmSec.Comdat != UINT32_MAX)
+        LinkingSec->Comdats[WasmSec.Comdat].Entries.emplace_back(
+            WasmYAML::ComdatEntry{wasm::WASM_COMDAT_SECTION, SectionIndex});
+      SectionIndex++;
+    }
 
     uint32_t SymbolIndex = 0;
     for (const wasm::WasmSymbolInfo &Symbol : Obj.linkingData().SymbolTable) {
       WasmYAML::SymbolInfo Info;
       Info.Index = SymbolIndex++;
       Info.Kind = static_cast<uint32_t>(Symbol.Kind);
       Info.Name = Symbol.Name;
       Info.Flags = Symbol.Flags;