[lld] [llvm] [llvm-dlltool] Fix renamed imports without a separate regular import entry (PR #98229)

via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 9 14:37:26 PDT 2024


Martin =?utf-8?q?Storsjö?= <martin at martin.st>,
Martin =?utf-8?q?Storsjö?= <martin at martin.st>,
Martin =?utf-8?q?Storsjö?= <martin at martin.st>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/98229 at github.com>


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lld

Author: Martin Storsjö (mstorsjo)

<details>
<summary>Changes</summary>

Normally, when doing renamed imports, we do this by providing a
weak alias, towards another regular import, for the symbol we
want to actually import. In a def file, this looks like this:

    regularfunc
    renamedfunc == regularfunc

However, if we want to link against a function in a DLL, where we
(intentionally) don't provide a regular import for that symbol
with the name in its DLL, doing the renamed import with a weak
alias doesn't work, as there's no symbol that the weak alias can
point towards.

We can't make up such an import either, as we may intentionally
not want to provide a regular import for that name.

This situation can either be resolved by using the "long" import
library format (as e.g. produced by GNU dlltool), or by using the
new short import library name type "export as".

This patch implements it by using the "export as" name type.

When producing a renamed import, defer emitting it until all regular
imports have been produced. If the renamed import refers to a
symbol that does exist as a regular import entry, produce a
weak alias, just as before. (This implementation also avoids needing
to know whether the symbol that the alias points towards actually
is prefixed or not, too.)

If the renamed import points at a symbol that isn't otherwise
available (or is available as a renamed symbol itself), generate
an "export as" import entry.

This name type is new, and is normally used in ARM64EC import
libraries, but can also be used for other architectures.

---
Full diff: https://github.com/llvm/llvm-project/pull/98229.diff


6 Files Affected:

- (modified) lld/test/COFF/lib.test (+8-8) 
- (modified) llvm/lib/Object/COFFImportFile.cpp (+62-14) 
- (modified) llvm/lib/Object/COFFModuleDefinition.cpp (-2) 
- (modified) llvm/test/tools/llvm-dlltool/coff-decorated.def (+17-1) 
- (modified) llvm/test/tools/llvm-dlltool/coff-weak-exports.def (+15-4) 
- (added) llvm/test/tools/llvm-dlltool/renaming.def (+39) 


``````````diff
diff --git a/lld/test/COFF/lib.test b/lld/test/COFF/lib.test
index 7525ef4226cda..82abca6ec9307 100644
--- a/lld/test/COFF/lib.test
+++ b/lld/test/COFF/lib.test
@@ -1,6 +1,14 @@
 # RUN: lld-link /machine:x64 /def:%S/Inputs/library.def /out:%t.lib
 # RUN: llvm-nm %t.lib | FileCheck %s
 
+CHECK: 00000000 R __imp_constant
+CHECK: 00000000 R constant
+
+CHECK: 00000000 D __imp_data
+
+CHECK: 00000000 T __imp_function
+CHECK: 00000000 T function
+
 CHECK: 00000000 a @comp.id
 CHECK: 00000000 a @feat.00
 CHECK: 00000000 W alias
@@ -11,11 +19,3 @@ CHECK: 00000000 a @feat.00
 CHECK: 00000000 W __imp_alias
 CHECK:          U __imp_function
 
-CHECK: 00000000 R __imp_constant
-CHECK: 00000000 R constant
-
-CHECK: 00000000 D __imp_data
-
-CHECK: 00000000 T __imp_function
-CHECK: 00000000 T function
-
diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp
index fd8aca393e90b..6922ac6994469 100644
--- a/llvm/lib/Object/COFFImportFile.cpp
+++ b/llvm/lib/Object/COFFImportFile.cpp
@@ -12,6 +12,8 @@
 
 #include "llvm/Object/COFFImportFile.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ArchiveWriter.h"
@@ -52,18 +54,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 +67,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);
@@ -667,6 +681,13 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
 
   auto addExports = [&](ArrayRef<COFFShortExport> Exp,
                         MachineTypes M) -> Error {
+    StringMap<std::string> RegularImports;
+    struct Deferred {
+      std::string Name;
+      ImportType ImportType;
+      const COFFShortExport *Export;
+    };
+    SmallVector<Deferred, 0> Renames;
     for (const COFFShortExport &E : Exp) {
       if (E.Private)
         continue;
@@ -690,12 +711,6 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
         Name.swap(*ReplacedName);
       }
 
-      if (!E.ImportName.empty() && Name != E.ImportName) {
-        Members.push_back(OF.createWeakExternal(E.ImportName, Name, false, M));
-        Members.push_back(OF.createWeakExternal(E.ImportName, Name, true, M));
-        continue;
-      }
-
       ImportNameType NameType;
       std::string ExportName;
       if (E.Noname) {
@@ -703,6 +718,23 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
       } else if (!E.ExportAs.empty()) {
         NameType = IMPORT_NAME_EXPORTAS;
         ExportName = E.ExportAs;
+      } else if (!E.ImportName.empty()) {
+        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 {
+          Deferred D;
+          D.Name = Name;
+          D.ImportType = ImportType;
+          D.Export = &E;
+          Renames.push_back(D);
+          continue;
+        }
       } else {
         NameType = getNameType(SymbolName, E.Name, M, MinGW);
       }
@@ -722,9 +754,25 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path,
         }
       }
 
