-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[LLD][COFF] Fix handling of invalid ARM64EC function names #116252
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
Conversation
Depends on #116250. |
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-lld-coff Author: Jacek Caban (cjacek) ChangesThis is a follow-up to #115567. Full diff: https://github.com/llvm/llvm-project/pull/116252.diff 8 Files Affected:
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index ec8ac814d31562..d6d6cc8f394f0c 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -714,9 +714,8 @@ Symbol *LinkerDriver::addUndefined(StringRef name, bool aliasEC) {
Symbol *t = ctx.symtab.addUndefined(saver().save(*mangledName));
u->setWeakAlias(t, true);
}
- } else {
- std::optional<std::string> demangledName =
- getArm64ECDemangledFunctionName(name);
+ } else if (std::optional<std::string> demangledName =
+ getArm64ECDemangledFunctionName(name)) {
Symbol *us = ctx.symtab.addUndefined(saver().save(*demangledName));
auto u = dyn_cast<Undefined>(us);
if (u && !u->weakAlias)
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index 35d54d4945f7f4..df3c5a176b52e0 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -669,8 +669,11 @@ bool checkLazyECPair(SymbolTable *symtab, StringRef name, InputFile *f) {
if (std::optional<std::string> mangledName =
getArm64ECMangledFunctionName(name))
pairName = std::move(*mangledName);
+ else if (std::optional<std::string> demangledName =
+ getArm64ECDemangledFunctionName(name))
+ pairName = std::move(*demangledName);
else
- pairName = *getArm64ECDemangledFunctionName(name);
+ return true;
Symbol *sym = symtab->find(pairName);
if (!sym)
diff --git a/lld/test/COFF/arm64ec-invalid-name.s b/lld/test/COFF/arm64ec-invalid-name.s
new file mode 100644
index 00000000000000..043d39e0c8089c
--- /dev/null
+++ b/lld/test/COFF/arm64ec-invalid-name.s
@@ -0,0 +1,15 @@
+// REQUIRES: aarch64
+
+// Verify that an error is emitted when attempting to export an invalid function name.
+// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows %s -o %t.obj
+// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows %S/Inputs/loadconfig-arm64ec.s -o %t-loadconfig.obj
+// RUN: not lld-link -machine:arm64ec -dll -noentry "-export:?func" %t-loadconfig.obj %t.obj 2>&1 | FileCheck %s
+// CHECK: error: Invalid ARM64EC function name '?func'
+
+// Verify that we can handle an invalid function name in the archive map.
+// RUN: llvm-lib -machine:arm64ec -out:%t.lib %t.obj
+// RUN: lld-link -machine:arm64ec -dll -noentry %t-loadconfig.obj %t.lib
+
+ .globl "?func"
+"?func":
+ ret
diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp
index cc0a5da7e0d16a..ff3dcf9e13ffaf 100644
--- a/llvm/lib/Object/COFFImportFile.cpp
+++ b/llvm/lib/Object/COFFImportFile.cpp
@@ -756,8 +756,15 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
}
Name = std::move(*MangledName);
} else if (!E.Noname && ExportName.empty()) {
+ std::optional<std::string> DemangledName =
+ getArm64ECDemangledFunctionName(Name);
+ if (!DemangledName)
+ return make_error<StringError>(
+ StringRef(Twine("Invalid ARM64EC function name '" + Name + "'")
+ .str()),
+ object_error::parse_failed);
NameType = IMPORT_NAME_EXPORTAS;
- ExportName = std::move(*getArm64ECDemangledFunctionName(Name));
+ ExportName = std::move(*DemangledName);
}
}
diff --git a/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp b/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
index 15e4cac08cd4ed..b3dcc0f9866842 100644
--- a/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
+++ b/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
@@ -243,8 +243,14 @@ int llvm::dlltoolDriverMain(llvm::ArrayRef<const char *> ArgsArr) {
}
std::string Path = std::string(Args.getLastArgValue(OPT_l));
- if (!Path.empty() && writeImportLibrary(OutputFile, Path, Exports, Machine,
- /*MinGW=*/true, NativeExports))
- return 1;
+ if (!Path.empty()) {
+ if (Error E = writeImportLibrary(OutputFile, Path, Exports, Machine,
+ /*MinGW=*/true, NativeExports)) {
+ handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
+ llvm::errs() << EI.message() << "\n";
+ });
+ return 1;
+ }
+ }
return 0;
}
diff --git a/llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp b/llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
index 07389a5ffb2b8a..2e0841ba02b543 100644
--- a/llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
+++ b/llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
@@ -419,10 +419,15 @@ int llvm::libDriverMain(ArrayRef<const char *> ArgsArr) {
OutputFile = std::move(NativeDef->OutputFile);
}
- return writeImportLibrary(OutputFile, OutputPath, Def->Exports, LibMachine,
- /*MinGW=*/false, NativeExports)
- ? 1
- : 0;
+ if (Error E =
+ writeImportLibrary(OutputFile, OutputPath, Def->Exports, LibMachine,
+ /*MinGW=*/false, NativeExports)) {
+ handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
+ llvm::errs() << OutputPath << ": " << EI.message() << "\n";
+ });
+ return 1;
+ }
+ return 0;
}
// If no input files and not told otherwise, silently do nothing to match
diff --git a/llvm/test/tools/llvm-dlltool/arm64ec-invalid-name.test b/llvm/test/tools/llvm-dlltool/arm64ec-invalid-name.test
new file mode 100644
index 00000000000000..9a32a7afd5e200
--- /dev/null
+++ b/llvm/test/tools/llvm-dlltool/arm64ec-invalid-name.test
@@ -0,0 +1,6 @@
+; RUN: not llvm-dlltool -m arm64ec -d %s -l %t.lib 2>&1 | FileCheck %s
+; CHECK: Invalid ARM64EC function name '?func'
+
+LIBRARY test.dll
+EXPORTS
+ ?func
diff --git a/llvm/test/tools/llvm-lib/arm64ec-invalid-name.test b/llvm/test/tools/llvm-lib/arm64ec-invalid-name.test
new file mode 100644
index 00000000000000..696b4c96eed22e
--- /dev/null
+++ b/llvm/test/tools/llvm-lib/arm64ec-invalid-name.test
@@ -0,0 +1,6 @@
+; RUN: not llvm-lib -machine:arm64ec -def:%s -out:%t.lib 2>&1 | FileCheck %s
+; CHECK: Invalid ARM64EC function name '?func'
+
+LIBRARY test.dll
+EXPORTS
+ ?func
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Jacek Caban (cjacek) ChangesThis is a follow-up to #115567. Full diff: https://github.com/llvm/llvm-project/pull/116252.diff 8 Files Affected:
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index ec8ac814d31562..d6d6cc8f394f0c 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -714,9 +714,8 @@ Symbol *LinkerDriver::addUndefined(StringRef name, bool aliasEC) {
Symbol *t = ctx.symtab.addUndefined(saver().save(*mangledName));
u->setWeakAlias(t, true);
}
- } else {
- std::optional<std::string> demangledName =
- getArm64ECDemangledFunctionName(name);
+ } else if (std::optional<std::string> demangledName =
+ getArm64ECDemangledFunctionName(name)) {
Symbol *us = ctx.symtab.addUndefined(saver().save(*demangledName));
auto u = dyn_cast<Undefined>(us);
if (u && !u->weakAlias)
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index 35d54d4945f7f4..df3c5a176b52e0 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -669,8 +669,11 @@ bool checkLazyECPair(SymbolTable *symtab, StringRef name, InputFile *f) {
if (std::optional<std::string> mangledName =
getArm64ECMangledFunctionName(name))
pairName = std::move(*mangledName);
+ else if (std::optional<std::string> demangledName =
+ getArm64ECDemangledFunctionName(name))
+ pairName = std::move(*demangledName);
else
- pairName = *getArm64ECDemangledFunctionName(name);
+ return true;
Symbol *sym = symtab->find(pairName);
if (!sym)
diff --git a/lld/test/COFF/arm64ec-invalid-name.s b/lld/test/COFF/arm64ec-invalid-name.s
new file mode 100644
index 00000000000000..043d39e0c8089c
--- /dev/null
+++ b/lld/test/COFF/arm64ec-invalid-name.s
@@ -0,0 +1,15 @@
+// REQUIRES: aarch64
+
+// Verify that an error is emitted when attempting to export an invalid function name.
+// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows %s -o %t.obj
+// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows %S/Inputs/loadconfig-arm64ec.s -o %t-loadconfig.obj
+// RUN: not lld-link -machine:arm64ec -dll -noentry "-export:?func" %t-loadconfig.obj %t.obj 2>&1 | FileCheck %s
+// CHECK: error: Invalid ARM64EC function name '?func'
+
+// Verify that we can handle an invalid function name in the archive map.
+// RUN: llvm-lib -machine:arm64ec -out:%t.lib %t.obj
+// RUN: lld-link -machine:arm64ec -dll -noentry %t-loadconfig.obj %t.lib
+
+ .globl "?func"
+"?func":
+ ret
diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp
index cc0a5da7e0d16a..ff3dcf9e13ffaf 100644
--- a/llvm/lib/Object/COFFImportFile.cpp
+++ b/llvm/lib/Object/COFFImportFile.cpp
@@ -756,8 +756,15 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
}
Name = std::move(*MangledName);
} else if (!E.Noname && ExportName.empty()) {
+ std::optional<std::string> DemangledName =
+ getArm64ECDemangledFunctionName(Name);
+ if (!DemangledName)
+ return make_error<StringError>(
+ StringRef(Twine("Invalid ARM64EC function name '" + Name + "'")
+ .str()),
+ object_error::parse_failed);
NameType = IMPORT_NAME_EXPORTAS;
- ExportName = std::move(*getArm64ECDemangledFunctionName(Name));
+ ExportName = std::move(*DemangledName);
}
}
diff --git a/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp b/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
index 15e4cac08cd4ed..b3dcc0f9866842 100644
--- a/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
+++ b/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
@@ -243,8 +243,14 @@ int llvm::dlltoolDriverMain(llvm::ArrayRef<const char *> ArgsArr) {
}
std::string Path = std::string(Args.getLastArgValue(OPT_l));
- if (!Path.empty() && writeImportLibrary(OutputFile, Path, Exports, Machine,
- /*MinGW=*/true, NativeExports))
- return 1;
+ if (!Path.empty()) {
+ if (Error E = writeImportLibrary(OutputFile, Path, Exports, Machine,
+ /*MinGW=*/true, NativeExports)) {
+ handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
+ llvm::errs() << EI.message() << "\n";
+ });
+ return 1;
+ }
+ }
return 0;
}
diff --git a/llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp b/llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
index 07389a5ffb2b8a..2e0841ba02b543 100644
--- a/llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
+++ b/llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
@@ -419,10 +419,15 @@ int llvm::libDriverMain(ArrayRef<const char *> ArgsArr) {
OutputFile = std::move(NativeDef->OutputFile);
}
- return writeImportLibrary(OutputFile, OutputPath, Def->Exports, LibMachine,
- /*MinGW=*/false, NativeExports)
- ? 1
- : 0;
+ if (Error E =
+ writeImportLibrary(OutputFile, OutputPath, Def->Exports, LibMachine,
+ /*MinGW=*/false, NativeExports)) {
+ handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
+ llvm::errs() << OutputPath << ": " << EI.message() << "\n";
+ });
+ return 1;
+ }
+ return 0;
}
// If no input files and not told otherwise, silently do nothing to match
diff --git a/llvm/test/tools/llvm-dlltool/arm64ec-invalid-name.test b/llvm/test/tools/llvm-dlltool/arm64ec-invalid-name.test
new file mode 100644
index 00000000000000..9a32a7afd5e200
--- /dev/null
+++ b/llvm/test/tools/llvm-dlltool/arm64ec-invalid-name.test
@@ -0,0 +1,6 @@
+; RUN: not llvm-dlltool -m arm64ec -d %s -l %t.lib 2>&1 | FileCheck %s
+; CHECK: Invalid ARM64EC function name '?func'
+
+LIBRARY test.dll
+EXPORTS
+ ?func
diff --git a/llvm/test/tools/llvm-lib/arm64ec-invalid-name.test b/llvm/test/tools/llvm-lib/arm64ec-invalid-name.test
new file mode 100644
index 00000000000000..696b4c96eed22e
--- /dev/null
+++ b/llvm/test/tools/llvm-lib/arm64ec-invalid-name.test
@@ -0,0 +1,6 @@
+; RUN: not llvm-lib -machine:arm64ec -def:%s -out:%t.lib 2>&1 | FileCheck %s
+; CHECK: Invalid ARM64EC function name '?func'
+
+LIBRARY test.dll
+EXPORTS
+ ?func
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM: one minor question, and can you please expand the description to explain what this change is for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Since these symbols cannot be mangled or demangled, there is no symbol to check for conflicts in checkLazyECPair, nor is there an alias to create in addUndefined. Attempting to create an import library with such symbols results in an error; the patch includes a test to ensure the error is handled correctly. This is a follow-up to llvm#115567.
da91c66
to
e086990
Compare
Since these symbols cannot be mangled or demangled, there is no symbol to check for conflicts in
checkLazyECPair
, nor is there an alias to create inaddUndefined
. Attempting to create an import library with such symbols results in an error; the patch includes a test to ensure the error is handled correctly.This is a follow-up to #115567.