[WebAssembly] Error on R_WASM_MEMORY_ADDR relocations against undefined data symbols.

[sbc100]

Summary

For these types of relocations an absolute memory address is
required which is not possible for undefined data symbols. For symbols
that can be undefined at link time (i.e. external data symbols in
shared libraries) a different type of relocation (i.e. via a GOT) will
be needed.

[sbc100]

Add test

[ruiu]

lld/wasm/Writer.cpp

1038 ↗

(On Diff #190524)

This function is getting longer. I wonder if you can move this lambda out of this function.

[sbc100]

• fix error

[sbc100]

lld/wasm/Writer.cpp

1038 ↗

(On Diff #190524)

Ok to do that in a follow up?

[ruiu]

LGTM

lld/wasm/Writer.cpp

1038 ↗

(On Diff #190524)

I'd personally like to do beforehand so that the code is cleaner at any point of time, but a follow-up patch is OK.

[sbc100]

• split out lambda

[sbc100]

lld/wasm/Writer.cpp

1038 ↗

(On Diff #190524)

OK I did with this CL.

lld/trunk/test/wasm/shared.ll

 ; RUN: llc -O0 -filetype=obj %s -o %t.o
 ; RUN: wasm-ld -shared -o %t.wasm %t.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 target triple = "wasm32-unknown-unknown"
 
 @data = hidden global i32 2, align 4
 @indirect_func = local_unnamed_addr global i32 ()* @foo, align 4
 @indirect_func_external = local_unnamed_addr global void ()* @func_external, align 4
 
 define default i32 @foo() {
 entry:
   ; To ensure we use __stack_pointer
   %ptr = alloca i32
   %0 = load i32, i32* @data, align 4
-  %1 = load i32, i32* @data_external, align 4
-  %2 = load i32 ()*, i32 ()** @indirect_func, align 4
-  call i32 %2()
-  ret i32 %1
+  ; TODO(sbc): Re-enable once the codegen supports generating the correct
+  ; relocation type when referencing external data in shared libraries.
+  ; %1 = load i32, i32* @data_external, align 4
+  %1 = load i32 ()*, i32 ()** @indirect_func, align 4
+  call i32 %1()
+  ret i32 %0
 }
 
 declare void @func_external()
 
 @data_external = external global i32
 
 
 ; check for dylink section at start
 
 ; CHECK:      Sections:
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            dylink
-; CHECK-NEXT:     MemorySize:      8
+; CHECK-NEXT:     MemorySize:      12
 ; CHECK-NEXT:     MemoryAlignment: 2
 ; CHECK-NEXT:     TableSize:       2
 ; CHECK-NEXT:     TableAlignment:  0
 ; CHECK-NEXT:     Needed:          []
 ; CHECK-NEXT:   - Type:            TYPE
 
 ; check for import of __table_base and __memory_base globals
 

 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         GlobalType:      I32
 ; CHECK-NEXT:         GlobalMutable:   false
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           __table_base
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         GlobalType:      I32
 ; CHECK-NEXT:         GlobalMutable:   false
-; CHECK-NEXT:       - Module:          env
-; CHECK-NEXT:         Field:           data_external
-; CHECK-NEXT:         Kind:            GLOBAL
-; CHECK-NEXT:         GlobalType:      I32
-; CHECK-NEXT:         GlobalMutable:   true
+; XCHECK-NEXT:       - Module:          env
+; XCHECK-NEXT:         Field:           data_external
+; XCHECK-NEXT:         Kind:            GLOBAL
+; XCHECK-NEXT:         GlobalType:      I32
+; XCHECK-NEXT:         GlobalMutable:   true
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           func_external
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         SigIndex:        1
 
 ; check for elem segment initialized with __table_base global as offset
 
 ; CHECK:        - Type:            ELEM
 ; CHECK-NEXT:     Segments:
 ; CHECK-NEXT:       - Offset:
 ; CHECK-NEXT:           Opcode:          GLOBAL_GET
 ; CHECK-NEXT:           Index:           2
 ; CHECK-NEXT:         Functions:       [ 1, 0 ]
 
 ; check the data segment initialized with __memory_base global as offset
 
 ; CHECK:        - Type:            DATA
 ; CHECK-NEXT:     Segments:
 ; CHECK-NEXT:       - SectionOffset:   6
 ; CHECK-NEXT:         InitFlags:       0
 ; CHECK-NEXT:         Offset:
 ; CHECK-NEXT:           Opcode:          GLOBAL_GET
 ; CHECK-NEXT:           Index:           1
-; CHECK-NEXT:         Content:         '0000000001000000'
+; CHECK-NEXT:         Content:         '020000000000000001000000'

lld/trunk/test/wasm/undefined-data.ll

+; RUN: llc -filetype=obj %s -o %t.o
+; RUN: not wasm-ld -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=UNDEF
+; RUN: not wasm-ld --allow-undefined -o %t.wasm %t.o 2>&1 | FileCheck %s -check-prefix=BADRELOC
+
+target triple = "wasm32-unknown-unknown"
+
+@data_external = external global i32
+
+define i32 @_start() {
+entry:
+    %0 = load i32, i32* @data_external, align 4
+    ret i32 %0
+}
+
+; UNDEF: undefined symbol: data_external
+; BADRELOC: undefined-data.ll.tmp.o: relocation of type R_WASM_MEMORY_ADDR_* against undefined data symbol: data_external

lld/trunk/wasm/Writer.cpp

 private:
   void openFile();
 
   uint32_t lookupType(const WasmSignature &Sig);
   uint32_t registerType(const WasmSignature &Sig);
 
   void createCtorFunction();
   void calculateInitFunctions();
+  void processRelocations(InputChunk *Chunk);
   void assignIndexes();
   void calculateImports();
   void calculateExports();
   void calculateCustomSections();
   void assignSymtab();
   void calculateTypes();
   void createOutputSegments();
   void layoutMemory();
void Writer::calculateTypes() {
 
   for (const InputFunction *F : InputFunctions)
     registerType(F->Signature);
 
   for (const InputEvent *E : InputEvents)
     registerType(E->Signature);
 }
 
+void Writer::processRelocations(InputChunk *Chunk) {
+  if (!Chunk->Live)
+    return;
+  ObjFile *File = Chunk->File;
+  ArrayRef<WasmSignature> Types = File->getWasmObj()->types();
+  for (const WasmRelocation &Reloc : Chunk->getRelocations()) {
+    switch (Reloc.Type) {
+    case R_WASM_TABLE_INDEX_I32:
+    case R_WASM_TABLE_INDEX_SLEB: {
+      FunctionSymbol *Sym = File->getFunctionSymbol(Reloc.Index);
+      if (Sym->hasTableIndex() || !Sym->hasFunctionIndex())
+        continue;
+      Sym->setTableIndex(TableBase + IndirectFunctions.size());
+      IndirectFunctions.emplace_back(Sym);
+      break;
+    }
+    case R_WASM_TYPE_INDEX_LEB:
+      // Mark target type as live
+      File->TypeMap[Reloc.Index] = registerType(Types[Reloc.Index]);
+      File->TypeIsUsed[Reloc.Index] = true;
+      break;
+    case R_WASM_MEMORY_ADDR_SLEB:
+    case R_WASM_MEMORY_ADDR_I32:
+    case R_WASM_MEMORY_ADDR_LEB: {
+      DataSymbol *DataSym = File->getDataSymbol(Reloc.Index);
+      if (!Config->Relocatable && !isa<DefinedData>(DataSym) &&
+          !DataSym->isWeak())
+        error(File->getName() +
+              ": relocation of type R_WASM_MEMORY_ADDR_* "
+              "against undefined data symbol: " +
+              DataSym->getName());
+      break;
+    }
+    }
+  }
+}
+
 void Writer::assignIndexes() {
   assert(InputFunctions.empty());
   uint32_t FunctionIndex = NumImportedFunctions;
   auto AddDefinedFunction = [&](InputFunction *Func) {
     if (!Func->Live)
       return;
     InputFunctions.emplace_back(Func);
     Func->setFunctionIndex(FunctionIndex++);
   };
 
   for (InputFunction *Func : Symtab->SyntheticFunctions)
     AddDefinedFunction(Func);
 
   for (ObjFile *File : Symtab->ObjectFiles) {
     LLVM_DEBUG(dbgs() << "Functions: " << File->getName() << "\n");
     for (InputFunction *Func : File->Functions)
       AddDefinedFunction(Func);
   }
 
-  uint32_t TableIndex = TableBase;
-  auto HandleRelocs = [&](InputChunk *Chunk) {
-    if (!Chunk->Live)
-      return;
-    ObjFile *File = Chunk->File;
-    ArrayRef<WasmSignature> Types = File->getWasmObj()->types();
-    for (const WasmRelocation &Reloc : Chunk->getRelocations()) {
-      if (Reloc.Type == R_WASM_TABLE_INDEX_I32 ||
-          Reloc.Type == R_WASM_TABLE_INDEX_SLEB) {
-        FunctionSymbol *Sym = File->getFunctionSymbol(Reloc.Index);
-        if (Sym->hasTableIndex() || !Sym->hasFunctionIndex())
-          continue;
-        Sym->setTableIndex(TableIndex++);
-        IndirectFunctions.emplace_back(Sym);
-      } else if (Reloc.Type == R_WASM_TYPE_INDEX_LEB) {
-        // Mark target type as live
-        File->TypeMap[Reloc.Index] = registerType(Types[Reloc.Index]);
-        File->TypeIsUsed[Reloc.Index] = true;
-      }
-    }
-  };
-
   for (ObjFile *File : Symtab->ObjectFiles) {
     LLVM_DEBUG(dbgs() << "Handle relocs: " << File->getName() << "\n");
     for (InputChunk *Chunk : File->Functions)
-      HandleRelocs(Chunk);
+      processRelocations(Chunk);
     for (InputChunk *Chunk : File->Segments)
-      HandleRelocs(Chunk);
+      processRelocations(Chunk);
     for (auto &P : File->CustomSections)
-      HandleRelocs(P);
+      processRelocations(P);
   }
 
   assert(InputGlobals.empty());
   uint32_t GlobalIndex = NumImportedGlobals;
   auto AddDefinedGlobal = [&](InputGlobal *Global) {
     if (Global->Live) {
       LLVM_DEBUG(dbgs() << "AddDefinedGlobal: " << GlobalIndex << "\n");
       Global->setGlobalIndex(GlobalIndex++);