+      RegularImports[applyNameType(NameType, Name)] = Name;
       Members.push_back(OF.createShortImport(Name, E.Ordinal, ImportType,
                                              NameType, ExportName, M));
     }
+    for (const auto &D : Renames) {
+      auto It = RegularImports.find(D.Export->ImportName);
+      if (It != RegularImports.end()) {
+        // We have a regular import entry for a symbol with the name we
+        // want to reference; produce an alias pointing at that.
+        StringRef Symbol = It->second;
+        if (D.ImportType == IMPORT_CODE)
+          Members.push_back(OF.createWeakExternal(Symbol, D.Name, false, M));
+        Members.push_back(OF.createWeakExternal(Symbol, D.Name, true, M));
+      } else {
+        Members.push_back(OF.createShortImport(
+            D.Name, D.Export->Ordinal, D.ImportType, IMPORT_NAME_EXPORTAS,
+            D.Export->ImportName, M));
+      }
+    }
     return Error::success();
   };
 
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/test/tools/llvm-dlltool/coff-decorated.def b/llvm/test/tools/llvm-dlltool/coff-decorated.def
index fc81f23d09d6c..f5685fb1cf0c6 100644
--- a/llvm/test/tools/llvm-dlltool/coff-decorated.def
+++ b/llvm/test/tools/llvm-dlltool/coff-decorated.def
@@ -7,12 +7,16 @@ EXPORTS
 CdeclFunction
 StdcallFunction at 4
 @FastcallFunction at 4
-StdcallAlias at 4==StdcallFunction at 4
+StdcallAlias at 4==StdcallFunction
 ??_7exception@@6B@
 StdcallExportName at 4=StdcallInternalFunction at 4
 OtherStdcallExportName at 4=CdeclInternalFunction
 CdeclExportName=StdcallInternalFunction at 4
 
+NoprefixStdcall at 4 == NoprefixStdcall at 4
+DecoratedStdcall at 4 == _DecoratedStdcall at 4
+UndecoratedStdcall at 4 == UndecoratedStdcall
+
 ; CHECK:      Name type: noprefix
 ; CHECK-NEXT: Export name: CdeclFunction
 ; CHECK-NEXT: Symbol: __imp__CdeclFunction
@@ -43,3 +47,15 @@ CdeclExportName=StdcallInternalFunction at 4
 ; CHECK-NEXT: Export name: CdeclExportName
 ; CHECK-NEXT: Symbol: __imp__CdeclExportName
 ; CHECK-NEXT: Symbol: _CdeclExportName
