[WebAssembly] Don't duplicate strings in SYM_INFO subsection

[sbc100]

Summary

Instead reference symbols by type+index.

This change is based on D42117, but doesn't depend on any other patches.

Fixes: https://github.com/WebAssembly/tool-conventions/issues/30

[sbc100]

• revert

[sbc100]

• revert

[ncw]

This is really doubling-down on the current import-and-export strategy!

My concern now is #5 rebased on top of this will be larger, and have more test changes. Now that syminfo depends on the duplicate-imports, when they're taken away the diff is going to be that much bigger. I just don't see the incentive to merge #6 first, when my goal is to keep the diff to an absolute minimum in commits like #5 which are "contentful" changes. The order of changes I'd come up was based on trying to minimise the number of times the tests would have to be updated (to reduce the chance of masking a bug by updating the same things repeatedly).

This change is using/abusing the Wasm indexes in the same way as my #2, but #2 really had to land before #5, whereas this is just a cosmetic change to reduce filesize, and doesn't really need to.

Hmm. It looks like it should work, so I guess your order is OK. Thanks for helping me land the changes!

[ncw]

include/llvm/Object/Wasm.h

268

It's a shame we need two maps now! I think it my patch order, we were able to change the map's type in one commit, then remove it in the next.

lib/MC/WasmObjectWriter.cpp

1045

You've dropped a if (Flags != 0) check.

[ncw]

Oh, having seen the LLD changes I'm torn about whether it's good to land this one first or not.

What it really comes down to is: are you planning to land #5 or not? All the other changes are based around #5, and if that one doesn't go in, I'll be sad, because it's really the main change I'm trying to achieve to with the whole patch chain. Having a single unique index per Symbol, and a single unique import/export per Symbol actually enables these changes to refer to Symbols by index.

Unless you are planning to land #5 subsequently (or something else that does the same job), I'd be uneasy landing #6 at all - personally.

[sbc100]

In D42279#981492, @ncw wrote:

Oh, having seen the LLD changes I'm torn about whether it's good to land this one first or not.

What it really comes down to is: are you planning to land #5 or not? All the other changes are based around #5, and if that one doesn't go in, I'll be sad, because it's really the main change I'm trying to achieve to with the whole patch chain. Having a single unique index per Symbol, and a single unique import/export per Symbol actually enables these changes to refer to Symbols by index.

Unless you are planning to land #5 subsequently (or something else that does the same job), I'd be uneasy landing #6 at all - personally.

I'm still not sure about landing #5. The goal of this patch was to address issue 30 but it should also make you #5 patch simply perhaps?

Not that I think about it though, maybe it time to create a prototype of my "full symbol table" idea so that it can be compared the #5 change.

Perhaps landing this breaking change in the interim is not such a good idea.

[ncw]

Yeah, I can see why it's scary, because it involves changing the way things are actually modelled. But I find it easier to reason about the linker once #5 has landed.

We have Symbols, written out by Clang (WasmObjectWriter) and LLD (with --relocatable), and in Clang we have the duplicated-import hack where some symbols are written as both an import and an export - but LLD doesn't write out relocatable objects that hack, so there's disagreement between the tools, so we can't compare the outputs to check that "linking" a single object file with --relocatable leaves it unchanged.

And then we have to have special code in LLD (WasmObjectFile) when it reads in the object files to recover the symbol table from the imports and exports - but that's not documented in Linking.md and it doesn't make it clear how the indexes are actually worked out unless I think hard about it. And so Symbols don't have a unique index that's easy to reason about.

It comes down to this confusion between indexes into the array of functions and indexes into the array of symbols - but the symbols in a C file don't match the function bodies in a C file, it's just a lucky fluke for simple files. Once we cleanly separate the concept of function/global bodies from the concept of symbols, the linking model (should) become simpler.

This change is switching from referencing symbols by name to referencing them by index, but we haven't got a straightforward concept yet of symbol index. It's a shame to effectively increase the number of places where we're abusing symbols, which drives up the number of lines of diff in #5 (if it lands after this).

To me, "don't duplicate symbol strings" is very low priority, it just reduces filesize a bit. But getting the concepts straight should benefit the codebase throughout.

That's assuming that #5 does actually conceptually improve things. But #5 isn't even my idea! Having each Symbol represented by one import or export is how it was supposed to be, it's simply that the relocation indexes weren't being calculated correctly - in a sense #5 is conservative change that's just trying to get back to the original, simple, model.

include/llvm/Object/Wasm.h

 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_OBJECT_WASM_H
 #define LLVM_OBJECT_WASM_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <cstddef>
 #include <cstdint>
 #include <vector>
