[llvm-dlltool] Make sure to strip decorations from ExtName for renamed exports

[mstorsjo]

Summary

ExtName should not be decorated, just like Name.

This avoids double decoration on symbols in import libraries that use = for renaming functions. (Weak aliases, which use ==, worked fine with respect to decoration.)

[ruiu]

LGTM

[mstorsjo]

@hans - This is a pretty well isolated fix that might be mergeworthy if you still have the gates open, it shouldn't have any risk of regressions outside of its own scope at least :-)

[jacek]

It looks like decoration is killed too aggressively now. If aliasee symbol has no decoration, aliased symbol is stripped. For example:

LIBRARY test.dll

EXPORTS
TestForward@4=TestFunc

will result with _TestForward symbol instead of _TestForward@4

[mstorsjo]

In D66617#1642886, @jacek wrote:

It looks like decoration is killed too aggressively now. If aliasee symbol has no decoration, aliased symbol is stripped.

Ah, crap. @hans - no point in backporting this, as it's a bigger mess than it seemed first.

@jacek - the issue stems from how these work in msvc land. You can have /export:foo=bar where the decoration from the actual bar symbol gets transplanted on foo, see https://github.com/llvm/llvm-project/blob/master/llvm/lib/Object/COFFImportFile.cpp#L604. In this case, the original symbol has no decoration to transplant on the stripped ExtName.

[jacek]

Oh, I see. FWIW, it shouldn't be too hard to work around the problem in Wine. .def file is generated automatically, so we could just skip aliases for .def file used for importlibs. It seems like it would help a hypethetical Wine MSCV port anyway and it might be better for LLVM to be MSVC compatible in this case. I will experiment with it.

[jacek]

I can confirm that with a simple Wine change, its importlibs work fine.

[mstorsjo]

In D66617#1643092, @jacek wrote:

Oh, I see. FWIW, it shouldn't be too hard to work around the problem in Wine. .def file is generated automatically, so we could just skip aliases for .def file used for importlibs.

That sounds good

and it might be better for LLVM to be MSVC compatible in this case

Well, when invoked as llvm-dlltool (or lld in mingw mode), it should definitely behave as the gnu equivalents. (Whether each bug is worth spending time on is ofc a different matter... And gnu dlltool has got an awful lot of different flags affecting how the symbol decorations are mangled.)

llvm/trunk/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

for (COFFShortExport& E : Def->Exports) {
       // starting prefix here (either _ for cdecl/stdcall, @ for fastcall
       // or ? for C++ functions). Vectorcall functions won't have any
       // fixed prefix, but the function base name will still be at least
       // one char.
       E.Name = E.Name.substr(0, E.Name.find('@', 1));
       // By making sure E.SymbolName != E.Name for decorated symbols,
       // writeImportLibrary writes these symbols with the type
       // IMPORT_NAME_UNDECORATE.
+      E.ExtName = E.ExtName.substr(0, E.ExtName.find('@', 1));
     }
   }
 
   if (writeImportLibrary(Def->OutputFile, Path, Def->Exports, Machine, true))
     return 1;
   return 0;
 }

llvm/trunk/test/tools/llvm-dlltool/coff-decorated.def

 ; 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
 CdeclFunction
 StdcallFunction@4
 @FastcallFunction@4
 StdcallAlias@4==StdcallFunction@4
 ??_7exception@@6B@
+StdcallExportName@4=StdcallInternalFunction@4
 
 ; CHECK: Name type: noprefix
 ; CHECK: Symbol: __imp__CdeclFunction
 ; CHECK: Symbol: _CdeclFunction
 ; CHECK: Name type: undecorate
 ; CHECK: Symbol: __imp__StdcallFunction@4
 ; CHECK: Symbol: _StdcallFunction@4
 ; CHECK: Name type: undecorate
 ; CHECK: Symbol: __imp_@FastcallFunction@4
 ; CHECK: Symbol: @FastcallFunction@4
 ; CHECK: Name type: name
 ; CHECK: Symbol: __imp_??_7exception@@6B@
 ; CHECK: Symbol: ??_7exception@@6B@
 ; CHECK-NM: W _StdcallAlias@4
 ; CHECK-NM: U _StdcallFunction@4
+; CHECK: Symbol: __imp__StdcallExportName@4{{$}}
+; CHECK: Symbol: _StdcallExportName@4{{$}}