[WebAssembly] Add support for DWARF type units

[dschuff]

Summary

Since Wasm comdat sections work similarly to ELF, we can use that mechanism
to eliminate duplicate dwarf type information in the same way.

[dschuff]

llvm/lib/MC/MCObjectFileInfo.cpp

967

I may add a couple more tests to this, but I did want to ask @sbc100 about this, since I'm not 100% sure at the uniqueID field is for.

[sbc100]

llvm/lib/MC/WasmObjectWriter.cpp

1353 ↗

(On Diff #295369)

Just checking in the case where you were hitting this error the SectionName was duplicate but the Begin is uniquified ?

[dschuff]

llvm/lib/MC/MCObjectFileInfo.cpp

967

also let me be more clear about the question here: what is UniqueID for, and is it bad that I'm just passing it a number that is totally not unique?

llvm/lib/MC/WasmObjectWriter.cpp

1353 ↗

(On Diff #295369)

right. There's a second .debug_info section, so the name of Begin gets uniquified.

[aardappel]

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

48

I guess this doesn't actually test that only one copy of these remain after linking? That's already tested in LLD?

[dblaikie]

llvm/lib/MC/MCObjectFileInfo.cpp

967

For ELF, at least, I believe the unique ID is used to know which elements are to be treated as part of the same deduplication set.

If Wasm support in lld does the same thing, then using the same number for every type unit would mean the linked binary would end up with only one type definition - even when the input has many varied/independent type definitions. Likely not what's intended.

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

25–31

Given that S.plit DWARF type units don't require comdat support (the dwp tool will be aware of the type units and parse their boundaries, read their type hash from the TU Header, etc). /may/ be worth separating that functionality/testing from the comdat support/testing - but maybe not all that interesting to separate it into two separate patches. (& if you find the test coverage works better in one test, that's OK - just a thought)

48

Certainly lld shouldn't be tested here, no (llvm tests can't depend on other subprojects like lld) - but yeah, some kind of comdat deduplication tests should exist in lld (they don't have to be DWARF specific - the DWARF type deduplication is meant to piggy-back on the existing comdat deduplication infrastructure in thel inker)

[dschuff]

llvm/lib/MC/MCObjectFileInfo.cpp

967

For wasm I had thought that was what the 3rd argument (Group) was for. So if that's what UniqueID is for, then I have the same question about Group :)

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

25–31

I copied this test more-or-less directly from X86 :)
Although I will say it's kind of nice to test all the header types in one go like this just because it's slightly annoying to construct the LLVM metadata for debuginfo tests, so it's nice to be able to share that as much as possible. Previously we just didn't have enough coverage for split-dwarf anyway.

48

@aardappel yes, pretty much what @dblaikie said :)

[dblaikie]

llvm/lib/MC/MCObjectFileInfo.cpp

967

Oh, fair enough - I hadn't read closely. Yeah, guess it's up to you folks/how the wasm object format works... - so I'm with you on the "what is the uniqueID field for" (& what's the other field that's taking the hash?) & I'll leave it to you folks to... hash out.

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

25–31

Ah, fair enough - symmetry sounds good :)

[dschuff]

llvm/lib/MC/MCObjectFileInfo.cpp

967

@sbc100 ping, is this what we want here?

[sbc100]

This comment was removed by sbc100.

[sbc100]

llvm/lib/MC/MCObjectFileInfo.cpp

967

It looks like you can use GenericSectionID rather and ~0 here (and we should change that elsewhere too).

Other than that lgtm

[dschuff]

use GenericSectionID instead of ~0

[dschuff]

This broke the bots for some strange reason that didn't reproduce locally. But because it was ~all of them, I probably just did something stupid.

[dblaikie]

In D88603#2357973, @dschuff wrote:

This broke the bots for some strange reason that didn't reproduce locally. But because it was ~all of them, I probably just did something stupid.

Good to have links to buildbots and/or quotes from buildbots (incase the logs get retired) about the specifics so other folks can chip in/know what went wrong, etc.

[dschuff]

In D88603#2359554, @dblaikie wrote:

In D88603#2357973, @dschuff wrote:

This broke the bots for some strange reason that didn't reproduce locally. But because it was ~all of them, I probably just did something stupid.

Good to have links to buildbots and/or quotes from buildbots (incase the logs get retired) about the specifics so other folks can chip in/know what went wrong, etc.

