From 62bf9e01723478f11a9bded5940ab8410c5faa34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Tue, 16 Apr 2024 14:27:44 +0300 Subject: [PATCH] [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 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 Exports, - COFF::MachineTypes Machine, bool MinGW, - ArrayRef NativeExports = std::nullopt); +Error writeImportLibrary(StringRef ImportName, StringRef Path, + ArrayRef Exports, + COFF::MachineTypes Machine, bool MinGW, + ArrayRef 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 Exports, MachineTypes Machine, bool MinGW, - ArrayRef NativeExports) { + ArrayRef 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 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; }