Skip to content

Commit

Permalink
WIP: build-llvm: Apply patches to be tested
Browse files Browse the repository at this point in the history
  • Loading branch information
@mstorsjo
mstorsjo committed Jul 14, 2024
1 parent b587900 commit 1675cbb
Show file tree
Hide file tree
Showing 4 changed files with 591 additions and 0 deletions.
1 change: 1 addition & 0 deletions build-llvm.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ if [ -n "$SYNC" ] || [ -n "$CHECKOUT" ]; then
;;
esac
fi
git am -3 ../patches/llvm-project/*.patch
cd ..
fi

Expand Down
132 changes: 132 additions & 0 deletions patches/llvm-project/0001-llvm-dlltool-Remove-the-i386-underscore-prefix-from-.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
From 62bf9e01723478f11a9bded5940ab8410c5faa34 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <[email protected]>
Date: Tue, 16 Apr 2024 14:27:44 +0300
Subject: [PATCH 1/3] [llvm-dlltool] Remove the i386 underscore prefix from
COFFImportFile::ImportName. NFC.

On i386, regular C level symbols are given an underscore prefix
in the symbols on the object file level. However, the exported
names from DLLs usually don't have this leading underscore.

When specified in a def file like "symbol == dllname", the "dllname"
is the name of the exported symbol from the DLL, which will be
linked against from an object file symbol named "_symbol"
(on i386).

The mechanism where one symbol is redirected to another one in
an import library is implemented with weak aliases. In that case,
we need to have the object file symbol level name for the target
of the import, as we make one object file symbol point at another
one. Therefore, we added an underscore to the ImportName field.

(This mechanism, with weak aliases, only works as long as the
target also is exported as is, in that form - this issue is
dealt with in a later commit.)

For clarity, for potentially handling the import renaming in
other ways, store the ImportName field unprefixed, containing
the actual name to import from the DLL.

When creating the aliases, add the prefix as needed. This requires
passing an extra AddUnderscores parameter to the writeImportLibrary
function; this is a temporary measure, until alias creation is
reworked in a later commit.

This doesn't preserve the corner case of checking !isDecorated()
before adding the prefix. This corner case isn't tested by any
of our existing tests, and only would trigger for
fastcall/vectorcall/MS C++ functions - while these kinds of
renames primarily are used in mingw-w64-crt import libraries
(which primarily handle cdecl and stdcall functions).
---
llvm/include/llvm/Object/COFFImportFile.h | 9 +++++----
llvm/lib/Object/COFFImportFile.cpp | 14 ++++++++++----
llvm/lib/Object/COFFModuleDefinition.cpp | 2 --
.../lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp | 5 +++--
4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/llvm/include/llvm/Object/COFFImportFile.h b/llvm/include/llvm/Object/COFFImportFile.h
index 649fb4930934..e91d5a9b3198 100644
--- a/llvm/include/llvm/Object/COFFImportFile.h
+++ b/llvm/include/llvm/Object/COFFImportFile.h
@@ -135,10 +135,11 @@ struct COFFShortExport {
/// linking both ARM64EC and pure ARM64 objects, and the linker will pick only
/// the exports relevant to the target platform. For non-hybrid targets,
/// the NativeExports parameter should not be used.
-Error writeImportLibrary(
- StringRef ImportName, StringRef Path, ArrayRef<COFFShortExport> Exports,
- COFF::MachineTypes Machine, bool MinGW,
- ArrayRef<COFFShortExport> NativeExports = std::nullopt);
+Error writeImportLibrary(StringRef ImportName, StringRef Path,
+ ArrayRef<COFFShortExport> Exports,
+ COFF::MachineTypes Machine, bool MinGW,
+ ArrayRef<COFFShortExport> NativeExports = std::nullopt,
+ bool AddUnderscores = true);

} // namespace object
} // namespace llvm
diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp
index a9f150a965c3..03af4921ddbc 100644
--- a/llvm/lib/Object/COFFImportFile.cpp
+++ b/llvm/lib/Object/COFFImportFile.cpp
@@ -645,7 +645,8 @@ NewArchiveMember ObjectFactory::createWeakExternal(StringRef Sym,
Error writeImportLibrary(StringRef ImportName, StringRef Path,
ArrayRef<COFFShortExport> Exports,
MachineTypes Machine, bool MinGW,
- ArrayRef<COFFShortExport> NativeExports) {
+ ArrayRef<COFFShortExport> NativeExports,
+ bool AddUnderscores) {

MachineTypes NativeMachine = Machine;
if (isArm64EC(Machine)) {
@@ -691,10 +692,15 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
}

if (!E.ImportName.empty() && Name != E.ImportName) {
+ StringRef Prefix = "";
+ if (Machine == IMAGE_FILE_MACHINE_I386 && AddUnderscores)
+ Prefix = "_";
+
if (ImportType == IMPORT_CODE)
- Members.push_back(
- OF.createWeakExternal(E.ImportName, Name, false, M));
- Members.push_back(OF.createWeakExternal(E.ImportName, Name, true, M));
+ Members.push_back(OF.createWeakExternal((Prefix + E.ImportName).str(),
+ Name, false, M));
+ Members.push_back(OF.createWeakExternal((Prefix + E.ImportName).str(),
+ Name, true, M));
continue;
}

diff --git a/llvm/lib/Object/COFFModuleDefinition.cpp b/llvm/lib/Object/COFFModuleDefinition.cpp
index 0c0bef1319e4..82c18539658e 100644
--- a/llvm/lib/Object/COFFModuleDefinition.cpp
+++ b/llvm/lib/Object/COFFModuleDefinition.cpp
@@ -282,8 +282,6 @@ private:
if (Tok.K == EqualEqual) {
read();
E.ImportName = std::string(Tok.Value);
- if (AddUnderscores && !isDecorated(E.ImportName, MingwDef))
- E.ImportName = std::string("_").append(E.ImportName);
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 15e4cac08cd4..012ad246888f 100644
--- a/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
+++ b/llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
@@ -243,8 +243,9 @@ 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))
+ if (!Path.empty() &&
+ writeImportLibrary(OutputFile, Path, Exports, Machine,
+ /*MinGW=*/true, NativeExports, AddUnderscores))
return 1;
return 0;
}
--
2.25.1

