[wasm-ld] Refactor WasmSym from static globals to per-link context #134970
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Use case as per #134809 While running clang-repl in the browser, we would have
However, we encountered a state-leak problem in wasm-ld: The WasmSym::initMemory symbol, once created in one link (e.g., for X.wasm), persists into subsequent linker runs (e.g., for Y.wasm) due to WasmSym being globally defined with static fields. This causes inconsistencies such as createInitMemoryFunction() being triggered when hasPassiveInitializedSegments() is false — leading to crashes/assertions. This surfaced as a misalignment between the actual linking context and the static WasmSym state, especially in scenarios where no segments required passive initialization, but initMemory was still marked live from a previous link. |
|
@llvm/pr-subscribers-lld-wasm @llvm/pr-subscribers-lld Author: Anutosh Bhat (anutosh491) ChangesTowards ##134809 (comment) This change moves WasmSym from a static global struct to an instance owned by Ctx, allowing it to be reset cleanly between linker runs. This enables safe support for multiple invocations of wasm-ld within the same process Patch is 41.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/134970.diff 9 Files Affected:
diff --git a/lld/wasm/Config.h b/lld/wasm/Config.h
index ffc5b84ca38b0..ad94b2a1386dc 100644
--- a/lld/wasm/Config.h
+++ b/lld/wasm/Config.h
@@ -32,6 +32,7 @@ class InputTable;
class InputGlobal;
class InputFunction;
class Symbol;
+struct WasmSym;
// For --unresolved-symbols.
enum class UnresolvedPolicy { ReportError, Warn, Ignore, ImportDynamic };
@@ -141,6 +142,9 @@ struct Ctx {
llvm::SmallVector<InputGlobal *, 0> syntheticGlobals;
llvm::SmallVector<InputTable *, 0> syntheticTables;
+ // Linker-generated symbols like __wasm_init_memory, __heap_base, etc.
+ std::unique_ptr<WasmSym> wasmSym;
+
// True if we are creating position-independent code.
bool isPic = false;
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index de976947474e1..b43632d5e420b 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -70,6 +70,8 @@ void Ctx::reset() {
isPic = false;
legacyFunctionTable = false;
emitBssSegments = false;
+ wasmSym.reset();
+ wasmSym = std::make_unique<WasmSym>();
}
namespace {
@@ -945,14 +947,14 @@ static void createSyntheticSymbols() {
true};
static llvm::wasm::WasmGlobalType mutableGlobalTypeI64 = {WASM_TYPE_I64,
true};
- WasmSym::callCtors = symtab->addSyntheticFunction(
+ ctx.wasmSym->callCtors = symtab->addSyntheticFunction(
"__wasm_call_ctors", WASM_SYMBOL_VISIBILITY_HIDDEN,
make<SyntheticFunction>(nullSignature, "__wasm_call_ctors"));
bool is64 = ctx.arg.is64.value_or(false);
if (ctx.isPic) {
- WasmSym::stackPointer =
+ ctx.wasmSym->stackPointer =
createUndefinedGlobal("__stack_pointer", ctx.arg.is64.value_or(false)
? &mutableGlobalTypeI64
: &mutableGlobalTypeI32);
@@ -962,21 +964,21 @@ static void createSyntheticSymbols() {
// See:
// https://github.com/WebAssembly/tool-conventions/blob/main/DynamicLinking.md
auto *globalType = is64 ? &globalTypeI64 : &globalTypeI32;
- WasmSym::memoryBase = createUndefinedGlobal("__memory_base", globalType);
- WasmSym::tableBase = createUndefinedGlobal("__table_base", globalType);
- WasmSym::memoryBase->markLive();
- WasmSym::tableBase->markLive();
+ ctx.wasmSym->memoryBase = createUndefinedGlobal("__memory_base", globalType);
+ ctx.wasmSym->tableBase = createUndefinedGlobal("__table_base", globalType);
+ ctx.wasmSym->memoryBase->markLive();
+ ctx.wasmSym->tableBase->markLive();
} else {
// For non-PIC code
- WasmSym::stackPointer = createGlobalVariable("__stack_pointer", true);
- WasmSym::stackPointer->markLive();
+ ctx.wasmSym->stackPointer = createGlobalVariable("__stack_pointer", true);
+ ctx.wasmSym->stackPointer->markLive();
}
if (ctx.arg.sharedMemory) {
- WasmSym::tlsBase = createGlobalVariable("__tls_base", true);
- WasmSym::tlsSize = createGlobalVariable("__tls_size", false);
- WasmSym::tlsAlign = createGlobalVariable("__tls_align", false);
- WasmSym::initTLS = symtab->addSyntheticFunction(
+ ctx.wasmSym->tlsBase = createGlobalVariable("__tls_base", true);
+ ctx.wasmSym->tlsSize = createGlobalVariable("__tls_size", false);
+ ctx.wasmSym->tlsAlign = createGlobalVariable("__tls_align", false);
+ ctx.wasmSym->initTLS = symtab->addSyntheticFunction(
"__wasm_init_tls", WASM_SYMBOL_VISIBILITY_HIDDEN,
make<SyntheticFunction>(
is64 ? i64ArgSignature : i32ArgSignature,
@@ -988,25 +990,25 @@ static void createOptionalSymbols() {
if (ctx.arg.relocatable)
return;
- WasmSym::dsoHandle = symtab->addOptionalDataSymbol("__dso_handle");
+ ctx.wasmSym->dsoHandle = symtab->addOptionalDataSymbol("__dso_handle");
if (!ctx.arg.shared)
- WasmSym::dataEnd = symtab->addOptionalDataSymbol("__data_end");
+ ctx.wasmSym->dataEnd = symtab->addOptionalDataSymbol("__data_end");
if (!ctx.isPic) {
- WasmSym::stackLow = symtab->addOptionalDataSymbol("__stack_low");
- WasmSym::stackHigh = symtab->addOptionalDataSymbol("__stack_high");
- WasmSym::globalBase = symtab->addOptionalDataSymbol("__global_base");
- WasmSym::heapBase = symtab->addOptionalDataSymbol("__heap_base");
- WasmSym::heapEnd = symtab->addOptionalDataSymbol("__heap_end");
- WasmSym::definedMemoryBase = symtab->addOptionalDataSymbol("__memory_base");
- WasmSym::definedTableBase = symtab->addOptionalDataSymbol("__table_base");
+ ctx.wasmSym->stackLow = symtab->addOptionalDataSymbol("__stack_low");
+ ctx.wasmSym->stackHigh = symtab->addOptionalDataSymbol("__stack_high");
+ ctx.wasmSym->globalBase = symtab->addOptionalDataSymbol("__global_base");
+ ctx.wasmSym->heapBase = symtab->addOptionalDataSymbol("__heap_base");
+ ctx.wasmSym->heapEnd = symtab->addOptionalDataSymbol("__heap_end");
+ ctx.wasmSym->definedMemoryBase = symtab->addOptionalDataSymbol("__memory_base");
+ ctx.wasmSym->definedTableBase = symtab->addOptionalDataSymbol("__table_base");
}
- WasmSym::firstPageEnd =
+ ctx.wasmSym->firstPageEnd =
symtab->addOptionalDataSymbol("__wasm_first_page_end");
- if (WasmSym::firstPageEnd)
- WasmSym::firstPageEnd->setVA(ctx.arg.pageSize);
+ if (ctx.wasmSym->firstPageEnd)
+ ctx.wasmSym->firstPageEnd->setVA(ctx.arg.pageSize);
// 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
@@ -1018,7 +1020,7 @@ static void createOptionalSymbols() {
// __tls_size and __tls_align are not needed in this case since they are only
// needed for __wasm_init_tls (which we do not create in this case).
if (!ctx.arg.sharedMemory)
- WasmSym::tlsBase = createOptionalGlobal("__tls_base", false);
+ ctx.wasmSym->tlsBase = createOptionalGlobal("__tls_base", false);
}
static void processStubLibrariesPreLTO() {
@@ -1393,9 +1395,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
// by libc/etc., because destructors are registered dynamically with
// `__cxa_atexit` and friends.
if (!ctx.arg.relocatable && !ctx.arg.shared &&
- !WasmSym::callCtors->isUsedInRegularObj &&
- WasmSym::callCtors->getName() != ctx.arg.entry &&
- !ctx.arg.exportedSymbols.count(WasmSym::callCtors->getName())) {
+ !ctx.wasmSym->callCtors->isUsedInRegularObj &&
+ ctx.wasmSym->callCtors->getName() != ctx.arg.entry &&
+ !ctx.arg.exportedSymbols.count(ctx.wasmSym->callCtors->getName())) {
if (Symbol *callDtors =
handleUndefined("__wasm_call_dtors", "<internal>")) {
if (auto *callDtorsFunc = dyn_cast<DefinedFunction>(callDtors)) {
@@ -1404,7 +1406,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
!callDtorsFunc->signature->Returns.empty())) {
error("__wasm_call_dtors must have no argument or return values");
}
- WasmSym::callDtors = callDtorsFunc;
+ ctx.wasmSym->callDtors = callDtorsFunc;
} else {
error("__wasm_call_dtors must be a function");
}
@@ -1497,7 +1499,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
markLive();
// Provide the indirect function table if needed.
- WasmSym::indirectFunctionTable =
+ ctx.wasmSym->indirectFunctionTable =
symtab->resolveIndirectFunctionTable(/*required =*/false);
if (errorCount())
diff --git a/lld/wasm/InputChunks.cpp b/lld/wasm/InputChunks.cpp
index 3668697382238..ac89f313ca4b5 100644
--- a/lld/wasm/InputChunks.cpp
+++ b/lld/wasm/InputChunks.cpp
@@ -411,9 +411,9 @@ bool InputChunk::generateRelocationCode(raw_ostream &os) const {
if (ctx.isPic) {
writeU8(os, WASM_OPCODE_GLOBAL_GET, "GLOBAL_GET");
if (isTLS())
- writeUleb128(os, WasmSym::tlsBase->getGlobalIndex(), "tls_base");
+ writeUleb128(os, ctx.wasmSym->tlsBase->getGlobalIndex(), "tls_base");
else
- writeUleb128(os, WasmSym::memoryBase->getGlobalIndex(), "memory_base");
+ writeUleb128(os, ctx.wasmSym->memoryBase->getGlobalIndex(), "memory_base");
writeU8(os, opcode_ptr_add, "ADD");
}
@@ -436,12 +436,12 @@ bool InputChunk::generateRelocationCode(raw_ostream &os) const {
}
} else {
assert(ctx.isPic);
- const GlobalSymbol* baseSymbol = WasmSym::memoryBase;
+ const GlobalSymbol* baseSymbol = ctx.wasmSym->memoryBase;
if (rel.Type == R_WASM_TABLE_INDEX_I32 ||
rel.Type == R_WASM_TABLE_INDEX_I64)
- baseSymbol = WasmSym::tableBase;
+ baseSymbol = ctx.wasmSym->tableBase;
else if (sym->isTLS())
- baseSymbol = WasmSym::tlsBase;
+ baseSymbol = ctx.wasmSym->tlsBase;
writeU8(os, WASM_OPCODE_GLOBAL_GET, "GLOBAL_GET");
writeUleb128(os, baseSymbol->getGlobalIndex(), "base");
writeU8(os, opcode_reloc_const, "CONST");
diff --git a/lld/wasm/MarkLive.cpp b/lld/wasm/MarkLive.cpp
index 13c7a3d894fe3..40b105821056d 100644
--- a/lld/wasm/MarkLive.cpp
+++ b/lld/wasm/MarkLive.cpp
@@ -114,8 +114,8 @@ void MarkLive::run() {
if (sym->isNoStrip() || sym->isExported())
enqueue(sym);
- if (WasmSym::callDtors)
- enqueue(WasmSym::callDtors);
+ if (ctx.wasmSym->callDtors)
+ enqueue(ctx.wasmSym->callDtors);
for (const ObjFile *obj : ctx.objectFiles)
if (obj->isLive()) {
@@ -131,7 +131,7 @@ void MarkLive::run() {
// If we have any non-discarded init functions, mark `__wasm_call_ctors` as
// live so that we assign it an index and call it.
if (isCallCtorsLive())
- WasmSym::callCtors->markLive();
+ ctx.wasmSym->callCtors->markLive();
}
void MarkLive::mark() {
diff --git a/lld/wasm/OutputSections.cpp b/lld/wasm/OutputSections.cpp
index 5038cd8ea965b..1864e4532fc36 100644
--- a/lld/wasm/OutputSections.cpp
+++ b/lld/wasm/OutputSections.cpp
@@ -124,7 +124,7 @@ void DataSection::finalizeContents() {
if ((segment->initFlags & WASM_DATA_SEGMENT_IS_PASSIVE) == 0) {
if (ctx.isPic && ctx.arg.extendedConst) {
writeU8(os, WASM_OPCODE_GLOBAL_GET, "global get");
- writeUleb128(os, WasmSym::memoryBase->getGlobalIndex(),
+ writeUleb128(os, ctx.wasmSym->memoryBase->getGlobalIndex(),
"literal (global index)");
if (segment->startVA) {
writePtrConst(os, segment->startVA, is64, "offset");
@@ -137,7 +137,7 @@ void DataSection::finalizeContents() {
if (ctx.isPic) {
assert(segment->startVA == 0);
initExpr.Inst.Opcode = WASM_OPCODE_GLOBAL_GET;
- initExpr.Inst.Value.Global = WasmSym::memoryBase->getGlobalIndex();
+ initExpr.Inst.Value.Global = ctx.wasmSym->memoryBase->getGlobalIndex();
} else {
initExpr = intConst(segment->startVA, is64);
}
diff --git a/lld/wasm/Symbols.cpp b/lld/wasm/Symbols.cpp
index af5ecf4f38f73..f2040441e6257 100644
--- a/lld/wasm/Symbols.cpp
+++ b/lld/wasm/Symbols.cpp
@@ -77,32 +77,6 @@ std::string toString(wasm::Symbol::Kind kind) {
}
namespace wasm {
-DefinedFunction *WasmSym::callCtors;
-DefinedFunction *WasmSym::callDtors;
-DefinedFunction *WasmSym::initMemory;
-DefinedFunction *WasmSym::applyGlobalRelocs;
-DefinedFunction *WasmSym::applyTLSRelocs;
-DefinedFunction *WasmSym::applyGlobalTLSRelocs;
-DefinedFunction *WasmSym::initTLS;
-DefinedData *WasmSym::firstPageEnd;
-DefinedFunction *WasmSym::startFunction;
-DefinedData *WasmSym::dsoHandle;
-DefinedData *WasmSym::dataEnd;
-DefinedData *WasmSym::globalBase;
-DefinedData *WasmSym::heapBase;
-DefinedData *WasmSym::heapEnd;
-DefinedData *WasmSym::initMemoryFlag;
-GlobalSymbol *WasmSym::stackPointer;
-DefinedData *WasmSym::stackLow;
-DefinedData *WasmSym::stackHigh;
-GlobalSymbol *WasmSym::tlsBase;
-GlobalSymbol *WasmSym::tlsSize;
-GlobalSymbol *WasmSym::tlsAlign;
-UndefinedGlobal *WasmSym::tableBase;
-DefinedData *WasmSym::definedTableBase;
-UndefinedGlobal *WasmSym::memoryBase;
-DefinedData *WasmSym::definedMemoryBase;
-TableSymbol *WasmSym::indirectFunctionTable;
WasmSymbolType Symbol::getWasmType() const {
if (isa<FunctionSymbol>(this))
diff --git a/lld/wasm/Symbols.h b/lld/wasm/Symbols.h
index 03a74da7230d0..4197fc7a17b14 100644
--- a/lld/wasm/Symbols.h
+++ b/lld/wasm/Symbols.h
@@ -541,32 +541,32 @@ class LazySymbol : public Symbol {
struct WasmSym {
// __global_base
// Symbol marking the start of the global section.
- static DefinedData *globalBase;
+ DefinedData *globalBase = nullptr;
// __stack_pointer/__stack_low/__stack_high
// Global that holds current value of stack pointer and data symbols marking
// the start and end of the stack region. stackPointer is initialized to
// stackHigh and grows downwards towards stackLow
- static GlobalSymbol *stackPointer;
- static DefinedData *stackLow;
- static DefinedData *stackHigh;
+ GlobalSymbol *stackPointer = nullptr;
+ DefinedData *stackLow = nullptr;
+ DefinedData *stackHigh = nullptr;
// __tls_base
// Global that holds the address of the base of the current thread's
// TLS block.
- static GlobalSymbol *tlsBase;
+ GlobalSymbol *tlsBase = nullptr;
// __tls_size
// Symbol whose value is the size of the TLS block.
- static GlobalSymbol *tlsSize;
+ GlobalSymbol *tlsSize = nullptr;
// __tls_size
// Symbol whose value is the alignment of the TLS block.
- static GlobalSymbol *tlsAlign;
+ GlobalSymbol *tlsAlign = nullptr;
// __data_end
// Symbol marking the end of the data and bss.
- static DefinedData *dataEnd;
+ DefinedData *dataEnd = nullptr;
// __heap_base/__heap_end
// Symbols marking the beginning and end of the "heap". It starts at the end
@@ -574,70 +574,70 @@ struct WasmSym {
// memory allocated by wasm-ld. This region of memory is not used by the
// linked code, so it may be used as a backing store for `sbrk` or `malloc`
// implementations.
- static DefinedData *heapBase;
- static DefinedData *heapEnd;
+ DefinedData *heapBase = nullptr;
+ DefinedData *heapEnd = nullptr;
// __wasm_first_page_end
// A symbol whose address is the end of the first page in memory (if any).
- static DefinedData *firstPageEnd;
+ DefinedData *firstPageEnd = nullptr;
// __wasm_init_memory_flag
// Symbol whose contents are nonzero iff memory has already been initialized.
- static DefinedData *initMemoryFlag;
+ DefinedData *initMemoryFlag = nullptr;
// __wasm_init_memory
// Function that initializes passive data segments during instantiation.
- static DefinedFunction *initMemory;
+ DefinedFunction *initMemory = nullptr;
// __wasm_call_ctors
// Function that directly calls all ctors in priority order.
- static DefinedFunction *callCtors;
+ DefinedFunction *callCtors = nullptr;
// __wasm_call_dtors
// Function that calls the libc/etc. cleanup function.
- static DefinedFunction *callDtors;
+ DefinedFunction *callDtors = nullptr;
// __wasm_apply_global_relocs
// Function that applies relocations to wasm globals post-instantiation.
// Unlike __wasm_apply_data_relocs this needs to run on every thread.
- static DefinedFunction *applyGlobalRelocs;
+ DefinedFunction *applyGlobalRelocs = nullptr;
// __wasm_apply_tls_relocs
// Like __wasm_apply_data_relocs but for TLS section. These must be
// delayed until __wasm_init_tls.
- static DefinedFunction *applyTLSRelocs;
+ DefinedFunction *applyTLSRelocs = nullptr;
// __wasm_apply_global_tls_relocs
// Like applyGlobalRelocs but for globals that hold TLS addresses. These
// must be delayed until __wasm_init_tls.
- static DefinedFunction *applyGlobalTLSRelocs;
+ DefinedFunction *applyGlobalTLSRelocs = nullptr;
// __wasm_init_tls
// Function that allocates thread-local storage and initializes it.
- static DefinedFunction *initTLS;
+ DefinedFunction *initTLS = nullptr;
// Pointer to the function that is to be used in the start section.
// (normally an alias of initMemory, or applyGlobalRelocs).
- static DefinedFunction *startFunction;
+ DefinedFunction *startFunction = nullptr;
// __dso_handle
// Symbol used in calls to __cxa_atexit to determine current DLL
- static DefinedData *dsoHandle;
+ DefinedData *dsoHandle = nullptr;
// __table_base
// Used in PIC code for offset of indirect function table
- static UndefinedGlobal *tableBase;
- static DefinedData *definedTableBase;
+ UndefinedGlobal *tableBase = nullptr;
+ DefinedData *definedTableBase = nullptr;
// __memory_base
// Used in PIC code for offset of global data
- static UndefinedGlobal *memoryBase;
- static DefinedData *definedMemoryBase;
+ UndefinedGlobal *memoryBase = nullptr;
+ DefinedData *definedMemoryBase = nullptr;
// __indirect_function_table
// Used as an address space for function pointers, with each function that is
// used as a function pointer being allocated a slot.
- static TableSymbol *indirectFunctionTable;
+ TableSymbol *indirectFunctionTable = nullptr;
};
// A buffer class that is large enough to hold any Symbol-derived
diff --git a/lld/wasm/SyntheticSections.cpp b/lld/wasm/SyntheticSections.cpp
index c88a6f742c35e..ce71e8f915fef 100644
--- a/lld/wasm/SyntheticSections.cpp
+++ b/lld/wasm/SyntheticSections.cpp
@@ -328,8 +328,8 @@ void TableSection::addTable(InputTable *table) {
// Some inputs require that the indirect function table be assigned to table
// number 0.
if (ctx.legacyFunctionTable &&
- isa<DefinedTable>(WasmSym::indirectFunctionTable) &&
- cast<DefinedTable>(WasmSym::indirectFunctionTable)->table == table) {
+ isa<DefinedTable>(ctx.wasmSym->indirectFunctionTable) &&
+ cast<DefinedTable>(ctx.wasmSym->indirectFunctionTable)->table == table) {
if (out.importSec->getNumImportedTables()) {
// Alack! Some other input imported a table, meaning that we are unable
// to assign table number 0 to the indirect function table.
@@ -408,8 +408,8 @@ void GlobalSection::assignIndexes() {
}
static void ensureIndirectFunctionTable() {
- if (!WasmSym::indirectFunctionTable)
- WasmSym::indirectFunctionTable =
+ if (!ctx.wasmSym->indirectFunctionTable)
+ ctx.wasmSym->indirectFunctionTable =
symtab->resolveIndirectFunctionTable(/*required =*/true);
}
@@ -443,9 +443,9 @@ void GlobalSection::generateRelocationCode(raw_ostream &os, bool TLS) const {
// Get __memory_base
writeU8(os, WASM_OPCODE_GLOBAL_GET, "GLOBAL_GET");
if (sym->isTLS())
- writeUleb128(os, WasmSym::tlsBase->getGlobalIndex(), "__tls_base");
+ writeUleb128(os, ctx.wasmSym->tlsBase->getGlobalIndex(), "__tls_base");
else
- writeUleb128(os, WasmSym::memoryBase->getGlobalIndex(),
+ writeUleb128(os, ctx.wasmSym->memoryBase->getGlobalIndex(),
"__memory_base");
// Add the virtual address of the data symbol
@@ -456,7 +456,7 @@ void GlobalSection::generateRelocationCode(raw_ostream &os, bool TLS) const {
continue;
// Get __table_base
writeU8(os, WASM_OPCODE_GLOBAL_GET, "GLOBAL_GET");
- writeUleb128(os, WasmSym::tableBase->getGlobalIndex(), "__table_base");
+ writeUleb128(os, ctx.wasmSym->tableBase->getGlobalIndex(), "__table_base");
// Add the table index to __table_base
writeU8(os, opcode_ptr_const, "CONST");
@@ -503,13 +503,13 @@ void GlobalSection::writeBody() {
if (ctx.arg.extendedConst && ctx.isPic) {
if (auto *d = dyn_cast<DefinedData>(sym)) {
if (!sym->isTLS()) {
- globalIdx = WasmSym::memoryBase->getGlobalIndex();
+ globalIdx = ctx.wasmSym->memoryBase->getGlobalIndex();
offset = d->getVA();
useExtendedConst = true;
}
} else if (auto *f = dyn_cast<FunctionSymbol>(sym)) {
if (!sym->isStub) {
- globalIdx = WasmSym::tableBase->getGlobalIndex();
+ globalIdx = ctx.wasmSym->tableBase->getGlobalIndex();
offset = f->getTableIndex();
useExtendedConst = true;
}
@@ -564,12 +564,12 @@ void ExportSection::writeBody() {
}
bool StartSection::isNeeded() const {
- return WasmSym::startFunction != nullptr;
+ return ctx.wasmSym->startFunction != nullptr;
}
void StartSection::writeBody() {
raw_ostream &os = bodyOutputStream;
- writeUleb128(os, WasmSym::startFunction->getFunct...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Hey @sbc100 Would be great if you could tell me if I am heading towards the correct direction (shall look into why the tests fail after that) |
|
If we want to go the much simpler route where we don't introduce anything in ctx, we can probably just reset the existing static globals in WasmSym to nullptr before each link. Something like this https://github.com/anutosh491/llvm-project/pull/1/files (made the changes in my local fork as an alternative solution) |
sbc100
reviewed
Apr 9, 2025
|
Looks like maybe you need to run |
|
Thanks a lot @sbc100 for the quick reviews here I have made the changes related to git formatting in the latest commit . |
…ultiple wasm-ld invocations
|
Squashed the commits into 2 meaningful commits. The CI on the commit passes too. I think this would now be ready from my side. EDIT: Also tried running the failing usecase I had (#134970 (comment)) with a local build with these changes. Works pretty smoothly ! |
|
Hey @sbc100 , Thanks for the review already but I would like to point out that once you're satisfied with the changes, something that would really help me would be if you could add a milestone label (so that the changes are picked up by llvm 20.1.3) Cause this ends up being a major blocker while running clang-repl in the browser (as one state affects the next state when we run cells one by one). Also should help me migrate xeus-cpp-lite to llvm 20 ! |
|
Gentle ping @sbc100 |
|
Tagging @MaskRay who might be able to help us with the reviews. Also cause I haven't heard from Sam since the PR was approved last week. As discussed above the PR is trying to replicate your work on the ELF linker (03be619#diff-ba5e9bcbcabe54e23f58f9466afb49d9720373eb11b9b3a42ddc2d653b035594) but for wasm-ld. Would be great if you could let me know if anything else is pending here. Thanks. |
|
(I will have limited internet access between April 20th and May 4th, and my response time may be delayed..) This is indeed similar to the ELF port and I hope we can move forward with this, but this needs @sbc100's approval. |
No issues ! Thanks a lot for going through the PR. |
MaskRay
approved these changes
Apr 18, 2025
Thank you ! |
|
Hey @sbc100 This Pr stands approved right now (also approved by you in the past) Would be grateful if you could help us take this in. Thank you ! |
|
This PR seems critical for downstream consumers. I'd like to merge it when the bots are green. |
|
Hey @vgvassilev, Thanks for pushing this as I didn't hear much after the approvals. Could you please also apply a milestone label so that this is picked up for the next release. Thanks again, the CI is green and this should be ready ! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/16649 Here is the relevant piece of the build log for the reference |
|
/cherry-pick 9cbbb74 |
|
Failed to cherry-pick: 9cbbb74 https://github.com/llvm/llvm-project/actions/runs/14705758050 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
|
/cherry-pick 9cbbb74 |
|
Failed to cherry-pick: 9cbbb74 https://github.com/llvm/llvm-project/actions/runs/14706009216 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
anutosh491
added a commit
to anutosh491/llvm-project
that referenced
this pull request
Apr 28, 2025
…lvm#134970) Towards This change moves WasmSym from a static global struct to an instance owned by Ctx, allowing it to be reset cleanly between linker runs. This enables safe support for multiple invocations of wasm-ld within the same process Changes done - Converted WasmSym from a static struct to a regular struct with instance members. - Added a std::unique_ptr<WasmSym> wasmSym field inside Ctx. - Reset wasmSym in Ctx::reset() to clear state between links. - Replaced all WasmSym:: references with ctx.wasmSym->. - Removed global symbol definitions from Symbols.cpp that are no longer needed. Clearing wasmSym in ctx.reset() ensures a clean slate for each link invocation, preventing symbol leakage across runs—critical when using wasm-ld/lld as a reentrant library where global state can cause subtle, hard-to-debug errors. --------- Co-authored-by: Vassil Vassilev <[email protected]> (cherry picked from commit 9cbbb74)
jyli0116
pushed a commit
to jyli0116/llvm-project
that referenced
this pull request
Apr 28, 2025
…lvm#134970) Towards #llvm#134809 (comment) This change moves WasmSym from a static global struct to an instance owned by Ctx, allowing it to be reset cleanly between linker runs. This enables safe support for multiple invocations of wasm-ld within the same process Changes done - Converted WasmSym from a static struct to a regular struct with instance members. - Added a std::unique_ptr<WasmSym> wasmSym field inside Ctx. - Reset wasmSym in Ctx::reset() to clear state between links. - Replaced all WasmSym:: references with ctx.wasmSym->. - Removed global symbol definitions from Symbols.cpp that are no longer needed. Clearing wasmSym in ctx.reset() ensures a clean slate for each link invocation, preventing symbol leakage across runs—critical when using wasm-ld/lld as a reentrant library where global state can cause subtle, hard-to-debug errors. --------- Co-authored-by: Vassil Vassilev <[email protected]>
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…lvm#134970) Towards #llvm#134809 (comment) This change moves WasmSym from a static global struct to an instance owned by Ctx, allowing it to be reset cleanly between linker runs. This enables safe support for multiple invocations of wasm-ld within the same process Changes done - Converted WasmSym from a static struct to a regular struct with instance members. - Added a std::unique_ptr<WasmSym> wasmSym field inside Ctx. - Reset wasmSym in Ctx::reset() to clear state between links. - Replaced all WasmSym:: references with ctx.wasmSym->. - Removed global symbol definitions from Symbols.cpp that are no longer needed. Clearing wasmSym in ctx.reset() ensures a clean slate for each link invocation, preventing symbol leakage across runs—critical when using wasm-ld/lld as a reentrant library where global state can cause subtle, hard-to-debug errors. --------- Co-authored-by: Vassil Vassilev <[email protected]>
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…lvm#134970) Towards #llvm#134809 (comment) This change moves WasmSym from a static global struct to an instance owned by Ctx, allowing it to be reset cleanly between linker runs. This enables safe support for multiple invocations of wasm-ld within the same process Changes done - Converted WasmSym from a static struct to a regular struct with instance members. - Added a std::unique_ptr<WasmSym> wasmSym field inside Ctx. - Reset wasmSym in Ctx::reset() to clear state between links. - Replaced all WasmSym:: references with ctx.wasmSym->. - Removed global symbol definitions from Symbols.cpp that are no longer needed. Clearing wasmSym in ctx.reset() ensures a clean slate for each link invocation, preventing symbol leakage across runs—critical when using wasm-ld/lld as a reentrant library where global state can cause subtle, hard-to-debug errors. --------- Co-authored-by: Vassil Vassilev <[email protected]>
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…lvm#134970) Towards #llvm#134809 (comment) This change moves WasmSym from a static global struct to an instance owned by Ctx, allowing it to be reset cleanly between linker runs. This enables safe support for multiple invocations of wasm-ld within the same process Changes done - Converted WasmSym from a static struct to a regular struct with instance members. - Added a std::unique_ptr<WasmSym> wasmSym field inside Ctx. - Reset wasmSym in Ctx::reset() to clear state between links. - Replaced all WasmSym:: references with ctx.wasmSym->. - Removed global symbol definitions from Symbols.cpp that are no longer needed. Clearing wasmSym in ctx.reset() ensures a clean slate for each link invocation, preventing symbol leakage across runs—critical when using wasm-ld/lld as a reentrant library where global state can cause subtle, hard-to-debug errors. --------- Co-authored-by: Vassil Vassilev <[email protected]>
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
…lvm#134970) Towards #llvm#134809 (comment) This change moves WasmSym from a static global struct to an instance owned by Ctx, allowing it to be reset cleanly between linker runs. This enables safe support for multiple invocations of wasm-ld within the same process Changes done - Converted WasmSym from a static struct to a regular struct with instance members. - Added a std::unique_ptr<WasmSym> wasmSym field inside Ctx. - Reset wasmSym in Ctx::reset() to clear state between links. - Replaced all WasmSym:: references with ctx.wasmSym->. - Removed global symbol definitions from Symbols.cpp that are no longer needed. Clearing wasmSym in ctx.reset() ensures a clean slate for each link invocation, preventing symbol leakage across runs—critical when using wasm-ld/lld as a reentrant library where global state can cause subtle, hard-to-debug errors. --------- Co-authored-by: Vassil Vassilev <[email protected]>
@anutosh491 Were you able to manually create a pull request? |
I upgraded the branch (as some changes had gone in in the release branch) |
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
May 10, 2025
…lvm#134970) Towards This change moves WasmSym from a static global struct to an instance owned by Ctx, allowing it to be reset cleanly between linker runs. This enables safe support for multiple invocations of wasm-ld within the same process Changes done - Converted WasmSym from a static struct to a regular struct with instance members. - Added a std::unique_ptr<WasmSym> wasmSym field inside Ctx. - Reset wasmSym in Ctx::reset() to clear state between links. - Replaced all WasmSym:: references with ctx.wasmSym->. - Removed global symbol definitions from Symbols.cpp that are no longer needed. Clearing wasmSym in ctx.reset() ensures a clean slate for each link invocation, preventing symbol leakage across runs—critical when using wasm-ld/lld as a reentrant library where global state can cause subtle, hard-to-debug errors. --------- Co-authored-by: Vassil Vassilev <[email protected]> (cherry picked from commit 9cbbb74)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Towards ##134809 (comment)
This change moves WasmSym from a static global struct to an instance owned by Ctx, allowing it to be reset cleanly between linker runs. This enables safe support for multiple invocations of wasm-ld within the same process
Changes done
Converted WasmSym from a static struct to a regular struct with instance members.
Added a std::unique_ptr wasmSym field inside Ctx.
Reset wasmSym in Ctx::reset() to clear state between links.
Replaced all WasmSym:: references with ctx.wasmSym->.
Removed global symbol definitions from Symbols.cpp that are no longer needed.
Clearing wasmSym in ctx.reset() ensures a clean slate for each link invocation, preventing symbol leakage across runs—critical when using wasm-ld/lld as a reentrant library where global state can cause subtle, hard-to-debug errors.