[llvm-objdump][WebAssembly] Fix llvm-objdump on files without symbols

[wingo]

Summary

If a file has no symbols, perhaps because it is a linked executable,
synthesize some symbols by walking the code section. Otherwise the
disassembler will try to treat the whole code section as a function,
which won't parse. Fixes https://bugs.llvm.org/show_bug.cgi?id=50957.

[sbc100]

Thanks for look into this! I've been meaning fix this forever but didn't realize the solution could be this simple. I'm excited to starting objdump in more of our test code once this lands.

Do you think it would make sense to do this at the libObject level rather than at the objdump level? i.e. have libObject guarantee that each function has a symbol? Or is it really only objdump and benefits from these fake symbols?

llvm/test/tools/llvm-objdump/wasm/executable-without-symbols.test

2

If we can we should try to find a way to avoid checking in the binary here.

Also, it would be good to have a test for a file that also lacks a name sections, in which case I would expect to see either empty names, or synthetic/fake names (which do you think make the most sense).

llvm/tools/llvm-objdump/llvm-objdump.cpp

724

Can we keep this function as is, and call it from addFallbackSymbols instead? This would keep the diff smaller and avoid extra nesting.

749

Perhaps assert Function->SymbolName.empty() then?

751

Looking at createSymbolInfo it looks like we always use STT_NOTYPE for wasm symbols.

1165

I think maybe we should leave these two lines alone and add a separate block:

if (Obj->isWam())
   addMissingWasmCodeSymbols(..);

The reasons is because we want to ensure that every function has a symbol don't we? So we might want to handle the case where some functions have symbols and some don't?

[jhenderson]

There's an awful lot of WebAssembly test failures in pre-merge. Are they related (they might not be)?

llvm/test/tools/llvm-objdump/wasm/executable-without-symbols.test

2

Would yaml2obj allow you to avoid the canned binary? Alternatively, presumably there's some way of building this from assembly?

4

Nit: not looked at wasm tests, but in most other llvm-objdump tests, we usually add comment markers (i.e. '#') for CHECK and RUN lines, even if they're not strictly necessary.

May be moot if you switch to yaml2obj or similar, as you'll need them anyway in that case.

[wingo]

Simplify test

[jhenderson]

Test changes look good. I'll leave @sbc100 to review things more thoroughly.

[wingo]

Rebase; will fix spurious presubmit errors

[wingo]

Add wasm symbols for any function without a symbol

[wingo]

Use STT_NOTYPE for synthesized symbols

[wingo]

Thanks all for the feedback! I think the updated patch addresses the feedback; PTAL :)

[wingo]

Tighten up tests

[wingo]

Prettify tests

[sbc100]

Great! This will allow for far more use of objdump in our testing elsewhere. Thanks for working on this.

llvm/test/tools/llvm-objdump/wasm/executable-without-symbols-debugnames.test

+# RUN: yaml2obj -o %t.wasm %s
+# RUN: llvm-objdump -d %t.wasm | FileCheck %s
+
+--- !WASM
+FileHeader:
+  Version:         0x1
+Sections:
+  - Type:            TYPE
+    Signatures:
+      - Index:           0
+        ParamTypes:      []
+        ReturnTypes:     []
+      - Index:           1
+        ParamTypes:
+          - I32
+        ReturnTypes:
+          - I32
+  - Type:            FUNCTION
+    FunctionTypes:   [ 0, 1 ]
+  - Type:            CODE
+    Functions:
+      - Index:           0
+        Locals:          []
+        Body:            0B
+      - Index:           1
+        Locals:          []
+        Body:            20000B
+  - Type:            CUSTOM
+    Name:            name
+    FunctionNames:
+      - Index:           0
+        Name:            f
+      - Index:           1
+        Name:            g
+...
+
+# CHECK:       Disassembly of section CODE:
+# CHECK-EMPTY:
+# CHECK-NEXT:  00000000 <CODE>:
+# CHECK-NEXT:          # 2 functions in section.
+# CHECK-EMPTY:
+# CHECK-NEXT:  00000001 <f>:
+# CHECK-EMPTY:
+# CHECK-NEXT:         3: 0b             end
+# CHECK-EMPTY:
+# CHECK-NEXT:  00000004 <g>:
+# CHECK-EMPTY:
+# CHECK-NEXT:         6: 20 00          local.get 0
+# CHECK-NEXT:         8: 0b             end

llvm/test/tools/llvm-objdump/wasm/executable-without-symbols.test