150 changes: 150 additions & 0 deletions patches/llvm-project/0002-llvm-dlltool-Handle-import-renaming-using-other-name.patch
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
From 0d6df46274e47fc6dfd2085deb68352faecd0fc7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <[email protected]>
Date: Tue, 16 Apr 2024 15:22:37 +0300
Subject: [PATCH 2/3] [llvm-dlltool] Handle import renaming using other name
types, when possible

This avoids needing to use weak aliases for these cases. (Weak
aliases only work if there's another, regular import entry that
provide the desired symbol from the DLL.)
---
llvm/lib/Object/COFFImportFile.cpp | 66 +++++++++++++------
.../tools/llvm-dlltool/coff-decorated.def | 16 +++++
2 files changed, 61 insertions(+), 21 deletions(-)

diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp
index 03af4921ddbc..1ddc5d954de6 100644
--- a/llvm/lib/Object/COFFImportFile.cpp
+++ b/llvm/lib/Object/COFFImportFile.cpp
@@ -52,18 +52,12 @@ StringRef COFFImportFile::getFileFormatName() const {
}
}

-StringRef COFFImportFile::getExportName() const {
- const coff_import_header *hdr = getCOFFImportHeader();
- StringRef name = Data.getBuffer().substr(sizeof(*hdr)).split('\0').first;
-
+static StringRef applyNameType(ImportNameType Type, StringRef name) {
auto ltrim1 = [](StringRef s, StringRef chars) {
return !s.empty() && chars.contains(s[0]) ? s.substr(1) : s;
};

- switch (hdr->getNameType()) {
- case IMPORT_ORDINAL:
- name = "";
- break;
+ switch (Type) {
case IMPORT_NAME_NOPREFIX:
name = ltrim1(name, "?@_");
break;
@@ -71,6 +65,24 @@ StringRef COFFImportFile::getExportName() const {
name = ltrim1(name, "?@_");
name = name.substr(0, name.find('@'));
break;
+ default:
+ break;
+ }
+ return name;
+}
+
+StringRef COFFImportFile::getExportName() const {
+ const coff_import_header *hdr = getCOFFImportHeader();
+ StringRef name = Data.getBuffer().substr(sizeof(*hdr)).split('\0').first;
+
+ switch (hdr->getNameType()) {
+ case IMPORT_ORDINAL:
+ name = "";
+ break;
+ case IMPORT_NAME_NOPREFIX:
+ case IMPORT_NAME_UNDECORATE:
+ name = applyNameType(static_cast<ImportNameType>(hdr->getNameType()), name);
+ break;
case IMPORT_NAME_EXPORTAS: {
// Skip DLL name
name = Data.getBuffer().substr(sizeof(*hdr) + name.size() + 1);
@@ -691,19 +703,6 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
Name.swap(*ReplacedName);
}

- if (!E.ImportName.empty() && Name != E.ImportName) {
- StringRef Prefix = "";
- if (Machine == IMAGE_FILE_MACHINE_I386 && AddUnderscores)
- Prefix = "_";
-
- if (ImportType == IMPORT_CODE)
- Members.push_back(OF.createWeakExternal((Prefix + E.ImportName).str(),
- Name, false, M));
- Members.push_back(OF.createWeakExternal((Prefix + E.ImportName).str(),
- Name, true, M));
- continue;
- }
-
ImportNameType NameType;
std::string ExportName;
if (E.Noname) {
@@ -711,6 +710,31 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
} else if (!E.ExportAs.empty()) {
NameType = IMPORT_NAME_EXPORTAS;
ExportName = E.ExportAs;
+ } else if (!E.ImportName.empty()) {
+ // If we need to import from a specific ImportName, we may need to use
+ // a weak alias (which needs another import to point at). But if we can
+ // express ImportName based on the symbol name and a specific NameType,
+ // prefer that over an alias.
+ if (Machine == IMAGE_FILE_MACHINE_I386 &&
+ applyNameType(IMPORT_NAME_UNDECORATE, Name) == E.ImportName)
+ NameType = IMPORT_NAME_UNDECORATE;
+ else if (Machine == IMAGE_FILE_MACHINE_I386 &&
+ applyNameType(IMPORT_NAME_NOPREFIX, Name) == E.ImportName)
+ NameType = IMPORT_NAME_NOPREFIX;
+ else if (Name == E.ImportName)
+ NameType = IMPORT_NAME;
+ else {
+ StringRef Prefix = "";
+ if (Machine == IMAGE_FILE_MACHINE_I386 && AddUnderscores)
+ Prefix = "_";
+
+ if (ImportType == IMPORT_CODE)
+ Members.push_back(OF.createWeakExternal(
+ (Prefix + E.ImportName).str(), Name, false, M));
+ Members.push_back(OF.createWeakExternal((Prefix + E.ImportName).str(),
+ Name, true, M));
+ continue;
+ }
} else {
NameType = getNameType(SymbolName, E.Name, M, MinGW);
}
diff --git a/llvm/test/tools/llvm-dlltool/coff-decorated.def b/llvm/test/tools/llvm-dlltool/coff-decorated.def
index fc81f23d09d6..773e3762cc3d 100644
--- a/llvm/test/tools/llvm-dlltool/coff-decorated.def
+++ b/llvm/test/tools/llvm-dlltool/coff-decorated.def
@@ -13,6 +13,10 @@ StdcallExportName@4=StdcallInternalFunction@4
OtherStdcallExportName@4=CdeclInternalFunction
CdeclExportName=StdcallInternalFunction@4

+NoprefixStdcall@4 == NoprefixStdcall@4
+DecoratedStdcall@4 == _DecoratedStdcall@4
+UndecoratedStdcall@4 == UndecoratedStdcall
+
; CHECK: Name type: noprefix
; CHECK-NEXT: Export name: CdeclFunction
; CHECK-NEXT: Symbol: __imp__CdeclFunction
@@ -43,3 +47,15 @@ CdeclExportName=StdcallInternalFunction@4
; CHECK-NEXT: Export name: CdeclExportName
; CHECK-NEXT: Symbol: __imp__CdeclExportName
; CHECK-NEXT: Symbol: _CdeclExportName
+; CHECK: Name type: noprefix
+; CHECK-NEXT: Export name: NoprefixStdcall@4
+; CHECK-NEXT: Symbol: __imp__NoprefixStdcall@4
+; CHECK-NEXT: Symbol: _NoprefixStdcall@4
+; CHECK: Name type: name
+; CHECK-NEXT: Export name: _DecoratedStdcall@4
+; CHECK-NEXT: Symbol: __imp__DecoratedStdcall@4
+; CHECK-NEXT: Symbol: _DecoratedStdcall@4
+; CHECK: Name type: undecorate
+; CHECK-NEXT: Export name: UndecoratedStdcall
+; CHECK-NEXT: Symbol: __imp__UndecoratedStdcall@4
+; CHECK-NEXT: Symbol: _UndecoratedStdcall@4
--
2.25.1

Loading

0 comments on commit 1675cbb

Please sign in to comment.