Reapply "[lld][WebAssembly] Do not relocate ABSOLUTE symbols" (#154371) #154494

yamt · 2025-08-20T09:21:38Z

This reverts commit 28f2fb2.

The previous attempt was breaking things like __start_ symbols.

This time, use a separate flag instead of abusing ABOSULUTE.

…54371) This reverts commit 28f2fb2. The previous attempt was breaking things like __start_ symbols. This time, use a separate flag instead of abusing ABOSULUTE.

llvmbot · 2025-08-20T09:22:11Z

@llvm/pr-subscribers-lld

Author: YAMAMOTO Takashi (yamt)

Changes

This reverts commit 28f2fb2.

The previous attempt was breaking things like __start_ symbols.

This time, use a separate flag instead of abusing ABOSULUTE.

Full diff: https://github.com/llvm/llvm-project/pull/154494.diff

4 Files Affected:

(modified) lld/test/wasm/pie.s (+24-3)
(modified) lld/wasm/Driver.cpp (+3-1)
(modified) lld/wasm/Symbols.h (+4-1)
(modified) lld/wasm/SyntheticSections.cpp (+3-1)

diff --git a/lld/test/wasm/pie.s b/lld/test/wasm/pie.s
index 21eac79207318..78d6c85097b10 100644
--- a/lld/test/wasm/pie.s
+++ b/lld/test/wasm/pie.s
@@ -2,7 +2,7 @@
 # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-emscripten %S/Inputs/internal_func.s -o %t.internal_func.o
 # RUN: wasm-ld --no-gc-sections --experimental-pic -pie --unresolved-symbols=import-dynamic -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
+# RUN: llvm-objdump --disassemble-symbols=__wasm_call_ctors,__wasm_apply_data_relocs,__wasm_apply_global_relocs --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DISASSEM
 
 .functype external_func () -> ()
 .functype internal_func1 () -> (i32)
@@ -41,6 +41,9 @@ foo:
   drop
 
   global.get __stack_pointer
+  drop
+
+  global.get __wasm_first_page_end@GOT
   end_function
 
 get_data_address:
@@ -145,6 +148,18 @@ _start:
 # DISASSEM-LABEL:  <__wasm_apply_data_relocs>:
 # DISASSEM:        end
 
+# global 6 is __wasm_first_page_end, which is noReloc and
+# thus should not be relocated.
+#
+# DISASSEM-LABEL:  <__wasm_apply_global_relocs>:
+# DISASSEM:        global.set      4
+# DISASSEM:        global.set      5
+# DISASSEM-NOT:    global.set      6
+# DISASSEM:        global.set      7
+# DISASSEM:        global.set      8
+# DISASSEM:        global.set      9
+# 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
@@ -173,17 +188,23 @@ _start:
 # EXTENDED-CONST-NEXT:        Type:            I32
 # EXTENDED-CONST-NEXT:        Mutable:         false
 # EXTENDED-CONST-NEXT:        InitExpr:
+# EXTENDED-CONST-NEXT:          Opcode:        I32_CONST
+# EXTENDED-CONST-NEXT:          Value:         65536
+# EXTENDED-CONST-NEXT:      - Index:           7
+# 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:      - Index:           8
 # 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:      - Index:           9
 # EXTENDED-CONST-NEXT:        Type:            I32
 # EXTENDED-CONST-NEXT:        Mutable:         false
 # EXTENDED-CONST-NEXT:        InitExpr:
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 1c5d21c06f5af..5ad8c1bde289d 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -999,8 +999,10 @@ static void createOptionalSymbols() {
   }
 
   ctx.sym.firstPageEnd = symtab->addOptionalDataSymbol("__wasm_first_page_end");
-  if (ctx.sym.firstPageEnd)
+  if (ctx.sym.firstPageEnd) {
     ctx.sym.firstPageEnd->setVA(ctx.arg.pageSize);
+    ctx.sym.firstPageEnd->noReloc = true;
+  }
 
   // For non-shared memory programs we still need to define __tls_base since we
   // allow object files built with TLS to be linked into single threaded
