[llvm] [llvm-dlltool] Remove the i386 underscore prefix from COFFImportFile::ImportName. NFC. (PR #98226)

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 9 14:31:45 PDT 2024


https://github.com/mstorsjo created https://github.com/llvm/llvm-project/pull/98226

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.


>From 40a59e2992d090b8b92b239ac6d102ead9cb3f80 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Tue, 9 Jul 2024 15:13:03 +0300
Subject: [PATCH 1/2] [llvm-dlltool] Respect the DATA flag when creating
 aliases

---
 llvm/lib/Object/COFFImportFile.cpp                 | 4 +++-
 llvm/test/tools/llvm-dlltool/coff-weak-exports.def | 4 ++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp
index fd8aca393e90b..a9f150a965c35 100644
--- a/llvm/lib/Object/COFFImportFile.cpp
+++ b/llvm/lib/Object/COFFImportFile.cpp
@@ -691,7 +691,9 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
       }
 
       if (!E.ImportName.empty() && Name != E.ImportName) {
-        Members.push_back(OF.createWeakExternal(E.ImportName, Name, false, M));
+        if (ImportType == IMPORT_CODE)
+          Members.push_back(
+              OF.createWeakExternal(E.ImportName, Name, false, M));
         Members.push_back(OF.createWeakExternal(E.ImportName, Name, true, M));
         continue;
       }
diff --git a/llvm/test/tools/llvm-dlltool/coff-weak-exports.def b/llvm/test/tools/llvm-dlltool/coff-weak-exports.def
index dacc5f73530fd..67f0013bf170f 100644
--- a/llvm/test/tools/llvm-dlltool/coff-weak-exports.def
+++ b/llvm/test/tools/llvm-dlltool/coff-weak-exports.def
@@ -6,6 +6,7 @@
 LIBRARY test.dll
 EXPORTS
 TestFunction==AltTestFunction
+TestData DATA == AltTestData
 ; When creating an import library, the DLL internal function name of
 ; the implementation of a function isn't visible at all.
 ImpLibName = Implementation
@@ -20,6 +21,9 @@ ImpLibName3 = kernel32.Sleep
 ; CHECK-NEXT: W TestFunction
 ; CHECK:      U __imp_AltTestFunction
 ; CHECK-NEXT: W __imp_TestFunction
+; CHECK-NOT:  W TestData
+; CHECK:      U __imp_AltTestData
+; CHECK-NEXT: W __imp_TestData
 ; CHECK:      T ImpLibName
 ; CHECK-NEXT: T __imp_ImpLibName
 ; CHECK:      U AltTestFunction2

>From 07437ecea9150075e9bf8660acbc40a4c30c17fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Tue, 16 Apr 2024 14:27:44 +0300
Subject: [PATCH 2/2] [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.
---
 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 649fb4930934d..e91d5a9b3198a 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 a9f150a965c35..03af4921ddbca 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 0c0bef1319e44..82c18539658e8 100644
--- a/llvm/lib/Object/COFFModuleDefinition.cpp
+++ b/llvm/lib/Object/COFFModuleDefinition.cpp
@@ -282,8 +282,6 @@ class Parser {
       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 15e4cac08cd4e..012ad246888f9 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;
 }



More information about the llvm-commits mailing list