+; CHECK:      Name type: noprefix
+; CHECK-NEXT: Export name: NoprefixStdcall at 4
+; CHECK-NEXT: Symbol: __imp__NoprefixStdcall at 4
+; CHECK-NEXT: Symbol: _NoprefixStdcall at 4
+; CHECK:      Name type: name
+; CHECK-NEXT: Export name: _DecoratedStdcall at 4
+; CHECK-NEXT: Symbol: __imp__DecoratedStdcall at 4
+; CHECK-NEXT: Symbol: _DecoratedStdcall at 4
+; CHECK:      Name type: undecorate
+; CHECK-NEXT: Export name: UndecoratedStdcall
+; CHECK-NEXT: Symbol: __imp__UndecoratedStdcall at 4
+; CHECK-NEXT: Symbol: _UndecoratedStdcall at 4
diff --git a/llvm/test/tools/llvm-dlltool/coff-weak-exports.def b/llvm/test/tools/llvm-dlltool/coff-weak-exports.def
index dacc5f73530fd..b08040e29fa42 100644
--- a/llvm/test/tools/llvm-dlltool/coff-weak-exports.def
+++ b/llvm/test/tools/llvm-dlltool/coff-weak-exports.def
@@ -5,7 +5,11 @@
 
 LIBRARY test.dll
 EXPORTS
+AltTestFunction
+AltTestFunction2
+AltTestData
 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
@@ -16,18 +20,25 @@ ImpLibName2 = Implementation2 == AltTestFunction2
 ; matter for the import library
 ImpLibName3 = kernel32.Sleep
 
+; CHECK:      T AltTestFunction
+; CHECK-NEXT: T __imp_AltTestFunction
+; CHECK:      T AltTestFunction2
+; CHECK-NEXT: T __imp_AltTestFunction2
+; CHECK:      T ImpLibName
+; CHECK-NEXT: T __imp_ImpLibName
+; CHECK:      T ImpLibName3
+; CHECK-NEXT: T __imp_ImpLibName3
 ; CHECK:      U AltTestFunction
 ; CHECK-NEXT: W TestFunction
 ; CHECK:      U __imp_AltTestFunction
 ; CHECK-NEXT: W __imp_TestFunction
-; CHECK:      T ImpLibName
-; CHECK-NEXT: T __imp_ImpLibName
+; CHECK-NOT:  W TestData
+; CHECK:      U __imp_AltTestData
+; CHECK-NEXT: W __imp_TestData
 ; CHECK:      U AltTestFunction2
 ; CHECK-NEXT: W ImpLibName2
 ; CHECK:      U __imp_AltTestFunction2
 ; CHECK-NEXT: W __imp_ImpLibName2
-; CHECK:      T ImpLibName3
-; CHECK-NEXT: T __imp_ImpLibName3
 
 ; ARCH-NOT: unknown arch
 
diff --git a/llvm/test/tools/llvm-dlltool/renaming.def b/llvm/test/tools/llvm-dlltool/renaming.def
new file mode 100644
index 0000000000000..57fd472aa37cf
--- /dev/null
+++ b/llvm/test/tools/llvm-dlltool/renaming.def
@@ -0,0 +1,39 @@
+; RUN: llvm-dlltool -k -m i386 --input-def %s --output-lib %t.a
+; RUN: llvm-readobj %t.a | FileCheck %s
+; RUN: llvm-nm %t.a | FileCheck %s -check-prefix=CHECK-NM
+
+LIBRARY test.dll
+EXPORTS
+
+symbolname == actualimport
+
+dataname DATA == actualdata
+
+_wcstok == wcstok
+wcstok == wcstok_s
+
+; CHECK-NM-NOT: actualimport
+; CHECK-NM-NOT: actualdata
+
+; CHECK:      Type: code
+; CHECK-NEXT: Name type: export as
+; CHECK-NEXT: Export name: actualimport
+; CHECK-NEXT: Symbol: __imp__symbolname
+; CHECK-NEXT: Symbol: _symbolname
+
+; CHECK:      Type: data
+; CHECK-NEXT: Name type: export as
+; CHECK-NEXT: Export name: actualdata
+; CHECK-NEXT: Symbol: __imp__dataname
+
+; CHECK:      Type: code
+; CHECK-NEXT: Name type: export as
+; CHECK-NEXT: Export name: wcstok
+; CHECK-NEXT: Symbol: __imp___wcstok
+; CHECK-NEXT: Symbol: __wcstok
+
+; CHECK:      Type: code
+; CHECK-NEXT: Name type: export as
+; CHECK-NEXT: Export name: wcstok_s
+; CHECK-NEXT: Symbol: __imp__wcstok
+; CHECK-NEXT: Symbol: _wcstok

``````````

</details>


https://github.com/llvm/llvm-project/pull/98229


More information about the llvm-commits mailing list