diff --git a/lld/wasm/Symbols.h b/lld/wasm/Symbols.h
index 55ee21939ce07..caee04865545d 100644
--- a/lld/wasm/Symbols.h
+++ b/lld/wasm/Symbols.h
@@ -142,7 +142,7 @@ class Symbol {
       : name(name), file(f), symbolKind(k), referenced(!ctx.arg.gcSections),
         requiresGOT(false), isUsedInRegularObj(false), forceExport(false),
         forceImport(false), canInline(false), traced(false), isStub(false),
-        flags(flags) {}
+        noReloc(false), flags(flags) {}
 
   StringRef name;
   InputFile *file;
@@ -185,6 +185,9 @@ class Symbol {
   // the null function pointer).
   bool isStub : 1;
 
+  // True if this symbol is not relative to __memory_base.
+  bool noReloc : 1;
+
   uint32_t flags;
 
   std::optional<StringRef> importName;
diff --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp
index e1192706ea913..4d0c5abf799db 100644
--- a/lld/wasm/SyntheticSections.cpp
+++ b/lld/wasm/SyntheticSections.cpp
@@ -440,6 +440,8 @@ void GlobalSection::generateRelocationCode(raw_ostream &os, bool TLS) const {
                                  : WASM_OPCODE_I32_ADD;
 
   for (const Symbol *sym : internalGotSymbols) {
+    if (sym->noReloc)
+      continue;
     if (TLS != sym->isTLS())
       continue;
 
@@ -503,7 +505,7 @@ void GlobalSection::writeBody() {
     bool useExtendedConst = false;
     uint32_t globalIdx;
     int64_t offset;
-    if (ctx.arg.extendedConst && ctx.isPic) {
+    if (ctx.arg.extendedConst && ctx.isPic && !sym->noReloc) {
       if (auto *d = dyn_cast<DefinedData>(sym)) {
         if (!sym->isTLS()) {
           globalIdx = ctx.sym.memoryBase->getGlobalIndex();

llvmbot · 2025-08-20T09:22:12Z

@llvm/pr-subscribers-lld-wasm

Author: YAMAMOTO Takashi (yamt)

Changes

This reverts commit 28f2fb2.

The previous attempt was breaking things like __start_ symbols.

This time, use a separate flag instead of abusing ABOSULUTE.

Full diff: https://github.com/llvm/llvm-project/pull/154494.diff

4 Files Affected:

(modified) lld/test/wasm/pie.s (+24-3)
(modified) lld/wasm/Driver.cpp (+3-1)
(modified) lld/wasm/Symbols.h (+4-1)
(modified) lld/wasm/SyntheticSections.cpp (+3-1)

diff --git a/lld/test/wasm/pie.s b/lld/test/wasm/pie.s
index 21eac79207318..78d6c85097b10 100644
--- a/lld/test/wasm/pie.s
+++ b/lld/test/wasm/pie.s
@@ -2,7 +2,7 @@
 # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-emscripten %S/Inputs/internal_func.s -o %t.internal_func.o
 # RUN: wasm-ld --no-gc-sections --experimental-pic -pie --unresolved-symbols=import-dynamic -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
+# RUN: llvm-objdump --disassemble-symbols=__wasm_call_ctors,__wasm_apply_data_relocs,__wasm_apply_global_relocs --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DISASSEM
 
 .functype external_func () -> ()
 .functype internal_func1 () -> (i32)
@@ -41,6 +41,9 @@ foo:
   drop
 
   global.get __stack_pointer
+  drop
+
+  global.get __wasm_first_page_end@GOT
   end_function
 
 get_data_address:
@@ -145,6 +148,18 @@ _start:
 # DISASSEM-LABEL:  <__wasm_apply_data_relocs>:
 # DISASSEM:        end
 
+# global 6 is __wasm_first_page_end, which is noReloc and
+# thus should not be relocated.
+#
+# DISASSEM-LABEL:  <__wasm_apply_global_relocs>:
+# DISASSEM:        global.set      4
+# DISASSEM:        global.set      5
+# DISASSEM-NOT:    global.set      6
+# DISASSEM:        global.set      7
+# DISASSEM:        global.set      8
+# DISASSEM:        global.set      9
+# 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
@@ -173,17 +188,23 @@ _start:
 # EXTENDED-CONST-NEXT:        Type:            I32
 # EXTENDED-CONST-NEXT:        Mutable:         false
 # EXTENDED-CONST-NEXT:        InitExpr:
+# EXTENDED-CONST-NEXT:          Opcode:        I32_CONST
+# EXTENDED-CONST-NEXT:          Value:         65536
+# EXTENDED-CONST-NEXT:      - Index:           7
+# 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:      - Index:           8
 # 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:      - Index:           9
 # EXTENDED-CONST-NEXT:        Type:            I32
 # EXTENDED-CONST-NEXT:        Mutable:         false
 # EXTENDED-CONST-NEXT:        InitExpr:
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 1c5d21c06f5af..5ad8c1bde289d 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -999,8 +999,10 @@ static void createOptionalSymbols() {
   }
 
   ctx.sym.firstPageEnd = symtab->addOptionalDataSymbol("__wasm_first_page_end");
-  if (ctx.sym.firstPageEnd)
+  if (ctx.sym.firstPageEnd) {
     ctx.sym.firstPageEnd->setVA(ctx.arg.pageSize);
+    ctx.sym.firstPageEnd->noReloc = true;
+  }
 
   // For non-shared memory programs we still need to define __tls_base since we
   // allow object files built with TLS to be linked into single threaded
diff --git a/lld/wasm/Symbols.h b/lld/wasm/Symbols.h
index 55ee21939ce07..caee04865545d 100644
--- a/lld/wasm/Symbols.h
+++ b/lld/wasm/Symbols.h
@@ -142,7 +142,7 @@ class Symbol {
       : name(name), file(f), symbolKind(k), referenced(!ctx.arg.gcSections),
         requiresGOT(false), isUsedInRegularObj(false), forceExport(false),
         forceImport(false), canInline(false), traced(false), isStub(false),
-        flags(flags) {}
+        noReloc(false), flags(flags) {}
 
   StringRef name;
   InputFile *file;
@@ -185,6 +185,9 @@ class Symbol {
   // the null function pointer).
   bool isStub : 1;
 
+  // True if this symbol is not relative to __memory_base.
+  bool noReloc : 1;
+
   uint32_t flags;
 
   std::optional<StringRef> importName;
diff --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp
index e1192706ea913..4d0c5abf799db 100644
--- a/lld/wasm/SyntheticSections.cpp
+++ b/lld/wasm/SyntheticSections.cpp
@@ -440,6 +440,8 @@ void GlobalSection::generateRelocationCode(raw_ostream &os, bool TLS) const {
                                  : WASM_OPCODE_I32_ADD;
 
   for (const Symbol *sym : internalGotSymbols) {
+    if (sym->noReloc)
+      continue;
     if (TLS != sym->isTLS())
       continue;
 
@@ -503,7 +505,7 @@ void GlobalSection::writeBody() {
     bool useExtendedConst = false;
     uint32_t globalIdx;
     int64_t offset;
-    if (ctx.arg.extendedConst && ctx.isPic) {
+    if (ctx.arg.extendedConst && ctx.isPic && !sym->noReloc) {
       if (auto *d = dyn_cast<DefinedData>(sym)) {
         if (!sym->isTLS()) {
           globalIdx = ctx.sym.memoryBase->getGlobalIndex();

sbc100 · 2025-08-20T18:31:00Z

I'm not totally sure this is that right approach... i think maybe the existing ABSOLUTE symbol flag might the right thing, but we shouldn't be using to other symbols.

I think we need to look into it some more. What is the existing ABSOLUTE flag doing? How does it compare the the corresponding flag in the ELF linker? etc.

yamt · 2025-08-21T03:26:20Z

I'm not totally sure this is that right approach... i think maybe the existing ABSOLUTE symbol flag might the right thing, but we shouldn't be using to other symbols.

I think we need to look into it some more. What is the existing ABSOLUTE flag doing? How does it compare the the corresponding flag in the ELF linker? etc.

iiuc, the existing ABSOLUTE flag means relative to __memory_base/__table_base. (instead of being relative to a data segment.)
i feel the name is misleading.

i have no idea about ELF.
do you mean elf::isAbsolute?

yamt · 2025-08-22T02:06:15Z

i made a bit research on the existing ABSOLUTE symbols.

relative to __memory_base:

    __start_<secname>
    __stop_<secname>
    __data_end
    __dso_handle
    __wasm_init_memory_flag

undefined/imported for pic:

    __stack_low
    __stack_high
    __global_base
    __heap_base
    __heap_end
    __memory_base
    __table_base

???:

    __wasm_first_page_end

yamt · 2025-08-22T02:14:42Z

i made a bit research on the existing ABSOLUTE symbols.

relative to __memory_base:

    __start_<secname>
    __stop_<secname>
    __data_end
    __dso_handle
    __wasm_init_memory_flag

undefined/imported for pic:

    __stack_low
    __stack_high
    __global_base
    __heap_base
    __heap_end
    __memory_base
    __table_base

???:

    __wasm_first_page_end

@sbc100
btw, do you remember why __global_base and __data_end are treated differently?

yamt · 2025-08-24T22:18:50Z

I think we need to look into it some more. What is the existing ABSOLUTE flag doing? How does it compare the the corresponding flag in the ELF linker? etc.

i couldn't find the corresponding flag in ELF.
if i read the code correctly, in ELF, it seems that __start_/__stop_ symbols are relative (isAbsolute returns false) to the section which they belong to.

yamt · 2025-08-26T04:05:32Z

maybe we can remove the existing use of ABSOLUTE to follow ELF and then introduce a new "really-ABOSOLUTE" flag for wasm_first_page_end.
but i feel it might be rather confusing because the current ABSOLUTE flag is user-visible in object files.
what do you think?

yamt · 2025-09-16T23:59:05Z

anyway, as the semantics of __wasm_first_page_end is rather exceptional, i feel the current approach of this PR is not too bad.

sbc100 · 2025-09-17T21:00:15Z

I agree the __wasm_first_page_end is exceptional, yes. I'm not sure its worth adding a new field to every symbol just to support this though. Maybe its the only way. Sorry I've not had time to think more on this yet.

Reapply "[lld][WebAssembly] Do not relocate ABSOLUTE symbols" (llvm#1…

576af7e

…54371) This reverts commit 28f2fb2. The previous attempt was breaking things like __start_ symbols. This time, use a separate flag instead of abusing ABOSULUTE.

llvmbot added lld lld:wasm labels Aug 20, 2025

yamt mentioned this pull request Aug 20, 2025

[lld][WebAssembly] Do not relocate ABSOLUTE symbols #153763

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Reapply "[lld][WebAssembly] Do not relocate ABSOLUTE symbols" (#154371) #154494

Reapply "[lld][WebAssembly] Do not relocate ABSOLUTE symbols" (#154371) #154494

Uh oh!

yamt commented Aug 20, 2025 •

edited

Loading

Uh oh!

llvmbot commented Aug 20, 2025

Uh oh!

llvmbot commented Aug 20, 2025

Uh oh!

sbc100 commented Aug 20, 2025

Uh oh!

yamt commented Aug 21, 2025

Uh oh!

yamt commented Aug 22, 2025

Uh oh!

yamt commented Aug 22, 2025

Uh oh!

yamt commented Aug 24, 2025

Uh oh!

yamt commented Aug 26, 2025

Uh oh!

yamt commented Sep 16, 2025

Uh oh!

sbc100 commented Sep 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Reapply "[lld][WebAssembly] Do not relocate ABSOLUTE symbols" (#154371) #154494

Are you sure you want to change the base?

Reapply "[lld][WebAssembly] Do not relocate ABSOLUTE symbols" (#154371) #154494

Uh oh!

Conversation

yamt commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 20, 2025

Uh oh!

llvmbot commented Aug 20, 2025

Uh oh!

sbc100 commented Aug 20, 2025

Uh oh!

yamt commented Aug 21, 2025

Uh oh!

yamt commented Aug 22, 2025

Uh oh!

yamt commented Aug 22, 2025

Uh oh!

yamt commented Aug 24, 2025

Uh oh!

yamt commented Aug 26, 2025

Uh oh!

yamt commented Sep 16, 2025

Uh oh!

sbc100 commented Sep 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

yamt commented Aug 20, 2025 •

edited

Loading