[WebAssembly] Don't assume that strongly defined symbol are DSO-local

[sbc100]

Summary

The current PIC model for WebAssembly is more like ELF in that it
allows symbol interposition.

This means that more functions end up being addressed via the GOT
and fewer directly added to the wasm table.

One effect is a reduction in the number of wasm table entries similar
to the previous attempt in https://reviews.llvm.org/D61539 which was
reverted.

[sbc100]

cleanup

[sbc100]

Update tests

[sbc100]

Remove newline

[ruiu]

LGTM

[kripken]

This may have broken CI, as it shows as one of the commits in the first broken roll here:

https://chromium-review.googlesource.com/c/emscripten-releases/+/1604139

other.test_binaryen_metadce_hello is broken there. Maybe we just need to update expectations? Looks like they rose though, which is odd?

Waterfall CI also shows wasm2.test_dylink_raii_exceptions is broken on Module.dynCall_ifdi is not a function which is maybe also related to this, but just a guess.

[sbc100]

In D61772#1498187, @kripken wrote:

This may have broken CI, as it shows as one of the commits in the first broken roll here:

https://chromium-review.googlesource.com/c/emscripten-releases/+/1604139

other.test_binaryen_metadce_hello is broken there. Maybe we just need to update expectations? Looks like they rose though, which is odd?

Waterfall CI also shows wasm2.test_dylink_raii_exceptions is broken on Module.dynCall_ifdi is not a function which is maybe also related to this, but just a guess.

There are a couple of different issues right now with the waterfall.

The test_binaryen_metadce_hello failure is very strange. I've put this fix for it, but I can't see how it broke, certainly doesn't seem to be this change that did it: https://github.com/emscripten-core/emscripten/pull/8581

The test_dylink_raii_exceptions failure has a solution in binaryen: https://github.com/WebAssembly/binaryen/pull/2095. However I'm not sure that test is still failing on ToT.

test/wasm/pie.ll

 ; RUN: llc -relocation-model=pic -filetype=obj %s -o %t.o
 ; RUN: wasm-ld --no-gc-sections --allow-undefined -pie -o %t.wasm %t.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 
 target triple = "wasm32-unknown-unknown"
 
 @data = global i32 2, align 4
 @data_external = external global i32
 @indirect_func = local_unnamed_addr global i32 ()* @foo, align 4
 
 @data_addr = local_unnamed_addr global i32* @data, align 4
 @data_addr_external = local_unnamed_addr global i32* @data_external, align 4
 
-define i32 @foo() {
+define hidden i32 @foo() {
 entry:
   ; To ensure we use __stack_pointer
   %ptr = alloca i32
   %0 = load i32, i32* @data, align 4
   %1 = load i32 ()*, i32 ()** @indirect_func, align 4
   call i32 %1()
   ret i32 %0
 }

test/wasm/shared.ll

entry:
   ; To ensure we use __stack_pointer
   %ptr = alloca i32
   %0 = load i32, i32* @data, align 4
   %1 = load i32 ()*, i32 ()** @indirect_func, align 4
   call i32 %1()
   ret i32 %0
 }
 