Example link is http://lab.llvm.org:8011/#/builders/109/builds/1402
But, as I suspected, I just did something stupid (namely, I thought I had assertions enabled, but I didn't)

[dschuff]

use getOrCreate

[dschuff]

can't just getOrCreate symbol without dealing with the section
just disable the test for now as we don't need dw5

[dschuff]

fix diff; it should be against master

[dschuff]

@sbc100 I found that the cause of the assertion is that
in dwarf 5, the type units apparently go in the .debug_info section (instead of the .debug_type section), and this section already exists (but it was created as a non-comdat).
So when getWasmSection tries to look up an existing section it fails because the group is part of the key. Then it tries to create a new section but that fails because the name is duplicate.

For some reason this doesn't happen or is not a problem with ELF but I haven't looked up why yet.
For now I just disabled the dwarf5+TU tests since we don't really use dwarf5 yet anyway.

[dblaikie]

In D88603#2360845, @dschuff wrote:

@sbc100 I found that the cause of the assertion is that
in dwarf 5, the type units apparently go in the .debug_info section (instead of the .debug_type section), and this section already exists (but it was created as a non-comdat).
So when getWasmSection tries to look up an existing section it fails because the group is part of the key. Then it tries to create a new section but that fails because the name is duplicate.

For some reason this doesn't happen or is not a problem with ELF but I haven't looked up why yet.
For now I just disabled the dwarf5+TU tests since we don't really use dwarf5 yet anyway.

for ELF, each {section name, hash} pair denotes a distinct section (when using SHF_GROUP). Linker's deduplicate the group based on the hash, then squish all the distinct sections with the same name together. (same system that powers -ffunction-sections -fno-unique-section-names - even though each function section is called ".text", because they have different hashes and use SHF_GROUP, they are distinct sections in the object file)

[dblaikie]

In D88603#2361010, @dblaikie wrote:

In D88603#2360845, @dschuff wrote:

@sbc100 I found that the cause of the assertion is that
in dwarf 5, the type units apparently go in the .debug_info section (instead of the .debug_type section), and this section already exists (but it was created as a non-comdat).
So when getWasmSection tries to look up an existing section it fails because the group is part of the key. Then it tries to create a new section but that fails because the name is duplicate.

For some reason this doesn't happen or is not a problem with ELF but I haven't looked up why yet.
For now I just disabled the dwarf5+TU tests since we don't really use dwarf5 yet anyway.

for ELF, each {section name, hash} pair denotes a distinct section (when using SHF_GROUP). Linker's deduplicate the group based on the hash, then squish all the distinct sections with the same name together. (same system that powers -ffunction-sections -fno-unique-section-names - even though each function section is called ".text", because they have different hashes and use SHF_GROUP, they are distinct sections in the object file)

Without similar behavior for wasm, how does wasm deduplicate these type units, if they're all in one section together? (have you tested this with two types? I'd expect that to hit the same problem - of attempting to create a section that already exists when it went to create the second type unit)

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 = getOrCreateSymbol(CachedName);
+  MCSymbol *Begin = createSymbol(CachedName, false, 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/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
+
+; 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
+
+; 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).
+; (v2 thru v4 CUs are all the same, and TUs were invented in v4,
+; so we don't bother checking older versions.)
+
+; Test case built from:
+;struct S {
+;  int s1;
+;};
+;
+;S s;
+
+; Verify the v4 non-split headers.
+; Note that we check the exact offset of the DIEs because that tells us
+; the length of the header.
+;
+; SINGLE-4: .debug_info contents:
+; SINGLE-4: 0x00000000: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x0000000b: DW_TAG_compile_unit
+;
+; SINGLE-4: .debug_types contents:
+; SINGLE-4: 0x00000000: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; SINGLE-4: 0x00000017: DW_TAG_type_unit
+
+; Verify the v4 split headers.
+;
+; O-4: .debug_info contents:
+; O-4: 0x00000000: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; O-4: 0x0000000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_info.dwo contents:
+; DWO-4: 0x00000000: Compile Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x0000000b: DW_TAG_compile_unit
+;
+; DWO-4: .debug_types.dwo contents:
+; DWO-4: 0x00000000: Type Unit: {{.*}} version = 0x0004, abbr_offset
+; DWO-4: 0x00000017: DW_TAG_type_unit
+
+; Verify the v5 non-split headers. Type units come first.
+; All .debug_info sections are reported in one go, but the offset resets for
+; each new section.
+;
+; SINGLE-5: .debug_info contents:
+; SINGLE-5: 0x00000000: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_type, abbr_offset
+; SINGLE-5: 0x00000018: DW_TAG_type_unit
+; SINGLE-5-NOT: contents:
+; SINGLE-5: 0x00000000: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_compile, abbr_offset
+; SINGLE-5: 0x0000000c: DW_TAG_compile_unit
+
+; Verify the v5 split headers.
+;
+; O-5: .debug_info contents:
+; O-5: 0x00000000: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset
+; O-5-SAME:        DWO_id = 0xccd7e58ef8bf4aa6
+; O-5: 0x00000014: DW_TAG_skeleton_unit 
+;
+; DWO-5: .debug_info.dwo contents:
+; DWO-5: 0x00000000: Type Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_type, abbr_offset
+; DWO-5: 0x00000018: DW_TAG_type_unit
+; DWO-5: 0x00000033: Compile Unit: {{.*}} version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset
+; DWO-5-SAME:        DWO_id = 0xccd7e58ef8bf4aa6
+; DWO-5: 0x00000047: DW_TAG_compile_unit
+
+
+; 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)"}