+# RUN: yaml2obj -o %t.wasm %s
+# RUN: llvm-objdump -d %t.wasm | FileCheck %s
+
+--- !WASM
+FileHeader:
+  Version:         0x1
+Sections:
+  - Type:            TYPE
+    Signatures:
+      - Index:           0
+        ParamTypes:      []
+        ReturnTypes:     []
+      - Index:           1
+        ParamTypes:
+          - I32
+        ReturnTypes:
+          - I32
+  - Type:            FUNCTION
+    FunctionTypes:   [ 0, 1 ]
+  - Type:            CODE
+    Functions:
+      - Index:           0
+        Locals:          []
+        Body:            0B
+      - Index:           1
+        Locals:          []
+        Body:            20000B
+...
+
+# CHECK:       Disassembly of section CODE:
+# CHECK-EMPTY:
+# CHECK-NEXT:  00000000 <CODE>:
+# CHECK-NEXT:          # 2 functions in section.
+# CHECK-EMPTY:
+# CHECK-NEXT:  00000001 <>:
+# CHECK-EMPTY:
+# CHECK-NEXT:         3: 0b             end
+# CHECK-EMPTY:
+# CHECK-NEXT:  00000004 <>:
+# CHECK-EMPTY:
+# CHECK-NEXT:         6: 20 00          local.get 0
+# CHECK-NEXT:         8: 0b             end

llvm/tools/llvm-objdump/llvm-objdump.cpp

for (auto Symbol : Obj->getDynamicSymbolIterators()) {
     if (SecI == Obj->section_end())
       continue;
 
     AllSymbols[*SecI].emplace_back(Address, Name, SymbolType);
   }
 }
 
 static void
 addDynamicElfSymbols(const ObjectFile *Obj,
                      std::map<SectionRef, SectionSymbolsTy> &AllSymbols) {
   assert(Obj->isELF());
   if (auto *Elf32LEObj = dyn_cast<ELF32LEObjectFile>(Obj))
     addDynamicElfSymbols(Elf32LEObj, AllSymbols);
   else if (auto *Elf64LEObj = dyn_cast<ELF64LEObjectFile>(Obj))
     addDynamicElfSymbols(Elf64LEObj, AllSymbols);
   else if (auto *Elf32BEObj = dyn_cast<ELF32BEObjectFile>(Obj))
     addDynamicElfSymbols(Elf32BEObj, AllSymbols);
   else if (auto *Elf64BEObj = cast<ELF64BEObjectFile>(Obj))
     addDynamicElfSymbols(Elf64BEObj, AllSymbols);
   else
     llvm_unreachable("Unsupported binary format");
 }
 
+static Optional<SectionRef> getWasmCodeSection(const WasmObjectFile *Obj) {
+  for (auto SecI : Obj->sections()) {
+    const WasmSection &Section = Obj->getWasmSection(SecI);
+    if (Section.Type == wasm::WASM_SEC_CODE)
+      return SecI;
+  }
+  return None;
+}
+
+static void
+addMissingWasmCodeSymbols(const WasmObjectFile *Obj,
+                          std::map<SectionRef, SectionSymbolsTy> &AllSymbols) {
+  Optional<SectionRef> Section = getWasmCodeSection(Obj);
+  if (!Section)
+    return;
+  SectionSymbolsTy &Symbols = AllSymbols[*Section];
+
+  std::set<uint64_t> SymbolAddresses;
+  for (const auto &Sym : Symbols)
+    SymbolAddresses.insert(Sym.Addr);
+
+  for (const wasm::WasmFunction &Function : Obj->functions()) {
+    uint64_t Address = Function.CodeSectionOffset;
+    // Only add fallback symbols for functions not already present in the symbol
+    // table.
+    if (SymbolAddresses.count(Address))
+      continue;
+    // This function has no symbol, so it should have no SymbolName.
+    assert(Function.SymbolName.empty());
+    // We use DebugName for the name, though it may be empty if there is no
+    // "name" custom section, or that section is missing a name for this
+    // function.
+    StringRef Name = Function.DebugName;
+    Symbols.emplace_back(Address, Name, ELF::STT_NOTYPE);
+  }
+}
+
 static void addPltEntries(const ObjectFile *Obj,
                           std::map<SectionRef, SectionSymbolsTy> &AllSymbols,
                           StringSaver &Saver) {
   Optional<SectionRef> Plt = None;
   for (const SectionRef &Section : Obj->sections()) {
     Expected<StringRef> SecNameOrErr = Section.getName();
     if (!SecNameOrErr) {
       consumeError(SecNameOrErr.takeError());
for (const SymbolRef &Symbol : Obj->symbols()) {
     if (SecI != Obj->section_end())
       AllSymbols[*SecI].push_back(createSymbolInfo(Obj, Symbol));
     else
       AbsoluteSymbols.push_back(createSymbolInfo(Obj, Symbol));
   }
 
   if (AllSymbols.empty() && Obj->isELF())
     addDynamicElfSymbols(Obj, AllSymbols);
 
+  if (Obj->isWasm())
+    addMissingWasmCodeSymbols(cast<WasmObjectFile>(Obj), AllSymbols);
+
   BumpPtrAllocator A;
   StringSaver Saver(A);
   addPltEntries(Obj, AllSymbols, Saver);
 
   // Create a mapping from virtual address to section. An empty section can
   // cause more than one section at the same address. Sort such sections to be
   // before same-addressed non-empty sections so that symbol lookups prefer the
   // non-empty section.