-define default i32* @get_data_address() {
+define hidden i32* @get_data_address() {
 entry:
   ret i32* @data_external
 }
 
-define default i8* @get_func_address() {
+define hidden i8* @get_func_address() {
 entry:
   ret i8* bitcast (void ()* @func_external to i8*)
 }
 
 define default i8* @get_local_func_address() {
 entry:
   ; Verify that a function which is otherwise not address taken *is* added to
   ; the wasm table with referenced via R_WASM_TABLE_INDEX_REL_SLEB
   ret i8* bitcast (i8* ()* @get_func_address to i8*)
 }
 
 declare void @func_external()
 
 ; check for dylink section at start
 
 ; CHECK:      Sections:
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            dylink
 ; CHECK-NEXT:     MemorySize:      24
 ; CHECK-NEXT:     MemoryAlignment: 2
-; CHECK-NEXT:     TableSize:       3
+; 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:        - Type:            IMPORT
 ; CHECK-NEXT:     Imports:
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           memory
 ; CHECK-NEXT:         Kind:            MEMORY
 ; CHECK-NEXT:         Memory:
 ; CHECK-NEXT:           Initial:       0x0000000
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           __indirect_function_table
 ; CHECK-NEXT:         Kind:            TABLE
 ; CHECK-NEXT:         Table:
 ; CHECK-NEXT:           ElemType:        FUNCREF
 ; CHECK-NEXT:           Limits:
-; CHECK-NEXT:             Initial:         0x00000003
+; CHECK-NEXT:             Initial:         0x00000002
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           __stack_pointer
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         GlobalType:      I32
 ; CHECK-NEXT:         GlobalMutable:   true
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           __memory_base
 ; 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:           func_external
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         SigIndex:        1
 ; CHECK-NEXT:       - Module:          GOT.mem
-; CHECK-NEXT:         Field:           data_external
+; CHECK-NEXT:         Field:           indirect_func
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         GlobalType:      I32
 ; CHECK-NEXT:         GlobalMutable:   true
 ; CHECK-NEXT:       - Module:          GOT.func
 ; CHECK-NEXT:         Field:           func_external
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         GlobalType:      I32
 ; CHECK-NEXT:         GlobalMutable:   true
 ; CHECK-NEXT:       - Module:          GOT.mem
+; CHECK-NEXT:         Field:           data_external
+; CHECK-NEXT:         Kind:            GLOBAL
+; CHECK-NEXT:         GlobalType:      I32
+; CHECK-NEXT:         GlobalMutable:   true
+; CHECK-NEXT:       - Module:          GOT.mem
 ; CHECK-NEXT:         Field:           extern_struct
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         GlobalType:      I32
 ; CHECK-NEXT:         GlobalMutable:   true
 ; CHECK-NEXT:   - Type:            FUNCTION
 
 ; CHECK:        - Type:            EXPORT
 ; CHECK-NEXT:     Exports:
 ; CHECK-NEXT:       - Name:            __wasm_call_ctors
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         Index:           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:       [ 5, 3, 0 ]
+; CHECK-NEXT:         Functions:       [ 4, 3 ]
 
 ; check the generated code in __wasm_call_ctors and __wasm_apply_relocs functions
 ; TODO(sbc): Disassemble and verify instructions.
 
 ; CHECK:        - Type:            CODE
 ; CHECK-NEXT:     Functions:
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         Locals:          []
 ; CHECK-NEXT:         Body:            10020B
 ; CHECK-NEXT:       - Index:           2
 ; CHECK-NEXT:         Locals:          []
-; CHECK-NEXT:         Body:            230141046A230241016A360200230141086A23043602002301410C6A230141006A360200230141106A2303360200230141146A230541046A3602000B
+; CHECK-NEXT:         Body:            230141046A230241016A360200230141086A23043602002301410C6A230141006A360200230141106A2305360200230141146A230641046A3602000B
 
 ; 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:         '020000000100000002000000000000000000000000000000'
+; CHECK-NEXT:         Content:         '020000000100000000000000000000000000000000000000'

wasm/InputFiles.cpp

if (isa<DataSymbol>(Sym) && Sym->isUndefined()) {
       }
     }
   }
 
   switch (Reloc.Type) {
   case R_WASM_TABLE_INDEX_I32:
   case R_WASM_TABLE_INDEX_SLEB:
   case R_WASM_TABLE_INDEX_REL_SLEB:
+    if (Config->Pic && !getFunctionSymbol(Reloc.Index)->hasTableIndex())
+      return 0;
     return getFunctionSymbol(Reloc.Index)->getTableIndex();
   case R_WASM_MEMORY_ADDR_SLEB:
   case R_WASM_MEMORY_ADDR_I32:
   case R_WASM_MEMORY_ADDR_LEB:
   case R_WASM_MEMORY_ADDR_REL_SLEB:
     return cast<DefinedData>(Sym)->getVirtualAddress() + Reloc.Addend;
   case R_WASM_TYPE_INDEX_LEB:
     return TypeMap[Reloc.Index];

wasm/Writer.cpp

void Writer::calculateTypes() {
 
   for (const InputFunction *F : InputFunctions)
     registerType(F->Signature);
 
   for (const InputEvent *E : InputEvents)
     registerType(E->Signature);
 }
 
+static bool requiresGOTAccess(const Symbol* Sym) {
+  return Config->Pic && !Sym->isHidden() && !Sym->isLocal();
+}
+
 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:
     case R_WASM_TABLE_INDEX_REL_SLEB: {
       FunctionSymbol *Sym = File->getFunctionSymbol(Reloc.Index);
-      if (Sym->hasTableIndex() || !Sym->hasFunctionIndex())
-        continue;
+      if (Sym->hasTableIndex() || !Sym->hasFunctionIndex() || requiresGOTAccess(Sym))
+        break;
       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;
if (Config->Pic) {
         break;
       }
       case R_WASM_TABLE_INDEX_I32:
       case R_WASM_MEMORY_ADDR_I32: {
         // These relocation types are only present in the data section and
         // will be converted into code by `generateRelocationCode`.  This code
         // requires the symbols to have GOT entires.
         auto* Sym = File->getSymbols()[Reloc.Index];
-        if (!Sym->isHidden() && !Sym->isLocal() && !Sym->isInGOT()) {
+        if (requiresGOTAccess(Sym) && !Sym->isInGOT()) {
           Sym->setGOTIndex(NumImportedGlobals++);
           GOTSymbols.push_back(Sym);
         }
         break;
       }
       }
     }
   }