[LLD][COFF] Clarify EC vs. native symbols in diagnostics on ARM64X #130857
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
|
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-lld-coff Author: Jacek Caban (cjacek) ChangesOn ARM64X, symbol names alone are ambiguous as they may refer to either a native or an EC symbol. Append '(EC symbol)' or '(native symbol)' in diagnostic messages to distinguish them. Full diff: https://github.com/llvm/llvm-project/pull/130857.diff 10 Files Affected:
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index bb9f1407b51f7..256bec6bb636c 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -705,9 +705,9 @@ void ObjFile::handleComdatSelection(
// seems better though.
// (This behavior matches ModuleLinker::getComdatResult().)
if (selection != leaderSelection) {
- Log(ctx) << "conflicting comdat type for " << leader << ": "
- << (int)leaderSelection << " in " << leader->getFile() << " and "
- << (int)selection << " in " << this;
+ Log(ctx) << "conflicting comdat type for " << symtab.printSymbol(leader)
+ << ": " << (int)leaderSelection << " in " << leader->getFile()
+ << " and " << (int)selection << " in " << this;
symtab.reportDuplicate(leader, this);
return;
}
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index eff840d4d56c7..16afcc64316e3 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -214,10 +214,9 @@ struct UndefinedDiag {
std::vector<File> files;
};
-static void reportUndefinedSymbol(COFFLinkerContext &ctx,
- const UndefinedDiag &undefDiag) {
+void SymbolTable::reportUndefinedSymbol(const UndefinedDiag &undefDiag) {
auto diag = errorOrWarn(ctx);
- diag << "undefined symbol: " << undefDiag.sym;
+ diag << "undefined symbol: " << printSymbol(undefDiag.sym);
const size_t maxUndefReferences = 3;
size_t numDisplayedRefs = 0, numRefs = 0;
@@ -363,12 +362,12 @@ void SymbolTable::reportProblemSymbols(
for (Symbol *b : ctx.config.gcroot) {
if (undefs.count(b))
- errorOrWarn(ctx) << "<root>: undefined symbol: " << b;
+ errorOrWarn(ctx) << "<root>: undefined symbol: " << printSymbol(b);
if (localImports)
if (Symbol *imp = localImports->lookup(b))
- Warn(ctx) << "<root>: locally defined symbol imported: " << imp
- << " (defined in " << toString(imp->getFile())
- << ") [LNK4217]";
+ Warn(ctx) << "<root>: locally defined symbol imported: "
+ << printSymbol(imp) << " (defined in "
+ << toString(imp->getFile()) << ") [LNK4217]";
}
std::vector<UndefinedDiag> undefDiags;
@@ -389,7 +388,8 @@ void SymbolTable::reportProblemSymbols(
}
if (localImports)
if (Symbol *imp = localImports->lookup(sym))
- Warn(ctx) << file << ": locally defined symbol imported: " << imp
+ Warn(ctx) << file
+ << ": locally defined symbol imported: " << printSymbol(imp)
<< " (defined in " << imp->getFile() << ") [LNK4217]";
}
};
@@ -402,7 +402,7 @@ void SymbolTable::reportProblemSymbols(
processFile(file, file->getSymbols());
for (const UndefinedDiag &undefDiag : undefDiags)
- reportUndefinedSymbol(ctx, undefDiag);
+ reportUndefinedSymbol(undefDiag);
}
void SymbolTable::reportUnresolvable() {
@@ -822,7 +822,7 @@ void SymbolTable::reportDuplicate(Symbol *existing, InputFile *newFile,
uint32_t newSectionOffset) {
COFFSyncStream diag(ctx, ctx.config.forceMultiple ? DiagLevel::Warn
: DiagLevel::Err);
- diag << "duplicate symbol: " << existing;
+ diag << "duplicate symbol: " << printSymbol(existing);
DefinedRegular *d = dyn_cast<DefinedRegular>(existing);
if (d && isa<ObjFile>(d->getFile())) {
@@ -1350,6 +1350,13 @@ Symbol *SymbolTable::addUndefined(StringRef name) {
return addUndefined(name, nullptr, false);
}
+std::string SymbolTable::printSymbol(Symbol *sym) const {
+ std::string name = maybeDemangleSymbol(ctx, sym->getName());
+ if (ctx.hybridSymtab)
+ return name + (isEC() ? " (EC symbol)" : " (native symbol)");
+ return name;
+}
+
void SymbolTable::compileBitcodeFiles() {
if (bitcodeFileInstances.empty())
return;
diff --git a/lld/COFF/SymbolTable.h b/lld/COFF/SymbolTable.h
index 22a279a97538b..da19bf2800ecf 100644
--- a/lld/COFF/SymbolTable.h
+++ b/lld/COFF/SymbolTable.h
@@ -41,6 +41,8 @@ struct WrappedSymbol {
Symbol *wrap;
};
+struct UndefinedDiag;
+
// SymbolTable is a bucket of all known symbols, including defined,
// undefined, or lazy symbols (the last one is symbols in archive
// files whose archive members are not yet loaded).
@@ -195,6 +197,8 @@ class SymbolTable {
uint32_t loadConfigSize = 0;
void initializeLoadConfig();
+ std::string printSymbol(Symbol *sym) const;
+
private:
/// Given a name without "__imp_" prefix, returns a defined symbol
/// with the "__imp_" prefix, if it exists.
@@ -216,6 +220,7 @@ class SymbolTable {
reportProblemSymbols(const llvm::SmallPtrSetImpl<Symbol *> &undefs,
const llvm::DenseMap<Symbol *, Symbol *> *localImports,
bool needBitcodeFiles);
+ void reportUndefinedSymbol(const UndefinedDiag &undefDiag);
};
std::vector<std::string> getSymbolLocations(ObjFile *file, uint32_t symIndex);
diff --git a/lld/COFF/Symbols.cpp b/lld/COFF/Symbols.cpp
index fce50d41a663b..b3133b189df39 100644
--- a/lld/COFF/Symbols.cpp
+++ b/lld/COFF/Symbols.cpp
@@ -60,9 +60,11 @@ coff::operator<<(const COFFSyncStream &s,
return s;
}
+#if 0
const COFFSyncStream &coff::operator<<(const COFFSyncStream &s, Symbol *sym) {
return s << maybeDemangleSymbol(s.ctx, sym->getName());
}
+#endif
namespace coff {
diff --git a/lld/COFF/Symbols.h b/lld/COFF/Symbols.h
index 465d4df52c630..6c562dd4ebf3f 100644
--- a/lld/COFF/Symbols.h
+++ b/lld/COFF/Symbols.h
@@ -38,7 +38,7 @@ class SymbolTable;
const COFFSyncStream &operator<<(const COFFSyncStream &,
const llvm::object::Archive::Symbol *);
-const COFFSyncStream &operator<<(const COFFSyncStream &, Symbol *);
+// const COFFSyncStream &operator<<(const COFFSyncStream &, Symbol *);
// The base class for real symbol classes.
class Symbol {
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index a8148446a2657..f488721106e15 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1282,12 +1282,13 @@ void Writer::createImportTables() {
ctx.config.dllOrder[dll] = ctx.config.dllOrder.size();
if (file->impSym && !isa<DefinedImportData>(file->impSym))
- Fatal(ctx) << file->impSym << " was replaced";
+ Fatal(ctx) << file->symtab.printSymbol(file->impSym) << " was replaced";
DefinedImportData *impSym = cast_or_null<DefinedImportData>(file->impSym);
if (ctx.config.delayLoads.count(StringRef(file->dllName).lower())) {
if (!file->thunkSym)
Fatal(ctx) << "cannot delay-load " << toString(file)
- << " due to import of data: " << impSym;
+ << " due to import of data: "
+ << file->symtab.printSymbol(impSym);
delayIdata.add(impSym);
} else {
idata.add(impSym);
@@ -1306,7 +1307,8 @@ void Writer::appendImportThunks() {
if (file->thunkSym) {
if (!isa<DefinedImportThunk>(file->thunkSym))
- Fatal(ctx) << file->thunkSym << " was replaced";
+ Fatal(ctx) << file->symtab.printSymbol(file->thunkSym)
+ << " was replaced";
auto *chunk = cast<DefinedImportThunk>(file->thunkSym)->getChunk();
if (chunk->live)
textSec->addChunk(chunk);
@@ -1314,7 +1316,8 @@ void Writer::appendImportThunks() {
if (file->auxThunkSym) {
if (!isa<DefinedImportThunk>(file->auxThunkSym))
- Fatal(ctx) << file->auxThunkSym << " was replaced";
+ Fatal(ctx) << file->symtab.printSymbol(file->auxThunkSym)
+ << " was replaced";
auto *chunk = cast<DefinedImportThunk>(file->auxThunkSym)->getChunk();
if (chunk->live)
textSec->addChunk(chunk);
diff --git a/lld/test/COFF/arm64x-altnames.s b/lld/test/COFF/arm64x-altnames.s
index c161685bdc0e8..43a3f89db9a03 100644
--- a/lld/test/COFF/arm64x-altnames.s
+++ b/lld/test/COFF/arm64x-altnames.s
@@ -12,7 +12,7 @@
// RUN: 2>&1 | FileCheck --check-prefix=ERR-NATIVE %s
// ERR-NATIVE-NOT: test-arm64ec.obj
-// ERR-NATIVE: lld-link: error: undefined symbol: sym
+// ERR-NATIVE: lld-link: error: undefined symbol: sym (native symbol)
// ERR-NATIVE-NEXT: >>> referenced by test-arm64.obj:(.test)
// ERR-NATIVE-NOT: test-arm64ec.obj
@@ -25,7 +25,7 @@
// RUN: 2>&1 | FileCheck --check-prefix=ERR-EC %s
// ERR-EC-NOT: test-arm64.obj
-// ERR-EC: lld-link: error: undefined symbol: sym
+// ERR-EC: lld-link: error: undefined symbol: sym (EC symbol)
// ERR-EC-NEXT: >>> referenced by test-arm64ec.obj:(.test)
// ERR-EC-NOT: test-arm64.obj
diff --git a/lld/test/COFF/arm64x-incl.s b/lld/test/COFF/arm64x-incl.s
index 7ddfce1ebe693..42708e6bef317 100644
--- a/lld/test/COFF/arm64x-incl.s
+++ b/lld/test/COFF/arm64x-incl.s
@@ -40,14 +40,15 @@
// Check that including a missing symbol results in an error.
// RUN: not lld-link -machine:arm64x -out:err.dll -dll -noentry loadconfig-arm64.obj loadconfig-arm64ec.obj -include:sym sym-aarch64.obj \
-// RUN: 2>&1 | FileCheck --check-prefix=ERR %s
-// ERR: lld-link: error: <root>: undefined symbol: sym
+// RUN: 2>&1 | FileCheck --check-prefix=ERR-EC %s
+// ERR-EC: lld-link: error: <root>: undefined symbol: sym (EC symbol)
// RUN: not lld-link -machine:arm64x -out:err.dll -dll -noentry loadconfig-arm64.obj loadconfig-arm64ec.obj drectve-arm64ec.obj sym-aarch64.obj \
-// RUN: 2>&1 | FileCheck --check-prefix=ERR %s
+// RUN: 2>&1 | FileCheck --check-prefix=ERR-EC %s
// RUN: not lld-link -machine:arm64x -out:err.dll -dll -noentry loadconfig-arm64.obj loadconfig-arm64ec.obj drectve-aarch64.obj sym-arm64ec.obj \
-// RUN: 2>&1 | FileCheck --check-prefix=ERR %s
+// RUN: 2>&1 | FileCheck --check-prefix=ERR-NATIVE %s
+// ERR-NATIVE: lld-link: error: <root>: undefined symbol: sym (native symbol)
#--- sym-aarch64.s
.section ".test","dr"
diff --git a/lld/test/COFF/arm64x-symtab.s b/lld/test/COFF/arm64x-symtab.s
index 16d78b2acc49f..c634f8a6ed4c5 100644
--- a/lld/test/COFF/arm64x-symtab.s
+++ b/lld/test/COFF/arm64x-symtab.s
@@ -18,19 +18,19 @@
// RUN: not lld-link -machine:arm64x -dll -noentry -out:err1.dll symref-aarch64.obj sym-arm64ec.obj \
// RUN: 2>&1 | FileCheck --check-prefix=UNDEF %s
-// UNDEF: lld-link: error: undefined symbol: sym
+// UNDEF: lld-link: error: undefined symbol: sym (native symbol)
// UNDEF-NEXT: >>> referenced by symref-aarch64.obj:(.data)
// Check that EC object files can't reference native symbols.
// RUN: not lld-link -machine:arm64x -dll -noentry -out:out.dll symref-arm64ec.obj sym-aarch64.obj \
// RUN: 2>&1 | FileCheck --check-prefix=UNDEFEC %s
-// UNDEFEC: lld-link: error: undefined symbol: sym
+// UNDEFEC: lld-link: error: undefined symbol: sym (EC symbol)
// UNDEFEC-NEXT: >>> referenced by symref-arm64ec.obj:(.data)
// RUN: not lld-link -machine:arm64x -dll -noentry -out:out.dll symref-x86_64.obj sym-aarch64.obj \
// RUN: 2>&1 | FileCheck --check-prefix=UNDEFX86 %s
-// UNDEFX86: lld-link: error: undefined symbol: sym
+// UNDEFX86: lld-link: error: undefined symbol: sym (EC symbol)
// UNDEFX86-NEXT: >>> referenced by symref-x86_64.obj:(.data)
// RUN: not lld-link -machine:arm64x -dll -noentry -out:err2.dll symref-aarch64.obj sym-x86_64.obj \
diff --git a/lld/test/COFF/locally-imported-arm64x.s b/lld/test/COFF/locally-imported-arm64x.s
index 6091af2dfa167..d89ea230f8eb5 100644
--- a/lld/test/COFF/locally-imported-arm64x.s
+++ b/lld/test/COFF/locally-imported-arm64x.s
@@ -4,8 +4,8 @@
// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows %s -o %t.arm64ec.obj
// RUN: lld-link -machine:arm64x -dll -noentry %t.arm64.obj %t.arm64ec.obj -out:%t.dll 2>&1 | FileCheck --check-prefix=WARN %s
-// WARN: lld-link: warning: {{.*}}.arm64.obj: locally defined symbol imported: func
-// WARN-NEXT: lld-link: warning: {{.*}}.arm64ec.obj: locally defined symbol imported: func
+// WARN: lld-link: warning: {{.*}}.arm64.obj: locally defined symbol imported: func (native symbol)
+// WARN-NEXT: lld-link: warning: {{.*}}.arm64ec.obj: locally defined symbol imported: func (EC symbol)
// RUN: llvm-readobj --hex-dump=.test %t.dll | FileCheck --check-prefix=TEST %s
// TEST: 0x180005000 00300000 08300000
|
|
MSVC behaves similarly, using '(Arm64 Native Symbol)' and '(EC Symbol)' suffixes. It also appends the EC suffix for |
mstorsjo
approved these changes
Mar 13, 2025
There was a problem hiding this comment.
Mostly LGTM, but there seemed to be some leftover commented out code, that probably should be properly removed (or kept?), but commented out feels wrong.
I agree with the design decision to only print this when we have two separate namespaces. You mentioned that MS link.exe prints the EC disambiguation even for plain arm64ec links, but I presume it only prints the "arm64 native symbol" when linking a hybrid image?
lld/COFF/Symbols.cpp
Outdated
| @@ -60,9 +60,11 @@ coff::operator<<(const COFFSyncStream &s, | |||
| return s; | |||
| } | |||
|
|
|||
| #if 0 | |||
There was a problem hiding this comment.
This seems like a leftover - do you want to delete it, or keep it in some other form?
There was a problem hiding this comment.
Yes, I failed to commit its removal, sorry about that.
lld/COFF/Symbols.h
Outdated
| @@ -38,7 +38,7 @@ class SymbolTable; | |||
|
|
|||
| const COFFSyncStream &operator<<(const COFFSyncStream &, | |||
| const llvm::object::Archive::Symbol *); | |||
| const COFFSyncStream &operator<<(const COFFSyncStream &, Symbol *); | |||
| // const COFFSyncStream &operator<<(const COFFSyncStream &, Symbol *); | |||
On ARM64X, symbol names alone are ambiguous as they may refer to either a native or an EC symbol. Append '(EC symbol)' or '(native symbol)' in diagnostic messages to distinguish them.
When using -machine:arm64, no suffix is printed. I rechecked MSVC seems to allow native objects and maintains a separate namespace, similar to ARM64X. I'm not sure why it behaves that way, but if needed, replicating this behavior in lld-link shouldn’t be too difficult. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/6823 Here is the relevant piece of the build log for the reference |
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.
On ARM64X, symbol names alone are ambiguous as they may refer to either a native or an EC symbol. Append '(EC symbol)' or '(native symbol)' in diagnostic messages to distinguish them.