[lld][WebAssembly] Fix func reloc for internal GOT with extended-const

[sbc100]

lld/wasm/SyntheticSections.cpp

525

Can we combine these two blocks into a single if (config->extendedConst && config->isPic) block that handles these? (since they share most of the code).

Also, shouldn't there be a corresponding change to __wasm_apply_global_relocs to avoid applying a relocation?

Also, can you add a test for this (I would imagine by modifying lld/test/wasm/pie.ll perhaps?

[yamt]

lld/wasm/SyntheticSections.cpp

525

Can we combine these two blocks into a single if (config->extendedConst && config->isPic) block that handles these? (since they share most of the code).

ok

Also, shouldn't there be a corresponding change to __wasm_apply_global_relocs to avoid applying a relocation?

that part is already done. that's why it's broken and i noticed it.

Also, can you add a test for this (I would imagine by modifying lld/test/wasm/pie.ll perhaps?

ok. i will try.

[yamt]

reduce code dup, add test

[yamt]

fix test

[sbc100]

lld/test/wasm/Inputs/internal_func.ll

11 ↗

(On Diff #543929)

For new test files could you add them as .s files? I've been trying to convert all the .ll files for .s over time but have yet to finish that work.

It should be that same or fewer lines I think.

lld/test/wasm/pie.ll

39

Why define two different internal funcs?

lld/wasm/SyntheticSections.cpp

488

llvm coding style uses camel case for locals so this would be useExtendedConst and globalIdx

[yamt]

lld/test/wasm/Inputs/internal_func.ll

11 ↗

(On Diff #543929)

ok

lld/test/wasm/pie.ll

39

because one of them happened to have offset 0.

lld/wasm/SyntheticSections.cpp

488

ok

[yamt]

rename variables, convert to .s

[yamt]

apply clang-format

[sbc100]

Do you have llvm commit permissions or should I land this for you?

[yamt]

In D155542#4534604, @sbc100 wrote:

Do you have llvm commit permissions or should I land this for you?

i don't have any permissions on this repo.

lld/test/wasm/Inputs/internal_func.s

+  .globl  internal_func1
+internal_func1:
+  .functype internal_func1 () -> (i32)
+  i32.const 0
+  end_function
+
+  .globl  internal_func2
+internal_func2:
+  .functype internal_func2 () -> (i32)
+  i32.const 0
+  end_function

lld/test/wasm/pie.ll

 ; RUN: llc -relocation-model=pic -mattr=+mutable-globals -filetype=obj %s -o %t.o
-; RUN: wasm-ld --no-gc-sections --experimental-pic -pie -o %t.wasm %t.o
+; RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-emscripten -filetype=obj %S/Inputs/internal_func.s -o %t.internal_func.o
+; RUN: wasm-ld --no-gc-sections --experimental-pic -pie -o %t.wasm %t.o %t.internal_func.o
 ; RUN: obj2yaml %t.wasm | FileCheck %s
 ; RUN: llvm-objdump --disassemble-symbols=__wasm_call_ctors,__wasm_apply_data_relocs --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DISASSEM
 
 target triple = "wasm32-unknown-emscripten"
 
 @data = global i32 2, align 4
 @data_external = external global i32
 @indirect_func = local_unnamed_addr global ptr @foo, align 4
entry:
   ret i32 %0
 }
 
 define default ptr @get_data_address() {
 entry:
   ret ptr @data_addr_external
 }
 
+define default ptr @get_internal_func1_address() {
+entry:
+  ret ptr @internal_func1
+}
+
+define default ptr @get_internal_func2_address() {
+entry:
+  ret ptr @internal_func2
+}
+
 define void @_start() {
   call void @external_func()
   ret void
 }
 
 declare void @external_func()
+declare ptr @internal_func1()
+declare ptr @internal_func2()
 
 ; CHECK:      Sections:
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            dylink.0
 ; CHECK-NEXT:     MemorySize:      16
 ; CHECK-NEXT:     MemoryAlignment: 2
-; CHECK-NEXT:     TableSize:       1
+; CHECK-NEXT:     TableSize:       3
 ; CHECK-NEXT:     TableAlignment:  0
 ; CHECK-NEXT:     Needed:          []
 
 ; CHECK:        - Type:            IMPORT
 ; CHECK-NEXT:     Imports:
 ; CHECK-NEXT:      - Module:          env
 ; CHECK-NEXT:        Field:           __indirect_function_table
 ; CHECK-NEXT:        Kind:            TABLE
 ; CHECK-NEXT:        Table:
 ; CHECK-NEXT:          Index:           0
 ; CHECK-NEXT:          ElemType:        FUNCREF
 ; CHECK-NEXT:          Limits:
-; CHECK-NEXT:            Minimum:         0x1
+; CHECK-NEXT:            Minimum:         0x3
 ; 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:         Name:            __wasm_apply_data_relocs
 ; CHECK-NEXT:       - Index:           3
 ; CHECK-NEXT:         Name:            __wasm_apply_global_relocs
 ; CHECK-NEXT:       - Index:           4
 ; CHECK-NEXT:         Name:            foo
 ; CHECK-NEXT:       - Index:           5
 ; CHECK-NEXT:         Name:            get_data_address
 ; CHECK-NEXT:       - Index:           6
+; CHECK-NEXT:         Name:            get_internal_func1_address
+; CHECK-NEXT:       - Index:           7
+; CHECK-NEXT:         Name:            get_internal_func2_address
+; CHECK-NEXT:       - Index:           8
 ; CHECK-NEXT:         Name:            _start
+; CHECK-NEXT:       - Index:           9
+; CHECK-NEXT:         Name:            internal_func1
+; CHECK-NEXT:       - Index:           10
+; CHECK-NEXT:         Name:            internal_func2
 ; CHECK-NEXT:     GlobalNames:
 
 ; DISASSEM-LABEL:  <__wasm_call_ctors>:
 ; DISASSEM-EMPTY:
 ; DISASSEM-NEXT:   end
 
 ; DISASSEM-LABEL:  <__wasm_apply_data_relocs>:
 ; DISASSEM:        end
 
 ; Run the same test with extended-const support.  When this is available
 ; we don't need __wasm_apply_global_relocs and instead rely on the add
 ; instruction in the InitExpr.  We also, therefore, do not need these globals
 ; to be mutable.
 
 ; RUN: llc -relocation-model=pic -mattr=+extended-const,+mutable-globals,+atomics,+bulk-memory -filetype=obj %s -o %t.extended.o
-; RUN: wasm-ld --no-gc-sections --allow-undefined --experimental-pic -pie -o %t.extended.wasm %t.extended.o
+; RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-emscripten -filetype=obj %S/Inputs/internal_func.s -o %t.internal_func.extended.o
+; RUN: wasm-ld --no-gc-sections --experimental-pic -pie -o %t.extended.wasm %t.extended.o %t.internal_func.extended.o
 ; RUN: obj2yaml %t.extended.wasm | FileCheck %s --check-prefix=EXTENDED-CONST
 
 ; EXTENDED-CONST-NOT: __wasm_apply_global_relocs
 
 ; EXTENDED-CONST:       - Type:            GLOBAL
 ; EXTENDED-CONST-NEXT:    Globals:
 ; EXTENDED-CONST-NEXT:      - Index:           4
 ; EXTENDED-CONST-NEXT:        Type:            I32

 ; EXTENDED-CONST-NEXT:      - Index:           6
 ; EXTENDED-CONST-NEXT:        Type:            I32
 ; EXTENDED-CONST-NEXT:        Mutable:         false
 ; EXTENDED-CONST-NEXT:        InitExpr:
 ; EXTENDED-CONST-NEXT:          Extended:        true
 ; This instruction sequence decodes to:
 ; (global.get[0x23] 0x1 i32.const[0x41] 0x0C i32.add[0x6A] end[0x0b])
 ; EXTENDED-CONST-NEXT:          Body:            2301410C6A0B
+; EXTENDED-CONST-NEXT:      - Index:           7
+; EXTENDED-CONST-NEXT:        Type:            I32
+; EXTENDED-CONST-NEXT:        Mutable:         false
+; EXTENDED-CONST-NEXT:        InitExpr:
+; EXTENDED-CONST-NEXT:          Opcode:        GLOBAL_GET
+; EXTENDED-CONST-NEXT:          Index:         2
+; EXTENDED-CONST-NEXT:      - Index:           8
+; EXTENDED-CONST-NEXT:        Type:            I32
+; EXTENDED-CONST-NEXT:        Mutable:         false
+; EXTENDED-CONST-NEXT:        InitExpr:
+; EXTENDED-CONST-NEXT:          Extended:        true
+; This instruction sequence decodes to:
+; (global.get[0x23] 0x2 i32.const[0x41] 0x1 i32.add[0x6A] end[0x0b])
+; EXTENDED-CONST-NEXT:          Body:            230241016A0B
 
 ;  EXTENDED-CONST-NOT:  - Type:            START
 
 ;      EXTENDED-CONST:    FunctionNames:
 ; EXTENDED-CONST-NEXT:      - Index:           0
 ; EXTENDED-CONST-NEXT:        Name:            external_func
 ; EXTENDED-CONST-NEXT:      - Index:           1
 ; EXTENDED-CONST-NEXT:        Name:            __wasm_call_ctors
 ; EXTENDED-CONST-NEXT:      - Index:           2
 ; EXTENDED-CONST-NEXT:        Name:            __wasm_apply_data_relocs
 
 ; Run the same test with threading support.  In this mode
 ; we expect __wasm_init_memory and __wasm_apply_data_relocs
 ; to be generated along with __wasm_start as the start
 ; function.
 
 ; RUN: llc -relocation-model=pic -mattr=+mutable-globals,+atomics,+bulk-memory -filetype=obj %s -o %t.shmem.o
-; RUN: wasm-ld --no-gc-sections --shared-memory --allow-undefined --experimental-pic -pie -o %t.shmem.wasm %t.shmem.o
+; RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-emscripten -filetype=obj %S/Inputs/internal_func.s -o %t.internal_func.shmem.o
+; RUN: wasm-ld --no-gc-sections --shared-memory --experimental-pic -pie -o %t.shmem.wasm %t.shmem.o %t.internal_func.shmem.o
 ; RUN: obj2yaml %t.shmem.wasm | FileCheck %s --check-prefix=SHMEM
 ; RUN: llvm-objdump --disassemble-symbols=__wasm_start --no-show-raw-insn --no-leading-addr %t.shmem.wasm | FileCheck %s --check-prefix DISASSEM-SHMEM
 
 ; SHMEM:         - Type:            START
 ; SHMEM-NEXT:      StartFunction:   6
 
 ; DISASSEM-SHMEM-LABEL:  <__wasm_start>:
 ; DISASSEM-SHMEM-EMPTY:

 ; SHMEM-NEXT:        Name:            __wasm_apply_global_relocs
 ; SHMEM-NEXT:      - Index:           6
 ; SHMEM-NEXT:        Name:            __wasm_start
 ; SHMEM-NEXT:      - Index:           7
 ; SHMEM-NEXT:        Name:            foo
 ; SHMEM-NEXT:      - Index:           8
 ; SHMEM-NEXT:        Name:            get_data_address
 ; SHMEM-NEXT:      - Index:           9
+; SHMEM-NEXT:        Name:            get_internal_func1_address
+; SHMEM-NEXT:      - Index:           10
+; SHMEM-NEXT:        Name:            get_internal_func2_address
+; SHMEM-NEXT:      - Index:           11
 ; SHMEM-NEXT:        Name:            _start

lld/wasm/SyntheticSections.cpp

if (!sym->isStub) {
       // With multi-theadeding any TLS globals must be mutable since they get
       // set during `__wasm_apply_global_tls_relocs`
       if (config->sharedMemory && sym->isTLS())
         mutable_ = true;
     }
     WasmGlobalType type{itype, mutable_};
     writeGlobalType(os, type);
 
-    if (config->extendedConst && config->isPic && !sym->isTLS() &&
-        isa<DefinedData>(sym)) {
+    bool useExtendedConst = false;
+    uint32_t globalIdx;
+    int64_t offset;
+    if (config->extendedConst && config->isPic) {
+      if (auto *d = dyn_cast<DefinedData>(sym)) {
+        if (!sym->isTLS()) {
+          globalIdx = WasmSym::memoryBase->getGlobalIndex();
+          offset = d->getVA();
+          useExtendedConst = true;
+        }
+      } else if (auto *f = dyn_cast<FunctionSymbol>(sym)) {
+        if (!sym->isStub) {
+          globalIdx = WasmSym::tableBase->getGlobalIndex();
+          offset = f->getTableIndex();
+          useExtendedConst = true;
+        }
+      }
+    }
+    if (useExtendedConst) {
       // We can use an extended init expression to add a constant
-      // offset of __memory_base.
-      auto *d = cast<DefinedData>(sym);
+      // offset of __memory_base/__table_base.
       writeU8(os, WASM_OPCODE_GLOBAL_GET, "global get");
-      writeUleb128(os, WasmSym::memoryBase->getGlobalIndex(),
-                   "literal (global index)");
-      if (d->getVA()) {
-        writePtrConst(os, d->getVA(), is64, "offset");
+      writeUleb128(os, globalIdx, "literal (global index)");
+      if (offset) {
+        writePtrConst(os, offset, is64, "offset");
         writeU8(os, is64 ? WASM_OPCODE_I64_ADD : WASM_OPCODE_I32_ADD, "add");
       }
       writeU8(os, WASM_OPCODE_END, "opcode:end");
     } else {
       WasmInitExpr initExpr;
       if (auto *d = dyn_cast<DefinedData>(sym))
         initExpr = intConst(d->getVA(), is64);
       else if (auto *f = dyn_cast<FunctionSymbol>(sym))
         initExpr = intConst(f->isStub ? 0 : f->getTableIndex(), is64);
       else {
         assert(isa<UndefinedData>(sym));
         initExpr = intConst(0, is64);
       }
       writeInitExpr(os, initExpr);
     }
   }
   for (const DefinedData *sym : dataAddressGlobals) {
     WasmGlobalType type{itype, false};
     writeGlobalType(os, type);
     writeInitExpr(os, intConst(sym->getVA(), is64));
   }
 }