[WebAssebmly] Report undefined symbols correctly in objdump

[sbc100]

Summary

Peviously we were reporting undefined symbol as being defined
by the IMPORT sections.

This change reports undefined symbols in the same that other
formats do, and also removes the need to store the section
with each symbol (since it can be derived from the symbol
type).

[dberris]

LGTM

[ncw]

This looks good, maybe you spotted it could be improved when you saw I'd thoughtlessly copied the current llvm-objdump output for the new data/global symbol split!

There will be conflicts with the symtab work, but not especially hard to resolve. Having said that, I think the conflicts and merge procedure would be rather simpler if the symtab stuff went in first (since it's a bigger thing to rebase and rejig overall).

Edit: d'oh, you merged it literally as I was typing :(

[sbc100]

In D43101#1003747, @ncw wrote:

This looks good, maybe you spotted it could be improved when you saw I'd thoughtlessly copied the current llvm-objdump output for the new data/global symbol split!

There will be conflicts with the symtab work, but not especially hard to resolve. Having said that, I think the conflicts and merge procedure would be rather simpler if the symtab stuff went in first (since it's a bigger thing to rebase and rejig overall).

Edit: d'oh, you merged it literally as I was typing :(

I've already got your patches resolved and sitting on top of this. I not sure how you feel about me taking them on, but I'm currently experimenting with them a little. If you want to rebase you can, or I can keep working on my locally rebased version. The lld-side I was thinking about is a lot more conflicting so I might land symtab before it: https://reviews.llvm.org/D43112

[ncw]

In D43101#1003800, @sbc100 wrote:

I've already got your patches resolved and sitting on top of this. I not sure how you feel about me taking them on, but I'm currently experimenting with them a little. If you want to rebase you can, or I can keep working on my locally rebased version. The lld-side I was thinking about is a lot more conflicting so I might land symtab before it: https://reviews.llvm.org/D43112

That's fantastic, OK, I'm very happy for you to take them on and modify before you merge, thanks!

[ncw]

Having said that - I do have a queue of ~ten patches built on top of the symtab work; I do have in mind a sequence of tidy-ups and various changes to reorder/rename and extend the bits that are missing. So if you're adding much to it, it may that I have a similar patch already waiting. Maybe not though; change it as you want! I've learned my lesson from the last time I tried to set up a chain of 7 reviews though, it didn't work well.

llvm/trunk/include/llvm/Object/Wasm.h

 public:
   enum class SymbolType {
     FUNCTION_IMPORT,
     FUNCTION_EXPORT,
     GLOBAL_IMPORT,
     GLOBAL_EXPORT,
   };
 
-  WasmSymbol(StringRef Name, SymbolType Type, uint32_t Section,
-             uint32_t ElementIndex, uint32_t FunctionType = 0)
-      : Name(Name), Type(Type), Section(Section), ElementIndex(ElementIndex),
+  WasmSymbol(StringRef Name, SymbolType Type, uint32_t ElementIndex,
+             uint32_t FunctionType = 0)
+      : Name(Name), Type(Type), ElementIndex(ElementIndex),
         FunctionType(FunctionType) {}
 
   StringRef Name;
   SymbolType Type;
-  uint32_t Section;
   uint32_t Flags = 0;
 
   // Index into either the function or global index space.
   uint32_t ElementIndex;
 
   // For function, the type index
   uint32_t FunctionType;
 
   // Symbols can be both exported and imported (in the case of the weakly
   // defined symbol).  In this the import index is stored as AltIndex.
   uint32_t AltIndex = 0;
   bool HasAltIndex = false;
 
   void setAltIndex(uint32_t Index) {
     HasAltIndex = true;
     AltIndex = Index;
   }
 
   bool isTypeFunction() const {
-    return Type == WasmSymbol::SymbolType::FUNCTION_IMPORT ||
-           Type == WasmSymbol::SymbolType::FUNCTION_EXPORT;
+    return Type == SymbolType::FUNCTION_IMPORT ||
+           Type == SymbolType::FUNCTION_EXPORT;
   }
 
   bool isTypeGlobal() const {
     return Type == SymbolType::GLOBAL_IMPORT ||
            Type == SymbolType::GLOBAL_EXPORT;
   }
 
 
private:
   std::vector<WasmSymbol> Symbols;
   std::vector<StringRef> Comdats;
   std::vector<wasm::WasmFunctionName> DebugNames;
   uint32_t StartFunction = -1;
   bool HasLinkingSection = false;
   wasm::WasmLinkingData LinkingData;
   uint32_t NumImportedGlobals = 0;
   uint32_t NumImportedFunctions = 0;
-  uint32_t ImportSection = 0;
-  uint32_t ExportSection = 0;
+  uint32_t CodeSection = 0;
+  uint32_t DataSection = 0;
 
   StringMap<uint32_t> SymbolMap;
 };
 
 } // end namespace object
 
 inline raw_ostream &operator<<(raw_ostream &OS,
                                const object::WasmSymbol &Sym) {
   Sym.print(OS);
   return OS;
 }
 
 } // end namespace llvm
 
 #endif // LLVM_OBJECT_WASM_H

