[lld] wasm-ld: Add allow-multiple-definition flag (PR #97699)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Jul 7 03:54:27 PDT 2024
Martin =?utf-8?q?Žukovec?= <martin.zukovec at xlab.si>,
Martin =?utf-8?q?Žukovec?= <martin.zukovec at xlab.si>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/97699 at github.com>
https://github.com/mzukovec updated https://github.com/llvm/llvm-project/pull/97699
>From 7e67621bbe9e126762233838fcf4e6b218ae5a50 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20=C5=BDukovec?= <martin.zukovec at xlab.si>
Date: Thu, 4 Jul 2024 11:15:11 +0200
Subject: [PATCH 1/3] Add allow-multiple-definition flag for wasm-ld
---
.../wasm/Inputs/allow-multiple-definition.s | 6 +++
lld/test/wasm/allow-multiple-definition.s | 39 +++++++++++++++++++
lld/wasm/Config.h | 1 +
lld/wasm/Driver.cpp | 3 ++
lld/wasm/Options.td | 4 ++
lld/wasm/SymbolTable.cpp | 11 ++++--
6 files changed, 61 insertions(+), 3 deletions(-)
create mode 100644 lld/test/wasm/Inputs/allow-multiple-definition.s
create mode 100644 lld/test/wasm/allow-multiple-definition.s
diff --git a/lld/test/wasm/Inputs/allow-multiple-definition.s b/lld/test/wasm/Inputs/allow-multiple-definition.s
new file mode 100644
index 0000000000000..7a5577cb12791
--- /dev/null
+++ b/lld/test/wasm/Inputs/allow-multiple-definition.s
@@ -0,0 +1,6 @@
+ .hidden foo
+ .globl foo
+foo:
+ .functype foo () -> (i32)
+ i32.const 1
+ end_function
diff --git a/lld/test/wasm/allow-multiple-definition.s b/lld/test/wasm/allow-multiple-definition.s
new file mode 100644
index 0000000000000..605ed4965b15b
--- /dev/null
+++ b/lld/test/wasm/allow-multiple-definition.s
@@ -0,0 +1,39 @@
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %s -o %t1
+# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/allow-multiple-definition.s -o %t2
+# RUN: llvm-objdump --no-print-imm-hex -d %t1 | FileCheck --check-prefix=DEF1 %s
+# RUN: llvm-objdump --no-print-imm-hex -d %t2 | FileCheck --check-prefix=DEF2 %s
+# RUN: not wasm-ld -o %t.wasm %t1 %t2 2>&1 | FileCheck --check-prefix=DUP1 %s
+# RUN: not wasm-ld -o %t.wasm %t2 %t1 2>&1 | FileCheck --check-prefix=DUP2 %s
+# RUN: wasm-ld --allow-multiple-definition %t1 %t2 -o %t3
+# RUN: llvm-objdump --no-print-imm-hex -d %t3 | FileCheck --check-prefix=RES12 %s
+# RUN: wasm-ld --allow-multiple-definition %t2 %t1 -o %t4
+# RUN: llvm-objdump --no-print-imm-hex -d %t4 | FileCheck --check-prefix=RES21 %s
+
+
+# DUP1: duplicate symbol: foo
+# DUP2: duplicate symbol: foo
+
+# DEF1: i32.const 0
+# DEF2: i32.const 1
+
+# RES12: i32.const 0
+# RES21: i32.const 1
+
+# inputs contain different constants for function foo return.
+# Tests below checks that order of files in command line
+# affects on what symbol will be used.
+# If flag allow-multiple-definition is enabled the first
+# meet symbol should be used.
+
+ .hidden foo
+ .globl foo
+foo:
+ .functype foo () -> (i32)
+ i32.const 0
+ end_function
+
+ .globl _start
+_start:
+ .functype _start () -> (i32)
+ call foo
+ end_function
\ No newline at end of file
diff --git a/lld/wasm/Config.h b/lld/wasm/Config.h
index d0ffa83d111e0..f36300cf21e32 100644
--- a/lld/wasm/Config.h
+++ b/lld/wasm/Config.h
@@ -43,6 +43,7 @@ enum class BuildIdKind { None, Fast, Sha1, Hexstring, Uuid };
// and such fields have the same name as the corresponding options.
// Most fields are initialized by the driver.
struct Configuration {
+ bool allowMultipleDefinition;
bool bsymbolic;
bool checkFeatures;
bool compressRelocations;
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index d099689911fc6..3bc64b1ae387d 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -467,6 +467,9 @@ getBuildId(opt::InputArgList &args) {
// Initializes Config members by the command line options.
static void readConfigs(opt::InputArgList &args) {
+ config->allowMultipleDefinition =
+ args.hasFlag(OPT_allow_multiple_definition,
+ OPT_no_allow_multiple_definition, false);
config->bsymbolic = args.hasArg(OPT_Bsymbolic);
config->checkFeatures =
args.hasFlag(OPT_check_features, OPT_no_check_features, true);
diff --git a/lld/wasm/Options.td b/lld/wasm/Options.td
index 7e954822ef642..dc08ae6f81310 100644
--- a/lld/wasm/Options.td
+++ b/lld/wasm/Options.td
@@ -42,6 +42,10 @@ def Bdynamic: F<"Bdynamic">, HelpText<"Link against shared libraries">;
def Bstatic: F<"Bstatic">, HelpText<"Do not link against shared libraries (default)">;
+defm allow_multiple_definition: B<"allow-multiple-definition",
+ "Allow multiple definitions",
+ "Do not allow multiple definitions (default)">;
+
def build_id: F<"build-id">, HelpText<"Alias for --build-id=fast">;
def build_id_eq: J<"build-id=">, HelpText<"Generate build ID note">,
diff --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp
index 081f811cd139d..e50706d7c8a19 100644
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -310,9 +310,14 @@ static bool shouldReplace(const Symbol *existing, InputFile *newFile,
}
// Neither symbol is week. They conflict.
- error("duplicate symbol: " + toString(*existing) + "\n>>> defined in " +
- toString(existing->getFile()) + "\n>>> defined in " +
- toString(newFile));
+ std::string msg = "duplicate symbol: " + toString(*existing) +
+ "\n>>> defined in " + toString(existing->getFile()) +
+ "\n>>> defined in " + toString(newFile);
+ if (config->allowMultipleDefinition) {
+ warn(msg);
+ return false;
+ }
+ error(msg);
return true;
}
>From 4322301f712e38f606d2a694caca03e0e0f42e0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20=C5=BDukovec?= <martin.zukovec at xlab.si>
Date: Sun, 7 Jul 2024 12:30:56 +0200
Subject: [PATCH 2/3] Follow ELF test for multiple definitions. Add "-z
muldefs" option parsing.
---
lld/test/wasm/allow-multiple-definition.s | 28 +++++++--------
lld/wasm/Driver.cpp | 42 +++++++++++++++--------
2 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/lld/test/wasm/allow-multiple-definition.s b/lld/test/wasm/allow-multiple-definition.s
index 605ed4965b15b..3baa912bbfa55 100644
--- a/lld/test/wasm/allow-multiple-definition.s
+++ b/lld/test/wasm/allow-multiple-definition.s
@@ -1,23 +1,19 @@
# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %s -o %t1
# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/allow-multiple-definition.s -o %t2
-# RUN: llvm-objdump --no-print-imm-hex -d %t1 | FileCheck --check-prefix=DEF1 %s
-# RUN: llvm-objdump --no-print-imm-hex -d %t2 | FileCheck --check-prefix=DEF2 %s
-# RUN: not wasm-ld -o %t.wasm %t1 %t2 2>&1 | FileCheck --check-prefix=DUP1 %s
-# RUN: not wasm-ld -o %t.wasm %t2 %t1 2>&1 | FileCheck --check-prefix=DUP2 %s
-# RUN: wasm-ld --allow-multiple-definition %t1 %t2 -o %t3
-# RUN: llvm-objdump --no-print-imm-hex -d %t3 | FileCheck --check-prefix=RES12 %s
-# RUN: wasm-ld --allow-multiple-definition %t2 %t1 -o %t4
-# RUN: llvm-objdump --no-print-imm-hex -d %t4 | FileCheck --check-prefix=RES21 %s
+# RUN: not wasm-ld %t1 %t2 -o /dev/null
+# RUN: not wasm-ld --allow-multiple-definition --no-allow-multiple-definition %t1 %t2 -o /dev/null
+# RUN: wasm-ld --allow-multiple-definition --fatal-warnings %t1 %t2 -o %t3
+# RUN: wasm-ld --allow-multiple-definition --fatal-warnings %t2 %t1 -o %t4
+# RUN: llvm-objdump --no-print-imm-hex -d %t3 | FileCheck %s
+# RUN: llvm-objdump --no-print-imm-hex -d %t4 | FileCheck --check-prefix=REVERT %s
+# RUN: wasm-ld -z muldefs --fatal-warnings %t1 %t2 -o %t3
+# RUN: wasm-ld -z muldefs --fatal-warnings %t2 %t1 -o %t4
+# RUN: llvm-objdump --no-print-imm-hex -d %t3 | FileCheck %s
+# RUN: llvm-objdump --no-print-imm-hex -d %t4 | FileCheck --check-prefix=REVERT %s
-# DUP1: duplicate symbol: foo
-# DUP2: duplicate symbol: foo
-
-# DEF1: i32.const 0
-# DEF2: i32.const 1
-
-# RES12: i32.const 0
-# RES21: i32.const 1
+# CHECK: i32.const 0
+# REVERT: i32.const 1
# inputs contain different constants for function foo return.
# Tests below checks that order of files in command line
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 3bc64b1ae387d..81113f441c1b6 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -68,6 +68,7 @@ enum {
OPT_INVALID = 0,
#define OPTION(...) LLVM_MAKE_OPT_ID(__VA_ARGS__),
#include "Options.inc"
+
#undef OPTION
};
@@ -99,6 +100,17 @@ class LinkerDriver {
std::vector<InputFile *> files;
};
+
+static bool hasZOption(opt::InputArgList &args, StringRef key) {
+ bool ret = false;
+ for (const auto *arg : args.filtered(OPT_z))
+ if (key == arg->getValue()) {
+ ret = true;
+ arg->claim();
+ }
+ return ret;
+}
+
} // anonymous namespace
bool link(ArrayRef<const char *> args, llvm::raw_ostream &stdoutOS,
@@ -149,6 +161,7 @@ static constexpr opt::OptTable::Info optInfo[] = {
ALIASARGS, \
VALUES},
#include "Options.inc"
+
#undef OPTION
};
@@ -468,6 +481,7 @@ getBuildId(opt::InputArgList &args) {
// Initializes Config members by the command line options.
static void readConfigs(opt::InputArgList &args) {
config->allowMultipleDefinition =
+ hasZOption(args, "muldefs") ||
args.hasFlag(OPT_allow_multiple_definition,
OPT_no_allow_multiple_definition, false);
config->bsymbolic = args.hasArg(OPT_Bsymbolic);
@@ -495,8 +509,7 @@ static void readConfigs(opt::InputArgList &args) {
}
if (args.hasArg(OPT_export_memory_with_name)) {
- config->memoryExport =
- args.getLastArgValue(OPT_export_memory_with_name);
+ config->memoryExport = args.getLastArgValue(OPT_export_memory_with_name);
} else if (args.hasArg(OPT_export_memory)) {
config->memoryExport = memoryName;
} else {
@@ -660,8 +673,8 @@ static void setConfigs() {
error("--export-memory is incompatible with --shared");
}
if (!config->memoryImport.has_value()) {
- config->memoryImport =
- std::pair<llvm::StringRef, llvm::StringRef>(defaultModule, memoryName);
+ config->memoryImport = std::pair<llvm::StringRef, llvm::StringRef>(
+ defaultModule, memoryName);
}
}
@@ -800,7 +813,7 @@ static void writeWhyExtract() {
// Equivalent of demote demoteSharedAndLazySymbols() in the ELF linker
static void demoteLazySymbols() {
for (Symbol *sym : symtab->symbols()) {
- if (auto* s = dyn_cast<LazySymbol>(sym)) {
+ if (auto *s = dyn_cast<LazySymbol>(sym)) {
if (s->signature) {
LLVM_DEBUG(llvm::dbgs()
<< "demoting lazy func: " << s->getName() << "\n");
@@ -887,9 +900,8 @@ static void createSyntheticSymbols() {
WasmSym::tlsAlign = createGlobalVariable("__tls_align", false);
WasmSym::initTLS = symtab->addSyntheticFunction(
"__wasm_init_tls", WASM_SYMBOL_VISIBILITY_HIDDEN,
- make<SyntheticFunction>(
- is64 ? i64ArgSignature : i32ArgSignature,
- "__wasm_init_tls"));
+ make<SyntheticFunction>(is64 ? i64ArgSignature : i32ArgSignature,
+ "__wasm_init_tls"));
}
if (ctx.isPic ||
@@ -941,15 +953,15 @@ static void processStubLibrariesPreLTO() {
for (auto &stub_file : ctx.stubFiles) {
LLVM_DEBUG(llvm::dbgs()
<< "processing stub file: " << stub_file->getName() << "\n");
- for (auto [name, deps]: stub_file->symbolDependencies) {
- auto* sym = symtab->find(name);
+ for (auto [name, deps] : stub_file->symbolDependencies) {
+ auto *sym = symtab->find(name);
// If the symbol is not present at all (yet), or if it is present but
// undefined, then mark the dependent symbols as used by a regular
// object so they will be preserved and exported by the LTO process.
if (!sym || sym->isUndefined()) {
for (const auto dep : deps) {
- auto* needed = symtab->find(dep);
- if (needed ) {
+ auto *needed = symtab->find(dep);
+ if (needed) {
needed->isUsedInRegularObj = true;
}
}
@@ -1010,8 +1022,8 @@ static void processStubLibraries() {
// First look for any imported symbols that directly match
// the names of the stub imports
- for (auto [name, deps]: stub_file->symbolDependencies) {
- auto* sym = symtab->find(name);
+ for (auto [name, deps] : stub_file->symbolDependencies) {
+ auto *sym = symtab->find(name);
if (sym && sym->isUndefined()) {
depsAdded |= addStubSymbolDeps(stub_file, sym, deps);
} else {
@@ -1164,7 +1176,7 @@ static void splitSections() {
static bool isKnownZFlag(StringRef s) {
// For now, we only support a very limited set of -z flags
- return s.starts_with("stack-size=");
+ return s.starts_with("stack-size=") || s.starts_with("muldefs");
}
// Report a warning for an unknown -z option.
>From a0992b0db49f2402bfa7f28b18f477f4df6f38fb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20=C5=BDukovec?= <martin.zukovec at xlab.si>
Date: Sun, 7 Jul 2024 12:54:11 +0200
Subject: [PATCH 3/3] Add --noinhibit-exec to wasm-ld to follow ELF logic.
---
lld/test/wasm/allow-multiple-definition.s | 3 +++
lld/wasm/Config.h | 4 ++++
lld/wasm/Driver.cpp | 8 ++++++++
lld/wasm/Options.td | 3 +++
lld/wasm/SymbolTable.cpp | 8 +++-----
5 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/lld/test/wasm/allow-multiple-definition.s b/lld/test/wasm/allow-multiple-definition.s
index 3baa912bbfa55..497de0c3e81e7 100644
--- a/lld/test/wasm/allow-multiple-definition.s
+++ b/lld/test/wasm/allow-multiple-definition.s
@@ -7,6 +7,9 @@
# RUN: llvm-objdump --no-print-imm-hex -d %t3 | FileCheck %s
# RUN: llvm-objdump --no-print-imm-hex -d %t4 | FileCheck --check-prefix=REVERT %s
+# RUN: wasm-ld --noinhibit-exec %t2 %t1 -o /dev/null 2>&1 | FileCheck %s --check-prefix=WARN
+# WARN: warning: duplicate symbol: foo
+
# RUN: wasm-ld -z muldefs --fatal-warnings %t1 %t2 -o %t3
# RUN: wasm-ld -z muldefs --fatal-warnings %t2 %t1 -o %t4
# RUN: llvm-objdump --no-print-imm-hex -d %t3 | FileCheck %s
diff --git a/lld/wasm/Config.h b/lld/wasm/Config.h
index f36300cf21e32..98fd33373779a 100644
--- a/lld/wasm/Config.h
+++ b/lld/wasm/Config.h
@@ -12,6 +12,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/Twine.h"
#include "llvm/BinaryFormat/Wasm.h"
#include "llvm/Support/CachePruning.h"
#include <optional>
@@ -65,6 +66,7 @@ struct Configuration {
bool importUndefined;
std::optional<bool> is64;
bool mergeDataSegments;
+ bool noinhibitExec;
bool pie;
bool printGcSections;
bool relocatable;
@@ -148,6 +150,8 @@ struct Ctx {
extern Ctx ctx;
+void errorOrWarn(const llvm::Twine &msg);
+
} // namespace lld::wasm
#endif
diff --git a/lld/wasm/Driver.cpp b/lld/wasm/Driver.cpp
index 81113f441c1b6..30bed2f9a10f2 100644
--- a/lld/wasm/Driver.cpp
+++ b/lld/wasm/Driver.cpp
@@ -47,6 +47,13 @@ namespace lld::wasm {
Configuration *config;
Ctx ctx;
+void errorOrWarn(const llvm::Twine &msg) {
+ if (config->noinhibitExec)
+ warn(msg);
+ else
+ error(msg);
+}
+
void Ctx::reset() {
objectFiles.clear();
stubFiles.clear();
@@ -496,6 +503,7 @@ static void readConfigs(opt::InputArgList &args) {
config->exportAll = args.hasArg(OPT_export_all);
config->exportTable = args.hasArg(OPT_export_table);
config->growableTable = args.hasArg(OPT_growable_table);
+ config->noinhibitExec = args.hasArg(OPT_noinhibit_exec);
if (args.hasArg(OPT_import_memory_with_name)) {
config->memoryImport =
diff --git a/lld/wasm/Options.td b/lld/wasm/Options.td
index dc08ae6f81310..ed9690195613c 100644
--- a/lld/wasm/Options.td
+++ b/lld/wasm/Options.td
@@ -110,6 +110,9 @@ defm Map: Eq<"Map", "Print a link map to the specified file">;
def no_fatal_warnings: F<"no-fatal-warnings">;
+def noinhibit_exec: F<"noinhibit-exec">,
+ HelpText<"Retain the executable output file whenever it is still usable">;
+
def o: JoinedOrSeparate<["-"], "o">, MetaVarName<"<path>">,
HelpText<"Path to file to write output">;
diff --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp
index e50706d7c8a19..2d52f100e68e8 100644
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -310,14 +310,12 @@ static bool shouldReplace(const Symbol *existing, InputFile *newFile,
}
// Neither symbol is week. They conflict.
- std::string msg = "duplicate symbol: " + toString(*existing) +
- "\n>>> defined in " + toString(existing->getFile()) +
- "\n>>> defined in " + toString(newFile);
if (config->allowMultipleDefinition) {
- warn(msg);
return false;
}
- error(msg);
+ errorOrWarn("duplicate symbol: " + toString(*existing) + "\n>>> defined in " +
+ toString(existing->getFile()) + "\n>>> defined in " +
+ toString(newFile));
return true;
}
More information about the llvm-commits
mailing list