[WebAssembly] Write out undefined symbol addresses as zero

[sbc100]

Summary

The addresses of undefined symbols that make it into the final
executable (i.e. weak references to non-existant symbols) should
resolve to zero.

Also, make sure to not include function in the indirect function
table if they are not included in the output.

[ruiu]

Resolving undefined symbols to address zero seems reasonable, but ignoring addends for such symbols seems a bit odd. What is the rationale for doing that?

[sbc100]

• don't ignore addends

[ruiu]

LGTM

[sbc100]

You're right. Not sure what I thought that was necessary.

test/wasm/relocatable.ll

 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           12
 ; CHECK-NEXT:         Content:         '01000000'
 ; CHECK-NEXT:       - SectionOffset:   36
 ; CHECK-NEXT:         MemoryIndex:     0
 ; CHECK-NEXT:         Offset:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           16
-; CHECK-NEXT:         Content:         FFFFFFFF
+; CHECK-NEXT:         Content:         '00000000'
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        20
 ; CHECK-NEXT:     SegmentInfo:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         Name:            .rodata.hello_str
 ; CHECK-NEXT:         Alignment:       1
 ; CHECK-NEXT:         Flags:           [ ]

test/wasm/weak-undefined.ll

 ; CHECK-NEXT:         ParamTypes:
 ; CHECK-NEXT:   - Type:            FUNCTION
 ; CHECK-NEXT:     FunctionTypes:   [ 0, 0, 0 ]
 ; CHECK-NEXT:   - Type:            TABLE
 ; CHECK-NEXT:     Tables:
 ; CHECK-NEXT:       - ElemType:        ANYFUNC
 ; CHECK-NEXT:         Limits:
 ; CHECK-NEXT:           Flags:           [ HAS_MAX ]
-; CHECK-NEXT:           Initial:         0x00000002
-; CHECK-NEXT:           Maximum:         0x00000002
+; CHECK-NEXT:           Initial:         0x00000001
+; CHECK-NEXT:           Maximum:         0x00000001
 ; CHECK-NEXT:   - Type:            MEMORY
 ; CHECK-NEXT:     Memories:
 ; CHECK-NEXT:       - Initial:         0x00000002
 ; CHECK-NEXT:   - Type:            GLOBAL
 ; CHECK-NEXT:     Globals:
 ; CHECK-NEXT:       - Type:            I32
 ; CHECK-NEXT:         Mutable:         true
 ; CHECK-NEXT:         InitExpr:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           66560
 ; CHECK-NEXT:   - Type:            EXPORT
 ; CHECK-NEXT:     Exports:
 ; CHECK-NEXT:       - Name:            memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            _start
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           2
 ; CHECK-NEXT:       - Name:            get_address_of_foo
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:       - Name:            get_address_of_global_var
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           1
-; CHECK-NEXT:   - Type:            ELEM
-; CHECK-NEXT:     Segments:
-; CHECK-NEXT:       - Offset:
-; CHECK-NEXT:           Opcode:          I32_CONST
-; CHECK-NEXT:           Value:           1
-; CHECK-NEXT:         Functions:       [ 0 ]
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:
 ; CHECK-NEXT:       - Locals:
-; CHECK-NEXT:         Body:            4181808080000B
+; CHECK-NEXT:         Body:            4180808080000B
 ; CHECK-NEXT:       - Locals:
-; CHECK-NEXT:         Body:            41FFFFFFFF7F0B
+; CHECK-NEXT:         Body:            4180808080000B
 ; CHECK-NEXT:       - Locals:
-; CHECK-NEXT:         Body:            41002802FFFFFFFF0F0B
+; CHECK-NEXT:         Body:            4100280280808080000B
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        0
 ; CHECK-NEXT: ...

wasm/InputFiles.h

public:
   const WasmObjectFile *getWasmObj() const { return WasmObj.get(); }
 
   void dumpInfo() const;
 
   uint32_t relocateTypeIndex(uint32_t Original) const;
   uint32_t relocateFunctionIndex(uint32_t Original) const;
   uint32_t relocateGlobalIndex(uint32_t Original) const;
   uint32_t relocateTableIndex(uint32_t Original) const;
