[COFF] Rename the COFFShortExport::AliasTarget field. NFC. #89039
Conversation
|
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-llvm-binary-utilities Author: Martin Storsjö (mstorsjo) ChangesIt turns out that the previous name is vaguely misleading. When operating on a def file like "symbolname == dllname", that is supposed to make an import library entry, that when the symbol "symbolname" links against this, it imports the DLL symbol "dllname" from the referenced DLL. This doesn't need to involve any alias, and it doesn't need to imply that "dllname" is available on its own as a separate symbol in the import library at all. GNU dlltool implements import libraries in the form of "long import library", where each member is a regular object file with section chunks that compose the relevant .idata section pieces. There, this kind of import renaming does not involve any form of aliases, but the right .idata section just gets a different string than the symbol name. Full diff: https://github.com/llvm/llvm-project/pull/89039.diff 6 Files Affected:
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
index 917f88fc28280b..45851336721f24 100644
--- a/lld/COFF/Config.h
+++ b/lld/COFF/Config.h
@@ -55,7 +55,7 @@ struct Export {
StringRef name; // N in /export:N or /export:E=N
StringRef extName; // E in /export:E=N
StringRef exportAs; // E in /export:N,EXPORTAS,E
- StringRef aliasTarget; // GNU specific: N in "alias == N"
+ StringRef importTarget; // GNU specific: N in "othername == N"
Symbol *sym = nullptr;
uint16_t ordinal = 0;
bool noname = false;
@@ -75,7 +75,7 @@ struct Export {
bool operator==(const Export &e) const {
return (name == e.name && extName == e.extName && exportAs == e.exportAs &&
- aliasTarget == e.aliasTarget && ordinal == e.ordinal &&
+ importTarget == e.importTarget && ordinal == e.ordinal &&
noname == e.noname && data == e.data && isPrivate == e.isPrivate);
}
};
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index b0365b5b94417a..5612a30dfe74a1 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -934,7 +934,7 @@ void LinkerDriver::createImportLibrary(bool asLib) {
e2.SymbolName = std::string(e1.symbolName);
e2.ExtName = std::string(e1.extName);
e2.ExportAs = std::string(e1.exportAs);
- e2.AliasTarget = std::string(e1.aliasTarget);
+ e2.ImportTarget = std::string(e1.importTarget);
e2.Ordinal = e1.ordinal;
e2.Noname = e1.noname;
e2.Data = e1.data;
@@ -1034,7 +1034,7 @@ void LinkerDriver::parseModuleDefs(StringRef path) {
e2.extName = saver().save(e1.ExtName);
}
e2.exportAs = saver().save(e1.ExportAs);
- e2.aliasTarget = saver().save(e1.AliasTarget);
+ e2.importTarget = saver().save(e1.ImportTarget);
e2.ordinal = e1.Ordinal;
e2.noname = e1.Noname;
e2.data = e1.Data;
diff --git a/llvm/include/llvm/Object/COFFImportFile.h b/llvm/include/llvm/Object/COFFImportFile.h
index 2c06f529ecdfc5..2fdd2125ca3ea1 100644
--- a/llvm/include/llvm/Object/COFFImportFile.h
+++ b/llvm/include/llvm/Object/COFFImportFile.h
@@ -98,9 +98,11 @@ struct COFFShortExport {
/// "/export:foo=bar", this could be "_bar@8" if bar is stdcall.
std::string SymbolName;
- /// Creates a weak alias. This is the name of the weak aliasee. In a .def
+ /// Creates an import library entry that imports from a DLL export with a
+ /// different name. This is the name of the DLL export that should be
+ /// referenced when linking against this import library entry. In a .def
/// file, this is "baz" in "EXPORTS\nfoo = bar == baz".
- std::string AliasTarget;
+ std::string ImportTarget;
/// Specifies EXPORTAS name. In a .def file, this is "bar" in
/// "EXPORTS\nfoo EXPORTAS bar".
diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp
index 48c3ea0ed8f4e4..7c3874a5fec7a9 100644
--- a/llvm/lib/Object/COFFImportFile.cpp
+++ b/llvm/lib/Object/COFFImportFile.cpp
@@ -690,9 +690,9 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
Name.swap(*ReplacedName);
}
- if (!E.AliasTarget.empty() && Name != E.AliasTarget) {
- Members.push_back(OF.createWeakExternal(E.AliasTarget, Name, false, M));
- Members.push_back(OF.createWeakExternal(E.AliasTarget, Name, true, M));
+ if (!E.ImportTarget.empty() && Name != E.ImportTarget) {
+ Members.push_back(OF.createWeakExternal(E.ImportTarget, Name, false, M));
+ Members.push_back(OF.createWeakExternal(E.ImportTarget, Name, true, M));
continue;
}
diff --git a/llvm/lib/Object/COFFModuleDefinition.cpp b/llvm/lib/Object/COFFModuleDefinition.cpp
index 549348ac9226d0..09ac9631d2cfb1 100644
--- a/llvm/lib/Object/COFFModuleDefinition.cpp
+++ b/llvm/lib/Object/COFFModuleDefinition.cpp
@@ -281,9 +281,9 @@ class Parser {
}
if (Tok.K == EqualEqual) {
read();
- E.AliasTarget = std::string(Tok.Value);
- if (AddUnderscores && !isDecorated(E.AliasTarget, MingwDef))
- E.AliasTarget = std::string("_").append(E.AliasTarget);
+ E.ImportTarget = std::string(Tok.Value);
+ if (AddUnderscores && !isDecorated(E.ImportTarget, MingwDef))
+ E.ImportTarget = std::string("_").append(E.ImportTarget);
continue;
}
// EXPORTAS must be at the end of export definition
diff --git a/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp b/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
index d5fa05606d7f82..210580b4b28c88 100644
--- a/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
+++ b/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
@@ -227,7 +227,7 @@ int llvm::dlltoolDriverMain(llvm::ArrayRef<const char *> ArgsArr) {
if (Machine == IMAGE_FILE_MACHINE_I386 && Args.hasArg(OPT_k)) {
for (COFFShortExport &E : Exports) {
- if (!E.AliasTarget.empty() || (!E.Name.empty() && E.Name[0] == '?'))
+ if (!E.ImportTarget.empty() || (!E.Name.empty() && E.Name[0] == '?'))
continue;
E.SymbolName = E.Name;
// Trim off the trailing decoration. Symbols will always have a
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| /// file, this is "baz" in "EXPORTS\nfoo = bar == baz". | ||
| std::string AliasTarget; | ||
| std::string ImportTarget; |
There was a problem hiding this comment.
When trying to describe this field, the other field ExtName has a role very close to this one, but they're used in quite different contexts (the form of renaming a symbol at export, like /export:foo=bar, is only relevant while linking a DLL (and creating an import library at the same time), while this style, with renaming like foo == bar, mostly is relevant when creating an import library from a plain def file, with dlltool style tools.
I didn't want to try to unify them into one single field, as there are lots of small nuances (and big behaviour differences) between the two.
But I wanted to rename the AliasTarget field, because as long as it is named that way, it is very hard to reason about it as anything else than an alias, even though it's not necessarily an alias at all.
There was a problem hiding this comment.
Btw, if you have any better naming suggestion than ImportTarget, I’m all ears.
There was a problem hiding this comment.
Thanks, that sounds like a better alternative!
There was a problem hiding this comment.
Updated to ImportName now.
mstorsjo
force-pushed
the
importfile-alias-name
branch
from
a496284
to
f914eb5
Compare
It turns out that the previous name is vaguely misleading. When operating on a def file like "symbolname == dllname", that is supposed to make an import library entry, that when the symbol "symbolname" links against this, it imports the DLL symbol "dllname" from the referenced DLL. This doesn't need to involve any alias, and it doesn't need to imply that "dllname" is available on its own as a separate symbol in the import library at all. GNU dlltool implements import libraries in the form of "long import library", where each member is a regular object file with section chunks that compose the relevant .idata section pieces. There, this kind of import renaming does not involve any form of aliases, but the right .idata section just gets a different string than the symbol name.
mstorsjo
force-pushed
the
importfile-alias-name
branch
from
f914eb5
to
3d74b48
Compare
mstorsjo
changed the title
It turns out that the previous name is vaguely misleading.
When operating on a def file like "symbolname == dllname", that is supposed to make an import library entry, that when the symbol "symbolname" links against this, it imports the DLL symbol "dllname" from the referenced DLL. This doesn't need to involve any alias, and it doesn't need to imply that "dllname" is available on its own as a separate symbol in the import library at all.
GNU dlltool implements import libraries in the form of "long import library", where each member is a regular object file with section chunks that compose the relevant .idata section pieces. There, this kind of import renaming does not involve any form of aliases, but the right .idata section just gets a different string than the symbol name.