llvm/trunk/lib/Object/WasmObjectFile.cpp

void WasmObjectFile::populateSymbolTable() {
   size_t GlobalIndex = 0;
   size_t FunctionIndex = 0;
   for (const wasm::WasmImport& Import : Imports) {
     switch (Import.Kind) {
     case wasm::WASM_EXTERNAL_GLOBAL:
       assert(Import.Global.Type == wasm::WASM_TYPE_I32);
       SymbolMap.try_emplace(Import.Field, Symbols.size());
       Symbols.emplace_back(Import.Field, WasmSymbol::SymbolType::GLOBAL_IMPORT,
-                           ImportSection, GlobalIndex++);
+                           GlobalIndex++);
       DEBUG(dbgs() << "Adding import: " << Symbols.back()
                    << " sym index:" << Symbols.size() << "\n");
       break;
     case wasm::WASM_EXTERNAL_FUNCTION:
       SymbolMap.try_emplace(Import.Field, Symbols.size());
       Symbols.emplace_back(Import.Field,
                            WasmSymbol::SymbolType::FUNCTION_IMPORT,
-                           ImportSection, FunctionIndex++, Import.SigIndex);
+                           FunctionIndex++, Import.SigIndex);
       DEBUG(dbgs() << "Adding import: " << Symbols.back()
                    << " sym index:" << Symbols.size() << "\n");
       break;
     default:
       break;
     }
   }
 
   // Add exports to symbol table
   for (const wasm::WasmExport& Export : Exports) {
     if (Export.Kind == wasm::WASM_EXTERNAL_FUNCTION ||
         Export.Kind == wasm::WASM_EXTERNAL_GLOBAL) {
       WasmSymbol::SymbolType ExportType =
           Export.Kind == wasm::WASM_EXTERNAL_FUNCTION
               ? WasmSymbol::SymbolType::FUNCTION_EXPORT
               : WasmSymbol::SymbolType::GLOBAL_EXPORT;
       auto Pair = SymbolMap.try_emplace(Export.Name, Symbols.size());
       if (Pair.second) {
-        Symbols.emplace_back(Export.Name, ExportType,
-                             ExportSection, Export.Index);
+        Symbols.emplace_back(Export.Name, ExportType, Export.Index);
         DEBUG(dbgs() << "Adding export: " << Symbols.back()
                      << " sym index:" << Symbols.size() << "\n");
       } else {
         uint32_t SymIndex = Pair.first->second;
         const WasmSymbol &OldSym = Symbols[SymIndex];
-        WasmSymbol NewSym(Export.Name, ExportType, ExportSection, Export.Index);
+        WasmSymbol NewSym(Export.Name, ExportType, Export.Index);
         NewSym.setAltIndex(OldSym.ElementIndex);
         Symbols[SymIndex] = NewSym;
 
         DEBUG(dbgs() << "Replacing existing symbol:  " << NewSym
                      << " sym index:" << SymIndex << "\n");
       }
     }
     if (Export.Kind == wasm::WASM_EXTERNAL_FUNCTION &&
Error WasmObjectFile::parseTypeSection(const uint8_t *Ptr, const uint8_t *End) {
   }
   if (Ptr != End)
     return make_error<GenericBinaryError>("Type section ended prematurely",
                                           object_error::parse_failed);
   return Error::success();
 }
 
 Error WasmObjectFile::parseImportSection(const uint8_t *Ptr, const uint8_t *End) {
-  ImportSection = Sections.size();
   uint32_t Count = readVaruint32(Ptr);
   Imports.reserve(Count);
   for (uint32_t i = 0; i < Count; i++) {
     wasm::WasmImport Im;
     Im.Module = readString(Ptr);
     Im.Field = readString(Ptr);
     Im.Kind = readUint8(Ptr);
     switch (Im.Kind) {
Error WasmObjectFile::parseGlobalSection(const uint8_t *Ptr, const uint8_t *End) {
   }
   if (Ptr != End)
     return make_error<GenericBinaryError>("Global section ended prematurely",
                                           object_error::parse_failed);
   return Error::success();
 }
 
 Error WasmObjectFile::parseExportSection(const uint8_t *Ptr, const uint8_t *End) {
-  ExportSection = Sections.size();
   uint32_t Count = readVaruint32(Ptr);
   Exports.reserve(Count);
   for (uint32_t i = 0; i < Count; i++) {
     wasm::WasmExport Ex;
     Ex.Name = readString(Ptr);
     Ex.Kind = readUint8(Ptr);
     Ex.Index = readVaruint32(Ptr);
     switch (Ex.Kind) {
Error WasmObjectFile::parseStartSection(const uint8_t *Ptr, const uint8_t *End) {
   StartFunction = readVaruint32(Ptr);
   if (!isValidFunctionIndex(StartFunction))
     return make_error<GenericBinaryError>("Invalid start function",
                                           object_error::parse_failed);
   return Error::success();
 }
 
 Error WasmObjectFile::parseCodeSection(const uint8_t *Ptr, const uint8_t *End) {
+  CodeSection = Sections.size();
   const uint8_t *CodeSectionStart = Ptr;
   uint32_t FunctionCount = readVaruint32(Ptr);
   if (FunctionCount != FunctionTypes.size()) {
     return make_error<GenericBinaryError>("Invalid function count",
                                           object_error::parse_failed);
   }
 
   while (FunctionCount--) {
Error WasmObjectFile::parseElemSection(const uint8_t *Ptr, const uint8_t *End) {
   }
   if (Ptr != End)
     return make_error<GenericBinaryError>("Elem section ended prematurely",
                                           object_error::parse_failed);
   return Error::success();
 }
 
 Error WasmObjectFile::parseDataSection(const uint8_t *Ptr, const uint8_t *End) {
+  DataSection = Sections.size();
   const uint8_t *Start = Ptr;
   uint32_t Count = readVaruint32(Ptr);
   DataSegments.reserve(Count);
   while (Count--) {
     WasmSegment Segment;
     Segment.Data.MemoryIndex = readVaruint32(Ptr);
     if (Error Err = readInitExpr(Segment.Data.Offset, Ptr))
       return Err;
WasmObjectFile::getSymbolType(DataRefImpl Symb) const {
 
   llvm_unreachable("Unknown WasmSymbol::SymbolType");
   return SymbolRef::ST_Other;
 }
 
 Expected<section_iterator>
 WasmObjectFile::getSymbolSection(DataRefImpl Symb) const {
   DataRefImpl Ref;
-  Ref.d.a = getWasmSymbol(Symb).Section;
+  const WasmSymbol& Sym = getWasmSymbol(Symb);
+  if (Sym.Type == WasmSymbol::SymbolType::GLOBAL_EXPORT)
+    Ref.d.a = DataSection;
+  else if (Sym.Type == WasmSymbol::SymbolType::FUNCTION_EXPORT)
+    Ref.d.a = CodeSection;
+  else
+    return section_end();
   return section_iterator(SectionRef(Ref, this));
 }
 
 void WasmObjectFile::moveSectionNext(DataRefImpl &Sec) const { Sec.d.a++; }
 
 std::error_code WasmObjectFile::getSectionName(DataRefImpl Sec,
                                                StringRef &Res) const {
   const WasmSection &S = Sections[Sec.d.a];

llvm/trunk/test/MC/WebAssembly/weak-alias.ll

 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:       - Index:           2
 ; CHECK-NEXT:         Name:            .data.alias_address
 ; CHECK-NEXT:         Alignment:       8
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT: ...
 
 ; CHECK-SYMS: SYMBOL TABLE:
-; CHECK-SYMS-NEXT: 00000001 gw    F EXPORT  .hidden foo_alias
-; CHECK-SYMS-NEXT: 00000000 gw      EXPORT  .hidden bar_alias
-; CHECK-SYMS-NEXT: 00000001 g     F EXPORT  .hidden foo
-; CHECK-SYMS-NEXT: 00000002 g     F EXPORT  .hidden call_direct
-; CHECK-SYMS-NEXT: 00000003 g     F EXPORT  .hidden call_alias
-; CHECK-SYMS-NEXT: 00000004 g     F EXPORT  .hidden call_direct_ptr
-; CHECK-SYMS-NEXT: 00000008 g       EXPORT  direct_address
-; CHECK-SYMS-NEXT: 00000005 g     F EXPORT  .hidden call_alias_ptr
-; CHECK-SYMS-NEXT: 00000010 g       EXPORT  alias_address
-; CHECK-SYMS-NEXT: 00000000 g       EXPORT  bar
+; CHECK-SYMS-NEXT: 00000001 gw    F CODE  .hidden foo_alias
+; CHECK-SYMS-NEXT: 00000000 gw      DATA  .hidden bar_alias
+; CHECK-SYMS-NEXT: 00000001 g     F CODE  .hidden foo
+; CHECK-SYMS-NEXT: 00000002 g     F CODE  .hidden call_direct
+; CHECK-SYMS-NEXT: 00000003 g     F CODE  .hidden call_alias
+; CHECK-SYMS-NEXT: 00000004 g     F CODE  .hidden call_direct_ptr
+; CHECK-SYMS-NEXT: 00000008 g       DATA  direct_address
+; CHECK-SYMS-NEXT: 00000005 g     F CODE  .hidden call_alias_ptr
+; CHECK-SYMS-NEXT: 00000010 g       DATA  alias_address
+; CHECK-SYMS-NEXT: 00000000 g       DATA  bar

llvm/trunk/test/tools/llvm-objdump/WebAssembly/symbol-table.test

 RUN: llvm-objdump -t %p/../Inputs/trivial.obj.wasm | FileCheck %s
 
 CHECK:      SYMBOL TABLE:
-CHECK-NEXT: 00000000 g     F IMPORT puts
-CHECK-NEXT: 00000000 g     F IMPORT SomeOtherFunction
-CHECK-NEXT: 00000002 g     F EXPORT main
-CHECK-NEXT: 00000010 g       EXPORT var
+CHECK-NEXT: 00000000 g     F *UND*  puts
+CHECK-NEXT: 00000000 g     F *UND*  SomeOtherFunction
+CHECK-NEXT: 00000002 g     F CODE main
+CHECK-NEXT: 00000010 g       DATA var