-  uint32_t getRelocatedAddress(uint32_t Index) const;
+  uint32_t getRelocatedAddress(uint32_t Index, uint32_t Addend) const;
 
   // Returns true if the given function index is an imported function,
   // as opposed to the locally defined function.
   bool isImportedFunction(uint32_t Index) const;
 
   size_t NumFunctionImports() const { return FunctionImports; }
   size_t NumGlobalImports() const { return GlobalImports; }
 

wasm/InputFiles.cpp

 Symbol *ObjFile::getTableSymbol(uint32_t Index) const {
   return TableSymbols[Index];
 }
 
 Symbol *ObjFile::getGlobalSymbol(uint32_t Index) const {
   return GlobalSymbols[Index];
 }
 
-uint32_t ObjFile::getRelocatedAddress(uint32_t Index) const {
-  return getGlobalSymbol(Index)->getVirtualAddress();
+uint32_t ObjFile::getRelocatedAddress(uint32_t Index, uint32_t Addend) const {
+  return getGlobalSymbol(Index)->getVirtualAddress(Addend);
 }
 
 uint32_t ObjFile::relocateFunctionIndex(uint32_t Original) const {
   Symbol *Sym = getFunctionSymbol(Original);
   uint32_t Index = Sym->getOutputIndex();
   DEBUG(dbgs() << "relocateFunctionIndex: " << toString(*Sym) << ": "
                << Original << " -> " << Index << "\n");
   return Index;
 }
 
 uint32_t ObjFile::relocateTypeIndex(uint32_t Original) const {
   return TypeMap[Original];
 }
 
 uint32_t ObjFile::relocateTableIndex(uint32_t Original) const {
   Symbol *Sym = getTableSymbol(Original);
-  uint32_t Index = Sym->getTableIndex();
+  uint32_t Index = Sym->hasTableIndex() ? Sym->getTableIndex() : 0;
   DEBUG(dbgs() << "relocateTableIndex: " << toString(*Sym) << ": " << Original
                << " -> " << Index << "\n");
   return Index;
 }
 
 uint32_t ObjFile::relocateGlobalIndex(uint32_t Original) const {
   Symbol *Sym = getGlobalSymbol(Original);
-  uint32_t Index = Sym->getOutputIndex();
+  uint32_t Index = Sym->hasOutputIndex() ? Sym->getOutputIndex() : 0;
   DEBUG(dbgs() << "relocateGlobalIndex: " << toString(*Sym) << ": " << Original
                << " -> " << Index << "\n");
   return Index;
 }
 
 void ObjFile::parse() {
   // Parse a memory buffer as a wasm file.
   DEBUG(dbgs() << "Parsing object: " << toString(this) << "\n");

wasm/OutputSections.cpp

for (const WasmRelocation &Reloc : Relocs) {
     NewReloc.Reloc = Reloc;
     NewReloc.Reloc.Offset += OutputOffset;
     DEBUG(dbgs() << "reloc: type=" << Reloc.Type << " index=" << Reloc.Index
                  << " offset=" << Reloc.Offset
                  << " newOffset=" << NewReloc.Reloc.Offset << "\n");
 
     if (Config->EmitRelocs)
       NewReloc.NewIndex = calcNewIndex(File, Reloc);
-    else
-      NewReloc.NewIndex = UINT32_MAX;
 
     switch (Reloc.Type) {
     case R_WEBASSEMBLY_MEMORY_ADDR_SLEB:
     case R_WEBASSEMBLY_MEMORY_ADDR_I32:
     case R_WEBASSEMBLY_MEMORY_ADDR_LEB:
-      NewReloc.Value = File.getRelocatedAddress(Reloc.Index);
-      if (NewReloc.Value != UINT32_MAX)
-        NewReloc.Value += Reloc.Addend;
+      NewReloc.Value = File.getRelocatedAddress(Reloc.Index, Reloc.Addend);
       break;
     default:
       NewReloc.Value = calcNewIndex(File, Reloc);
       break;
     }
 
     OutputRelocs.emplace_back(NewReloc);
   }

wasm/Symbols.h

public:
   // Returns the file from which this symbol was created.
   InputFile *getFile() const { return File; }
 
   uint32_t getGlobalIndex() const;
   uint32_t getFunctionIndex() const;
 
   bool hasFunctionType() const { return FunctionType != nullptr; }
   const WasmSignature &getFunctionType() const;
-  uint32_t getOutputIndex() const;
-  uint32_t getTableIndex() const { return TableIndex.getValue(); }
 
-  // Returns the virtual address of a defined global.
-  // Only works for globals, not functions.
-  uint32_t getVirtualAddress() const;
+  uint32_t getOutputIndex() const { return OutputIndex.getValue(); }
+
+  // Returns true if an output index has been set for this symbol
+  bool hasOutputIndex() const { return OutputIndex.hasValue(); }
 
   // Set the output index of the symbol (in the function or global index
   // space of the output object.
   void setOutputIndex(uint32_t Index);
 
+  uint32_t getTableIndex() const { return TableIndex.getValue(); }
+
   // Returns true if a table index has been set for this symbol
   bool hasTableIndex() const { return TableIndex.hasValue(); }
 
   // Set the table index of the symbol
   void setTableIndex(uint32_t Index);
 
+  // Returns the virtual address of a defined global.
+  // Only works for globals, not functions.
+  uint32_t getVirtualAddress(uint32_t Addend = 0) const;
+
   void setVirtualAddress(uint32_t VA);
 
   void update(Kind K, InputFile *F = nullptr, const WasmSymbol *Sym = nullptr,
               const InputSegment *Segment = nullptr,
               const WasmSignature *Sig = nullptr);
 
   void setArchiveSymbol(const Archive::Symbol &Sym) { ArchiveSymbol = Sym; }
   const Archive::Symbol &getArchiveSymbol() { return ArchiveSymbol; }

wasm/Symbols.cpp

uint32_t Symbol::getFunctionIndex() const {
   return Sym->ElementIndex;
 }
 
 const WasmSignature &Symbol::getFunctionType() const {
   assert(FunctionType != nullptr);
   return *FunctionType;
 }
 
-uint32_t Symbol::getVirtualAddress() const {
+uint32_t Symbol::getVirtualAddress(uint32_t Addend) const {
   assert(isGlobal());
   DEBUG(dbgs() << "getVirtualAddress: " << getName() << "\n");
   if (isUndefined())
-    return UINT32_MAX;
+    return 0;
   if (VirtualAddress.hasValue())
-    return VirtualAddress.getValue();
+    return VirtualAddress.getValue() + Addend;
 
   assert(Sym != nullptr);
   ObjFile *Obj = cast<ObjFile>(File);
   const WasmGlobal &Global =
       Obj->getWasmObj()->globals()[getGlobalIndex() - Obj->NumGlobalImports()];
   assert(Global.Type == llvm::wasm::WASM_TYPE_I32);
   assert(Segment);
-  return Segment->translateVA(Global.InitExpr.Value.Int32);
-}
-
-uint32_t Symbol::getOutputIndex() const {
-  if (isUndefined() && isWeak())
-    return 0;
-  return OutputIndex.getValue();
+  return Segment->translateVA(Global.InitExpr.Value.Int32 + Addend);
 }
 
 void Symbol::setVirtualAddress(uint32_t Value) {
   DEBUG(dbgs() << "setVirtualAddress " << Name << " -> " << Value << "\n");
   assert(!VirtualAddress.hasValue());
   VirtualAddress = Value;
 }
 

wasm/Writer.cpp

for (Symbol *Sym : File->getSymbols()) {
         auto *Obj = cast<ObjFile>(Sym->getFile());
         Sym->setOutputIndex(Obj->FunctionIndexOffset +
                             Sym->getFunctionIndex());
       } else if (Config->EmitRelocs) {
         DefinedGlobals.emplace_back(Sym);
         Sym->setOutputIndex(GlobalIndex++);
       }
     }
+  }
 
+  for (ObjFile *File : Symtab->ObjectFiles) {
     for (Symbol *Sym : File->getTableSymbols()) {
-      if (!Sym->hasTableIndex()) {
-        Sym->setTableIndex(TableIndex++);
-        IndirectFunctions.emplace_back(Sym);
-      }
-    }
-  }
-}
+      if (Sym->hasTableIndex() || !Sym->hasOutputIndex())
+        continue;
 
 static StringRef getOutputDataSegmentName(StringRef Name) {
   if (Config->Relocatable)
     return Name;
 
   for (StringRef V :
        {".text.", ".rodata.", ".data.rel.ro.", ".data.", ".bss.rel.ro.",
         ".bss.", ".init_array.", ".fini_array.", ".ctors.", ".dtors.", ".tbss.",