private:
   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;
 
-  StringMap<uint32_t> SymbolMap;
+  // Map symbol name -> symbol index
+  StringMap<uint32_t> SymbolNames;
+  // Map pair(kind,index) -> symbol index
+  DenseMap<std::pair<uint32_t, uint32_t>, 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

include/llvm/ObjectYAML/WasmYAML.h

 struct Signature {
   uint32_t Index;
   SignatureForm Form = wasm::WASM_TYPE_FUNC;
   std::vector<ValueType> ParamTypes;
   ValueType ReturnType;
 };
 
 struct SymbolInfo {
-  StringRef Name;
+  ExportKind Kind;
+  uint32_t Index;
   SymbolFlags Flags;
 };
 
 struct InitFunction {
   uint32_t Priority;
   uint32_t FunctionIndex;
 };
 

lib/MC/WasmObjectWriter.cpp

 // Information about a single item which is part of a COMDAT.  For each data
 // segment or function which is in the COMDAT, there is a corresponding
 // WasmComdatEntry.
 struct WasmComdatEntry {
   unsigned Kind;
   uint32_t Index;
 };
 
+// Information about a single symbol to be written into the linking metadata
+struct WasmSymInfo {
+  uint32_t Kind;
+  uint32_t Index;
+  uint32_t Flags;
+};
+
 // Information about a single relocation.
 struct WasmRelocationEntry {
   uint64_t Offset;                  // Where is the relocation.
   const MCSymbolWasm *Symbol;       // The symbol to relocate with.
   int64_t Addend;                   // A value to add to the symbol.
   unsigned Type;                    // The type of the relocation.
   const MCSectionWasm *FixupSection;// The section the relocation is targeting.
 
void writeCodeSection(const MCAssembler &Asm, const MCAsmLayout &Layout,
                         ArrayRef<WasmFunction> Functions);
   void writeDataSection(ArrayRef<WasmDataSegment> Segments);
   void writeNameSection(ArrayRef<WasmFunction> Functions,
                         ArrayRef<WasmImport> Imports);
   void writeCodeRelocSection();
   void writeDataRelocSection();
   void writeLinkingMetaDataSection(
       ArrayRef<WasmDataSegment> Segments, uint32_t DataSize,
-      ArrayRef<std::pair<StringRef, uint32_t>> SymbolFlags,
+      ArrayRef<WasmSymInfo> SymbolInfo,
       ArrayRef<std::pair<uint16_t, uint32_t>> InitFuncs,
       const std::map<StringRef, std::vector<WasmComdatEntry>>& Comdats);
 
   uint32_t getProvisionalValue(const WasmRelocationEntry &RelEntry);
   void applyRelocations(ArrayRef<WasmRelocationEntry> Relocations,
                         uint64_t ContentsOffset);
 
   void writeRelocations(ArrayRef<WasmRelocationEntry> Relocations);
void WasmObjectWriter::writeDataRelocSection() {
 
   writeRelocations(DataRelocations);
 
   endSection(Section);
 }
 
 void WasmObjectWriter::writeLinkingMetaDataSection(
     ArrayRef<WasmDataSegment> Segments, uint32_t DataSize,
-    ArrayRef<std::pair<StringRef, uint32_t>> SymbolFlags,
+    ArrayRef<WasmSymInfo> SymbolInfo,
     ArrayRef<std::pair<uint16_t, uint32_t>> InitFuncs,
     const std::map<StringRef, std::vector<WasmComdatEntry>>& Comdats) {
   SectionBookkeeping Section;
   startSection(Section, wasm::WASM_SEC_CUSTOM, "linking");
   SectionBookkeeping SubSection;
 
-  if (SymbolFlags.size() != 0) {
+  if (!SymbolInfo.empty()) {
     startSection(SubSection, wasm::WASM_SYMBOL_INFO);
-    encodeULEB128(SymbolFlags.size(), getStream());
-    for (auto Pair: SymbolFlags) {
-      writeString(Pair.first);
-      encodeULEB128(Pair.second, getStream());
+    encodeULEB128(SymbolInfo.size(), getStream());
+    for (const auto &SI : SymbolInfo) {
+      encodeULEB128(SI.Kind, getStream());
+      encodeULEB128(SI.Index, getStream());
+      encodeULEB128(SI.Flags, getStream());
     }
     endSection(SubSection);
   }
 
   if (DataSize > 0) {
     startSection(SubSection, wasm::WASM_DATA_SIZE);
     encodeULEB128(DataSize, getStream());
     endSection(SubSection);
void WasmObjectWriter::writeObject(MCAssembler &Asm,
   MCContext &Ctx = Asm.getContext();
   wasm::ValType PtrType = is64Bit() ? wasm::ValType::I64 : wasm::ValType::I32;
 
   // Collect information from the available symbols.
   SmallVector<WasmFunction, 4> Functions;
   SmallVector<uint32_t, 4> TableElems;
   SmallVector<WasmImport, 4> Imports;
   SmallVector<WasmExport, 4> Exports;
-  SmallVector<std::pair<StringRef, uint32_t>, 4> SymbolFlags;
+  SmallVector<WasmSymInfo, 4> SymbolInfo;
   SmallVector<std::pair<uint16_t, uint32_t>, 2> InitFuncs;
   std::map<StringRef, std::vector<WasmComdatEntry>> Comdats;
   SmallVector<WasmDataSegment, 4> DataSegments;
   uint32_t DataSize = 0;
 
+  auto SetIndex = [&](const MCSymbolWasm &WS, uint32_t Index) {
+    SymbolIndices[&WS] = Index;
+    uint32_t Kind = WS.isFunction() ? wasm::WASM_EXTERNAL_FUNCTION
+                                    : wasm::WASM_EXTERNAL_GLOBAL;
+    uint32_t Flags = WS.isWeak() ? wasm::WASM_SYMBOL_BINDING_WEAK : 0;
+    Flags |= WS.isHidden() ? wasm::WASM_SYMBOL_VISIBILITY_HIDDEN : 0;
+    Flags |= !WS.isExternal() && WS.isDefined()
+                 ? wasm::WASM_SYMBOL_BINDING_LOCAL
+                 : 0;
+    SymbolInfo.emplace_back(WasmSymInfo{Kind, Index, Flags});
+  };
+
   // In the special .global_variables section, we've encoded global
   // variables used by the function. Translate them into the Globals
   // list.
   MCSectionWasm *GlobalVars =
       Ctx.getWasmSection(".global_variables", SectionKind::getMetadata());
   if (!GlobalVars->getFragmentList().empty()) {
     if (GlobalVars->getFragmentList().size() != 1)
       report_fatal_error("only one .global_variables fragment supported");
if ((!WS.isDefined() && !WS.isComdat()) ||
         WS.isVariable()) {
       WasmImport Import;
       Import.ModuleName = WS.getModuleName();
       Import.FieldName = WS.getName();
 
       if (WS.isFunction()) {
         Import.Kind = wasm::WASM_EXTERNAL_FUNCTION;
         Import.Type = getFunctionType(WS);
-        SymbolIndices[&WS] = NumFunctionImports;
+        SetIndex(WS, NumFunctionImports);
         ++NumFunctionImports;
       } else {
         Import.Kind = wasm::WASM_EXTERNAL_GLOBAL;
         Import.Type = int32_t(PtrType);
         Import.IsMutable = false;
-        SymbolIndices[&WS] = NumGlobalImports;
+        SetIndex(WS, NumGlobalImports);
 
         // If this global is the stack pointer, make it mutable.
         if (WS.getName() == "__stack_pointer")
           Import.IsMutable = true;
 
         ++NumGlobalImports;
       }
 
for (const MCSymbol &S : Asm.symbols()) {
     DEBUG(dbgs() << "MCSymbol: '" << S << "'"
                  << " isDefined=" << S.isDefined() << " isExternal="
                  << S.isExternal() << " isTemporary=" << S.isTemporary()
                  << " isFunction=" << WS.isFunction()
                  << " isWeak=" << WS.isWeak()
                  << " isHidden=" << WS.isHidden()
                  << " isVariable=" << WS.isVariable() << "\n");
 
-    if (WS.isWeak() || WS.isHidden()) {
-      uint32_t Flags = (WS.isWeak() ? wasm::WASM_SYMBOL_BINDING_WEAK : 0) |
-          (WS.isHidden() ? wasm::WASM_SYMBOL_VISIBILITY_HIDDEN : 0);
-      SymbolFlags.emplace_back(WS.getName(), Flags);
-    }
-
     if (WS.isVariable())
       continue;
 
     unsigned Index;
 
     if (WS.isFunction()) {
       if (WS.isDefined()) {
         if (WS.getOffset() != 0)
           report_fatal_error(
               "function sections must contain one function each");
 
         if (WS.getSize() == 0)
           report_fatal_error(
               "function symbols must have a size set with .size");
 
         // A definition. Take the next available index.
         Index = NumFunctionImports + Functions.size();
 
         // Prepare the function.
         WasmFunction Func;
         Func.Type = getFunctionType(WS);
         Func.Sym = &WS;
-        SymbolIndices[&WS] = Index;
+        SetIndex(WS, Index);
         Functions.push_back(Func);
       } else {
         // An import; the index was assigned above.
         Index = SymbolIndices.find(&WS)->second;
       }
 
       DEBUG(dbgs() << "  -> function index: " << Index << "\n");
    } else {
if (WS.isFunction()) {
       assert(DataSection.isWasmData());
 
       WasmGlobal Global;
       Global.Type = PtrType;
       Global.IsMutable = false;
       Global.HasImport = false;
       Global.InitialValue = DataSection.getMemoryOffset() + Layout.getSymbolOffset(WS);
       Global.ImportIndex = 0;
-      SymbolIndices[&WS] = Index;
+      SetIndex(WS, Index);
       DEBUG(dbgs() << "  -> global index: " << Index << "\n");
       Globals.push_back(Global);
     }
 
     // If the symbol is visible outside this translation unit, export it.
     if (WS.isDefined()) {
       WasmExport Export;
       Export.FieldName = WS.getName();
       Export.Index = Index;
       if (WS.isFunction())
         Export.Kind = wasm::WASM_EXTERNAL_FUNCTION;
       else
         Export.Kind = wasm::WASM_EXTERNAL_GLOBAL;
       DEBUG(dbgs() << "  -> export " << Exports.size() << "\n");
       Exports.push_back(Export);
 
-      if (!WS.isExternal())
-        SymbolFlags.emplace_back(WS.getName(), wasm::WASM_SYMBOL_BINDING_LOCAL);
-
       if (WS.isFunction()) {
         auto &Section = static_cast<MCSectionWasm &>(WS.getSection());
         if (const MCSymbolWasm *C = Section.getGroup())
           Comdats[C->getName()].emplace_back(
               WasmComdatEntry{wasm::WASM_COMDAT_FUNCTION, Index});
       }
     }
   }
for (const MCSymbol &S : Asm.symbols()) {
     Export.FieldName = WS.getName();
     Export.Index = Index;
     if (WS.isFunction())
       Export.Kind = wasm::WASM_EXTERNAL_FUNCTION;
     else
       Export.Kind = wasm::WASM_EXTERNAL_GLOBAL;
     DEBUG(dbgs() << "  -> export " << Exports.size() << "\n");
     Exports.push_back(Export);
-
-    if (!WS.isExternal())
-      SymbolFlags.emplace_back(WS.getName(), wasm::WASM_SYMBOL_BINDING_LOCAL);
   }
 
   {
     auto HandleReloc = [&](const WasmRelocationEntry &Rel) {
       // Functions referenced by a relocation need to prepared to be called
       // indirectly.
       const MCSymbolWasm& WS = *Rel.Symbol;
       if (WS.isFunction() && IndirectSymbolIndices.count(&WS) == 0) {
void WasmObjectWriter::writeObject(MCAssembler &Asm,
   writeGlobalSection();
   writeExportSection(Exports);
   writeElemSection(TableElems);
   writeCodeSection(Asm, Layout, Functions);
   writeDataSection(DataSegments);
   writeNameSection(Functions, Imports);
   writeCodeRelocSection();
   writeDataRelocSection();
-  writeLinkingMetaDataSection(DataSegments, DataSize, SymbolFlags,
-                              InitFuncs, Comdats);
+  writeLinkingMetaDataSection(DataSegments, DataSize, SymbolInfo, InitFuncs,
+                              Comdats);
 
   // TODO: Translate the .comment section to the output.
   // TODO: Translate debug sections to the output.
 }
 
 std::unique_ptr<MCObjectWriter>
 llvm::createWasmObjectWriter(std::unique_ptr<MCWasmObjectTargetWriter> MOTW,
                              raw_pwrite_stream &OS) {
   return llvm::make_unique<WasmObjectWriter>(std::move(MOTW), OS);
 }

lib/Object/WasmObjectFile.cpp

 void WasmObjectFile::populateSymbolTable() {
   // Add imports to symbol table
   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());
+      SymbolNames.try_emplace(Import.Field, Symbols.size());
+      SymbolMap.try_emplace({Import.Kind, GlobalIndex}, Symbols.size());
       Symbols.emplace_back(Import.Field, WasmSymbol::SymbolType::GLOBAL_IMPORT,
                            ImportSection, 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());
+      SymbolNames.try_emplace(Import.Field, Symbols.size());
+      SymbolMap.try_emplace({Import.Kind, FunctionIndex}, Symbols.size());
       Symbols.emplace_back(Import.Field,
                            WasmSymbol::SymbolType::FUNCTION_IMPORT,
                            ImportSection, 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());
+      auto Pair = SymbolNames.try_emplace(Export.Name, Symbols.size());
       if (Pair.second) {
+        SymbolMap.try_emplace({Export.Kind, Export.Index}, Symbols.size());
         Symbols.emplace_back(Export.Name, ExportType,
                              ExportSection, 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);
         NewSym.setAltIndex(OldSym.ElementIndex);
         Symbols[SymIndex] = NewSym;
+        SymbolMap.try_emplace({Export.Kind, Export.Index}, SymIndex);
 
         DEBUG(dbgs() << "Replacing existing symbol:  " << NewSym
                      << " sym index:" << SymIndex << "\n");
       }
     }
     if (Export.Kind == wasm::WASM_EXTERNAL_FUNCTION) {
       auto &Function = Functions[Export.Index - NumImportedFunctions];
       if (Function.Name.empty()) {
Error WasmObjectFile::parseLinkingSection(const uint8_t *Ptr,
   while (Ptr < End) {
     uint8_t Type = readVarint7(Ptr);
     uint32_t Size = readVaruint32(Ptr);
     const uint8_t *SubSectionEnd = Ptr + Size;
     switch (Type) {
     case wasm::WASM_SYMBOL_INFO: {
       uint32_t Count = readVaruint32(Ptr);
       while (Count--) {
-        StringRef Symbol = readString(Ptr);
+        uint32_t Kind = readVaruint32(Ptr);
+        uint32_t Index = readVaruint32(Ptr);
         uint32_t Flags = readVaruint32(Ptr);
-        auto iter = SymbolMap.find(Symbol);
+        auto iter = SymbolMap.find({Kind, Index});
         if (iter == SymbolMap.end()) {
           return make_error<GenericBinaryError>(
-              "Invalid symbol name in linking section: " + Symbol,
+              "Invalid symbol in SYM_INFO: " + Twine(Kind) + ":" + Twine(Index),
               object_error::parse_failed);
         }
         uint32_t SymIndex = iter->second;
         assert(SymIndex < Symbols.size());
         Symbols[SymIndex].Flags = Flags;
-        DEBUG(dbgs() << "Set symbol flags index:"
-                     << SymIndex << " name:"
-                     << Symbols[SymIndex].Name << " expected:"
-                     << Symbol << " flags: " << Flags << "\n");
+        DEBUG(dbgs() << "Set symbol flags index:" << SymIndex << " name:"
+                     << Symbols[SymIndex].Name << " flags: " << Flags << "\n");
       }
       break;
     }
     case wasm::WASM_DATA_SIZE:
       LinkingData.DataSize = readVaruint32(Ptr);
       break;
     case wasm::WASM_SEGMENT_INFO: {
       uint32_t Count = readVaruint32(Ptr);

lib/ObjectYAML/WasmYAML.cpp

 void MappingTraits<WasmYAML::Comdat>::mapping(
     IO &IO, WasmYAML::Comdat &Comdat) {
   IO.mapRequired("Name", Comdat.Name);
   IO.mapRequired("Entries", Comdat.Entries);
 }
 
 void MappingTraits<WasmYAML::SymbolInfo>::mapping(IO &IO,
                                                   WasmYAML::SymbolInfo &Info) {
-  IO.mapRequired("Name", Info.Name);
+  IO.mapRequired("Kind", Info.Kind);
+  IO.mapRequired("Index", Info.Index);
   IO.mapRequired("Flags", Info.Flags);
 }
 
 void ScalarBitSetTraits<WasmYAML::LimitFlags>::bitset(
     IO &IO, WasmYAML::LimitFlags &Value) {
 #define BCase(X) IO.bitSetCase(Value, #X, wasm::WASM_LIMITS_FLAG_##X)
   BCase(HAS_MAX);
 #undef BCase

test/MC/WebAssembly/array-fill.ll

 
 ; CHECK:        - Type:            DATA
 ; CHECK:              Content:         '0102'
 
 ; CHECK:        - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        2
 ; CHECK-NEXT:     SymbolInfo:      
-; CHECK-NEXT:       - Name:            gBd
+; CHECK-NEXT:       - Kind:            GLOBAL
+; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; CHECK-NEXT:     SegmentInfo:    
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            .data
 ; CHECK-NEXT:         Alignment:       1
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT: ...

test/MC/WebAssembly/comdat.ll

 ; CHECK-NEXT:      - Index:           2
 ; CHECK-NEXT:        Name:            basicInlineFn
 ; CHECK-NEXT:      - Index:           3
 ; CHECK-NEXT:        Name:            sharedFn
 ; CHECK-NEXT:  - Type:            CUSTOM
 ; CHECK-NEXT:    Name:            linking
 ; CHECK-NEXT:    DataSize:        3
 ; CHECK-NEXT:    SymbolInfo:
-; CHECK-NEXT:      - Name:            basicInlineFn
+; CHECK-NEXT:      - Kind:            FUNCTION
+; CHECK-NEXT:        Index:           2
 ; CHECK-NEXT:        Flags:           [ BINDING_WEAK ]
-; CHECK-NEXT:      - Name:            sharedFn
+; CHECK-NEXT:      - Kind:            FUNCTION
+; CHECK-NEXT:        Index:           3
 ; CHECK-NEXT:        Flags:           [ BINDING_WEAK ]
-; CHECK-NEXT:      - Name:            constantData
+; CHECK-NEXT:      - Kind:            GLOBAL
+; CHECK-NEXT:        Index:           0
 ; CHECK-NEXT:        Flags:           [ BINDING_WEAK ]
 ; CHECK-NEXT:    SegmentInfo:
 ; CHECK-NEXT:      - Index:           0
 ; CHECK-NEXT:        Name:            .rodata.constantData
 ; CHECK-NEXT:        Alignment:       1
 ; CHECK-NEXT:        Flags:           [  ]
 ; CHECK-NEXT:    Comdats:
 ; CHECK-NEXT:      - Name:            basicInlineFn

test/MC/WebAssembly/global-ctor-dtor.ll

 ; CHECK-NEXT:       - Index:           7
 ; CHECK-NEXT:         Name:            .Lcall_dtors
 ; CHECK-NEXT:       - Index:           8
 ; CHECK-NEXT:         Name:            .Lregister_call_dtors
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        4
 ; CHECK-NEXT:     SymbolInfo:      
-; CHECK-NEXT:       - Name:            __dso_handle
+; CHECK-NEXT:       - Kind:            GLOBAL
+; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Flags:           [ BINDING_WEAK, VISIBILITY_HIDDEN ]
-; CHECK-NEXT:       - Name:            .Lcall_dtors.42
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           5
 ; CHECK-NEXT:         Flags:           [ BINDING_LOCAL ]
-; CHECK-NEXT:       - Name:            .Lregister_call_dtors.42
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           6
 ; CHECK-NEXT:         Flags:           [ BINDING_LOCAL ]
-; CHECK-NEXT:       - Name:            .Lcall_dtors
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           7
 ; CHECK-NEXT:         Flags:           [ BINDING_LOCAL ]
-; CHECK-NEXT:       - Name:            .Lregister_call_dtors
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           8
 ; CHECK-NEXT:         Flags:           [ BINDING_LOCAL ]
 ; CHECK-NEXT:     SegmentInfo:     
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            .data.global1
 ; CHECK-NEXT:         Alignment:       8
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:     InitFunctions:     
 ; CHECK-NEXT:       - Priority: 42
 ; CHECK-NEXT:         FunctionIndex: 3
 ; CHECK-NEXT:       - Priority: 42
 ; CHECK-NEXT:         FunctionIndex: 6
 ; CHECK-NEXT:       - Priority: 65535
 ; CHECK-NEXT:         FunctionIndex: 4
 ; CHECK-NEXT:       - Priority: 65535
 ; CHECK-NEXT:         FunctionIndex: 8
 ; CHECK-NEXT: ...

test/MC/WebAssembly/unnamed-data.ll

 ; CHECK-NEXT:         Offset:          
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           24
 ; CHECK-NEXT:         Content:         '06000000'
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        28
 ; CHECK-NEXT:     SymbolInfo:      
-; CHECK-NEXT:       - Name:            .L.str1
+; CHECK-NEXT:       - Kind:            GLOBAL
+; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Flags:           [ BINDING_LOCAL ]
-; CHECK-NEXT:       - Name:            .L.str2
+; CHECK-NEXT:       - Kind:            GLOBAL
+; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:         Flags:           [ BINDING_LOCAL ]
 ; CHECK-NEXT:     SegmentInfo:    
 ; CHECK-NEXT:       - Index:       0
 ; CHECK-NEXT:         Name:        .rodata..L.str1
 ; CHECK-NEXT:         Alignment:   1
 ; CHECK-NEXT:         Flags:       [ ]
 ; CHECK-NEXT:       - Index:       1
 ; CHECK-NEXT:         Name:        .rodata..L.str2

test/MC/WebAssembly/visibility.ll

 }
 
 ; CHECK:        - Type:            CUSTOM
 
 ; CHECK:        - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        0
 ; CHECK-NEXT:     SymbolInfo:
-; CHECK-NEXT:       - Name:            hiddenVis
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; CHECK-NEXT: ...

test/MC/WebAssembly/weak-alias.ll

 ; CHECK-NEXT:       - Index:           4
 ; CHECK-NEXT:         Name:            call_direct_ptr
 ; CHECK-NEXT:       - Index:           5
 ; CHECK-NEXT:         Name:            call_alias_ptr
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        20
 ; CHECK-NEXT:     SymbolInfo:      
-; CHECK-NEXT:       - Name:            foo_alias
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:         Flags:           [ BINDING_WEAK, VISIBILITY_HIDDEN ]
-; CHECK-NEXT:       - Name:            bar_alias
+; CHECK-NEXT:       - Kind:            GLOBAL
+; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:         Flags:           [ BINDING_WEAK, VISIBILITY_HIDDEN ]
-; CHECK-NEXT:       - Name:            foo
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           1
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
-; CHECK-NEXT:       - Name:            call_direct
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
-; CHECK-NEXT:       - Name:            call_alias
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           3
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
-; CHECK-NEXT:       - Name:            call_direct_ptr
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           4
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
-; CHECK-NEXT:       - Name:            call_alias_ptr
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           5
 ; CHECK-NEXT:         Flags:           [ VISIBILITY_HIDDEN ]
 ; CHECK-NEXT:     SegmentInfo:    
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            .data.bar
 ; CHECK-NEXT:         Alignment:       8
 ; CHECK-NEXT:         Flags:           [ ]
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Name:            .data.direct_address

test/MC/WebAssembly/weak.ll

 ; CHECK-NEXT:     Name:            name
 ; CHECK-NEXT:     FunctionNames:   
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            weak_function
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        0
 ; CHECK-NEXT:     SymbolInfo:      
-; CHECK-NEXT:       - Name:            weak_external_data
+; CHECK-NEXT:       - Kind:            GLOBAL
+; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Flags:           [ BINDING_WEAK ]
-; CHECK-NEXT:       - Name:            weak_function
+; CHECK-NEXT:       - Kind:            FUNCTION
+; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Flags:           [ BINDING_WEAK, VISIBILITY_HIDDEN ]
 ; CHECK-NEXT: ...

test/ObjectYAML/wasm/linking_section.yaml

Segments:
         Offset:
           Opcode:          I32_CONST
           Value:           8
         Content:         '11110000'
   - Type:            CUSTOM
     Name:            linking
     DataSize:        999
     SymbolInfo:
-      - Name:            bar
+      - Kind:            FUNCTION
+        Index:           0
         Flags:           [ BINDING_WEAK ]
     SegmentInfo:
       - Index:       0
         Alignment:   4
         Flags:       [ ]
         Name:        mydata
       - Index:       1
         Alignment:   2
         Flags:       [ ]
         Name:        moredata
     InitFunctions:
       - Priority:      1
         FunctionIndex: 0
 ...
 # CHECK:       - Type:            CUSTOM
 # CHECK-NEXT:    Name:            linking
 # CHECK-NEXT:    DataSize:        999
 # CHECK-NEXT:    SymbolInfo:
-# CHECK-NEXT:      - Name:            bar
+# CHECK-NEXT:      - Kind:            FUNCTION
+# CHECK-NEXT:        Index:           0
 # CHECK-NEXT:        Flags:           [ BINDING_WEAK ]
 # CHECK-NEXT:    SegmentInfo:
 # CHECK-NEXT:      - Index:           0
 # CHECK-NEXT:        Name:            mydata
 # CHECK-NEXT:        Alignment:       4
 # CHECK-NEXT:        Flags:           [ ]
 # CHECK-NEXT:      - Index:           1
 # CHECK-NEXT:        Name:            moredata
 # CHECK-NEXT:        Alignment:       2
 # CHECK-NEXT:        Flags:           [ ]
 # CHECK-NEXT:    InitFunctions:
 # CHECK-NEXT:       - Priority:       1
 # CHECK-NEXT:         FunctionIndex:  0
 # CHECK-NEXT: ...

test/ObjectYAML/wasm/weak_symbols.yaml

Functions:
         Body:            00
       - Index:           1
         Locals:
         Body:            00
   - Type:            CUSTOM
     Name:            linking
     DataSize:        10
     SymbolInfo:
-      - Name:            function_export
+      - Kind:            FUNCTION
+        Index:           1
         Flags:           [ BINDING_WEAK ]
-      - Name:            global_export
+      - Kind:            GLOBAL
+        Index:           0
         Flags:           [ BINDING_WEAK ]
 ...
 # CHECK: --- !WASM
 # CHECK: FileHeader:
 # CHECK:   Version:           0x00000001
 # CHECK: Sections:
 # CHECK:   - Type:            EXPORT
 # CHECK:     Exports:         
 # CHECK:       - Name:            function_export
 # CHECK:         Kind:            FUNCTION
 # CHECK:         Index:           1
 # CHECK:       - Name:            global_export
 # CHECK:         Kind:            GLOBAL
 # CHECK:         Index:           0
 # CHECK:   - Type:            CUSTOM
 # CHECK:     Name:            linking
 # CHECK:     DataSize:        10
 # CHECK:     SymbolInfo:
-# CHECK:       - Name:            function_export
+# CHECK:       - Kind:            FUNCTION
+# CHECK:         Index:           1
 # CHECK:         Flags:           [ BINDING_WEAK ]
-# CHECK:       - Name:            global_export
+# CHECK:       - Kind:            GLOBAL
+# CHECK:         Index:           0
 # CHECK:         Flags:           [ BINDING_WEAK ]

test/tools/llvm-nm/wasm/weak-symbols.yaml

Globals:
         Mutable:     false
         InitExpr:
           Opcode:          I32_CONST
           Value:           1024
   - Type:            EXPORT
     Exports:
       - Name:            weak_global_func
         Kind:            FUNCTION
-        Index:           0x00000004
+        Index:           4
       - Name:            weak_global_data
         Kind:            GLOBAL
-        Index:           0x00000003
+        Index:           3
   - Type:            CODE
     Functions:
       - Index:           1
         Locals:
         Body:            00
       - Index:           2
         Locals:
         Body:            00
       - Index:           3
         Locals:
         Body:            00
       - Index:           4
         Locals:
         Body:            00
   - Type:            CUSTOM
     Name:            linking
     DataSize:        0
     SymbolInfo:
-      - Name:            weak_global_func
+      - Kind:            FUNCTION
+        Index:           4
         Flags:           [ BINDING_WEAK ]
-      - Name:            weak_global_data
+      - Kind:            GLOBAL
+        Index:           3
         Flags:           [ BINDING_WEAK ]
-      - Name:            weak_import_func
+      - Kind:            FUNCTION
+        Index:           0
         Flags:           [ BINDING_WEAK ]
-      - Name:            weak_import_data
+      - Kind:            GLOBAL
+        Index:           0
         Flags:           [ BINDING_WEAK ]
 
 # CHECK: 00000400 W weak_global_data
 # CHECK: 00000004 W weak_global_func
 # CHECK:          w weak_import_data
 # CHECK:          w weak_import_func

tools/obj2yaml/wasm2yaml.cpp

for (const object::WasmSegment &Segment : Obj.dataSegments()) {
         Comdat.Entries.emplace_back(
             WasmYAML::ComdatEntry{wasm::WASM_COMDAT_DATA, SegmentIndex});
       }
       SegmentIndex++;
     }
     for (const object::SymbolRef& Sym: Obj.symbols()) {
       const object::WasmSymbol Symbol = Obj.getWasmSymbol(Sym);
       if (Symbol.Flags != 0) {
-        WasmYAML::SymbolInfo Info{Symbol.Name, Symbol.Flags};
+        WasmYAML::SymbolInfo Info{Symbol.isFunction()
+                                      ? wasm::WASM_EXTERNAL_FUNCTION
+                                      : wasm::WASM_EXTERNAL_GLOBAL,
+                                  Symbol.ElementIndex, Symbol.Flags};
         LinkingSec->SymbolInfos.emplace_back(Info);
       }
     }
     LinkingSec->DataSize = Obj.linkingData().DataSize;
     for (const wasm::WasmInitFunc &Func : Obj.linkingData().InitFunctions) {
       WasmYAML::InitFunction F{Func.Priority, Func.FunctionIndex};
       LinkingSec->InitFunctions.emplace_back(F);
     }

tools/yaml2obj/yaml2wasm.cpp

int WasmWriter::writeSectionContent(raw_ostream &OS, WasmYAML::LinkingSection &Section) {
   SubSection.Done();
 
   // SYMBOL_INFO subsection
   if (Section.SymbolInfos.size()) {
     encodeULEB128(wasm::WASM_SYMBOL_INFO, OS);
 
     encodeULEB128(Section.SymbolInfos.size(), SubSection.GetStream());
     for (const WasmYAML::SymbolInfo &Info : Section.SymbolInfos) {
-      writeStringRef(Info.Name, SubSection.GetStream());
+      encodeULEB128(Info.Kind, SubSection.GetStream());
+      encodeULEB128(Info.Index, SubSection.GetStream());
       encodeULEB128(Info.Flags, SubSection.GetStream());
     }
 
     SubSection.Done();
   }
 
   // SEGMENT_NAMES subsection
   if (Section.SegmentInfos.size()) {