Skip to content

[lld][WebAssembly] Fix TLS-relative relocations when linking without shared memory #116136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lld/test/wasm/tls-non-shared-memory.s
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ tls1:

# RUN: wasm-ld --no-gc-sections --no-entry -o %t.wasm %t.o
# RUN: obj2yaml %t.wasm | FileCheck %s
# RUN: llvm-objdump --disassemble-symbols=get_tls1 --no-show-raw-insn --no-leading-addr %t.wasm | FileCheck %s --check-prefixes DIS

# RUN: wasm-ld --experimental-pic -shared -o %t.so %t.o
# RUN: obj2yaml %t.so | FileCheck %s --check-prefixes=SHARED,PIC
Expand Down Expand Up @@ -97,6 +98,14 @@ tls1:
# CHECK-NEXT: Content: 2A000000
# CHECK-NEXT: - Type: CUSTOM

# The constant value here which we add to `__tls_base` should not be absolute
# but relative to `__tls_base`, in this case zero rather than 1024.
# DIS: <get_tls1>:
# DIS-EMPTY:
# DIS-NEXT: global.get 1
# DIS-NEXT: i32.const 0
# DIS-NEXT: i32.add
# DIS-NEXT: end

# In PIC mode we expect TLS data and non-TLS data to be merged into
# a single segment which is initialized via the __memory_base import
Expand Down
9 changes: 4 additions & 5 deletions lld/wasm/Symbols.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,11 @@ uint32_t DefinedFunction::getExportedFunctionIndex() const {
return function->getFunctionIndex();
}

uint64_t DefinedData::getVA() const {
uint64_t DefinedData::getVA(bool absolute) const {
LLVM_DEBUG(dbgs() << "getVA: " << getName() << "\n");
// In the shared memory case, TLS symbols are relative to the start of the TLS
// output segment (__tls_base). When building without shared memory, TLS
// symbols absolute, just like non-TLS.
if (isTLS() && config->sharedMemory)
// TLS symbols (by default) are relative to the start of the TLS output
// segment (__tls_base).
if (isTLS() && !absolute)
return getOutputSegmentOffset();
if (segment)
return segment->getVA(value);
Expand Down
4 changes: 3 additions & 1 deletion lld/wasm/Symbols.h
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,9 @@ class DefinedData : public DataSymbol {
static bool classof(const Symbol *s) { return s->kind() == DefinedDataKind; }

// Returns the output virtual address of a defined data symbol.
uint64_t getVA() const;
// For TLS symbols, by default (unless absolute is set), this returns an
// address relative the `__tls_base`.
uint64_t getVA(bool absolute = false) const;
void setVA(uint64_t va);

// Returns the offset of a defined data symbol within its OutputSegment.
Expand Down
5 changes: 4 additions & 1 deletion lld/wasm/SyntheticSections.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,10 @@ void GlobalSection::writeBody() {
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the extended-const case also need to be updated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No since in that case we do the addition with __tls_base as part of the const expression. i.e. its don't for us at instantiation time.

So I guess there are 3 ways this addition can happen:

  1. At instantiation time via extended const
  2. As part of __wasm_apply_global_tls_relocs at runtime
  3. At static link time via getVA(absolute=true)

WasmInitExpr initExpr;
if (auto *d = dyn_cast<DefinedData>(sym))
initExpr = intConst(d->getVA(), is64);
// In the sharedMemory case TLS globals are set during
// `__wasm_apply_global_tls_relocs`, but in the non-shared case
// we know the absolute value at link time.
initExpr = intConst(d->getVA(/*absolute=*/!config->sharedMemory), is64);
else if (auto *f = dyn_cast<FunctionSymbol>(sym))
initExpr = intConst(f->isStub ? 0 : f->getTableIndex(), is64);
else {
Expand Down
Loading