[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-external.ll

-; RUN: llc -mtriple=wasm32-unknown-unknown-wasm -filetype=obj -o %t.o %s
-; RUN: lld -flavor wasm -strip-debug %t.o -o %t.wasm
-; RUN: obj2yaml %t.wasm | FileCheck %s
-
-; Test that undefined weak externals (global_var) and (foo) don't cause
-; link failures and resolve to zero.
-
-@global_var = extern_weak global i32, align 4
-
-declare extern_weak i32 @foo()
-
-define i8* @get_address_of_foo() #0 {
-entry:
-  ret i8* bitcast (i32 ()* @foo to i8*)
-}
-
-define i32* @get_address_of_global_var() #0 {
-  ret i32* @global_var
-}
-
-define i32 @_start() #0 {
-entry:
-    %0 = load i32, i32* @global_var, align 4
-    ret i32 %0
-}
-
-; CHECK:      --- !WASM
-; CHECK-NEXT: FileHeader:
-; CHECK-NEXT:   Version:         0x00000001
-; CHECK-NEXT: Sections:
-; CHECK-NEXT:   - Type:            TYPE
-; CHECK-NEXT:     Signatures:
-; CHECK-NEXT:       - Index:           0
-; CHECK-NEXT:         ReturnType:      I32
-; 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:   - 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:       - Locals:
-; CHECK-NEXT:         Body:            41FFFFFFFF7F0B
-; CHECK-NEXT:       - Locals:
-; CHECK-NEXT:         Body:            41002802FFFFFFFF0F0B
-; CHECK-NEXT:   - Type:            CUSTOM
-; CHECK-NEXT:     Name:            linking
-; CHECK-NEXT:     DataSize:        0
-; CHECK-NEXT: ...

test/wasm/weak-undefined.ll

+; RUN: llc -mtriple=wasm32-unknown-unknown-wasm -filetype=obj -o %t.o %s
+; RUN: lld -flavor wasm -strip-debug %t.o -o %t.wasm
+; RUN: obj2yaml %t.wasm | FileCheck %s
+
+; Test that undefined weak externals (global_var) and (foo) don't cause
+; link failures and resolve to zero.
+
+@global_var = extern_weak global i32, align 4
+
+declare extern_weak i32 @foo()
+
+define i8* @get_address_of_foo() #0 {
+entry:
+  ret i8* bitcast (i32 ()* @foo to i8*)
+}
+
+define i32* @get_address_of_global_var() #0 {
+  ret i32* @global_var
+}
+
+define i32 @_start() #0 {
+entry:
+    %0 = load i32, i32* @global_var, align 4
+    ret i32 %0
+}
+
+; CHECK:      --- !WASM
+; CHECK-NEXT: FileHeader:
+; CHECK-NEXT:   Version:         0x00000001
+; CHECK-NEXT: Sections:
+; CHECK-NEXT:   - Type:            TYPE
+; CHECK-NEXT:     Signatures:
+; CHECK-NEXT:       - Index:           0
+; CHECK-NEXT:         ReturnType:      I32
+; 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:         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:            CODE
+; CHECK-NEXT:     Functions:
+; CHECK-NEXT:       - Locals:
+; CHECK-NEXT:         Body:            4180808080000B
+; CHECK-NEXT:       - Locals:
+; CHECK-NEXT:         Body:            4180808080000B
+; CHECK-NEXT:       - Locals:
+; CHECK-NEXT:         Body:            4100280280808080000B
+; CHECK-NEXT:   - Type:            CUSTOM
+; CHECK-NEXT:     Name:            linking
+; CHECK-NEXT:     DataSize:        0
+; CHECK-NEXT: ...

wasm/InputFiles.cpp

 }
 
 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() 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

const WasmSignature &Symbol::getFunctionType() const {
   assert(FunctionType != nullptr);
   return *FunctionType;
 }
 
 uint32_t Symbol::getVirtualAddress() const {
   assert(isGlobal());
   DEBUG(dbgs() << "getVirtualAddress: " << getName() << "\n");
   if (isUndefined())
-    return UINT32_MAX;
+    return 0;
   if (VirtualAddress.hasValue())
     return VirtualAddress.getValue();
 
   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();
-}
-
 void Symbol::setVirtualAddress(uint32_t Value) {
   DEBUG(dbgs() << "setVirtualAddress " << Name << " -> " << Value << "\n");
   assert(!VirtualAddress.hasValue());
   VirtualAddress = Value;
 }
 
 void Symbol::setOutputIndex(uint32_t Index) {
   DEBUG(dbgs() << "setOutputIndex " << Name << " -> " << Index << "\n");

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.",