This is an archive of the discontinued LLVM Phabricator instance.

[llvm-dlltool] Fix creating stdcall/fastcall import libraries for i386
ClosedPublic

Authored by mstorsjo on Aug 9 2017, 1:23 PM.

Details

Summary

Hook up the -k option (that in the original GNU dlltool removes the @n suffix from the symbol that the final executable ends up linked to).

In llvm-dlltool, make sure that functions end up with the undecorate name type if this option is set and they are decorated. In mingw, when creating import libraries from def files instead of creating an import library as a side effect of linking a DLL, the symbol names in the def contain the stdcall/fastcall decoration (but no leading underscore).

By setting the undecorate name type, a linker linking to the import library will omit the decoration from the DLL import entry.

With this in place, mingw-w64 for i386 built with llvm-dlltool/clang produces import libraries that actually work.

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.Aug 9 2017, 1:23 PM
rnk added inline comments.Aug 9 2017, 1:38 PM
lib/Object/COFFModuleDefinition.cpp
232 ↗(On Diff #110465)

Surely this doesn't work for fastcall functions, though? They look like @foo@4

mstorsjo added inline comments.Aug 9 2017, 1:46 PM
lib/Object/COFFModuleDefinition.cpp
232 ↗(On Diff #110465)

Hmm, indeed, that's true. Haven't seen such, but they are used in mingw so they clearly need to be handled.

I think it would work with just using rfind instead of find, what do you think?

Unrelatedly - do you think we should move this whole block into the dlltool driver instead, since it's mingw specific? We could do a pass over the exports after parsing the def, before passing it to the writer.

rnk added inline comments.Aug 9 2017, 1:48 PM
lib/Object/COFFModuleDefinition.cpp
232 ↗(On Diff #110465)

Unfortunately, vectorcall messes rfind up, it does foo@@4: https://msdn.microsoft.com/en-us/library/deaxefa7.aspx

mstorsjo added inline comments.Aug 9 2017, 2:00 PM
lib/Object/COFFModuleDefinition.cpp
232 ↗(On Diff #110465)

Ah, crap. Mingw-w64 doesn't seem to have any such functions in the def files though. But I'd rather make it handle them correctly at the same time in any case.

Wouldn't the existing check for isDecorated above also prepend an underscore to it (even though there shouldn't be any, according to that ref)?

martell edited edge metadata.Aug 9 2017, 2:19 PM

This is related to what we were talking about earlier on the mingw-w64 mailing list about unifying all def files for all 4 platforms. :)
Maybe we can add i386 to the unified version and add a flag when building mingw-w64 to use that along with using a flag to disable MingwDef.
In general though we do not want MingwDef to exist long term and I did not originally add this because .def files should not contain @4 @8 etc in the first place.

Although this might not do any harm though if done correctly.
E.Name.substr(0, E.Name.find('@')); does not seem like enough because many symbols can have @ and other special characters.
Can you make sure only a number exists after the @?

martell added inline comments.Aug 9 2017, 2:48 PM
lib/Object/COFFModuleDefinition.cpp
232 ↗(On Diff #110465)

In general Mingw def files in general breaks the COFF spec too much.
http://cygwin.com/ml/binutils/2004-09/msg00031.html
We should just put it behind the --kill-at flag.
I already have a stub for this in Options.td that you can use.

This is related to what we were talking about earlier on the mingw-w64 mailing list about unifying all def files for all 4 platforms. :)
Maybe we can add i386 to the unified version and add a flag when building mingw-w64 to use that along with using a flag to disable MingwDef.
In general though we do not want MingwDef to exist long term and I did not originally add this because .def files should not contain @4 @8 etc in the first place.

To be quite frank - that's not possible.

The def files, for the mingw usecase, most definitely must have the @4 decorator, there's no way around it. When I build an object file for my app, with an undefined reference to a stdcall function, it is in the form _LibraryFunc@4 (or perhaps __imp__LibraryFunc@4). When I link it against the import library, the import library most definitely needs to have a symbol named _LibraryFunc@4 with the decorator. This is what was broken temporarily before since the first generation of "COFF: migrate def parser from LLD to LLVM [2/2]", that @rnk fixed in r304573.

When creating an import library as a side effect of linking a DLL, it's fine that the .def file only has got the undecorated name without the @4 suffix, the linker will map it to the real symbol name and export the decorated symbol name in the import library. (And for link.exe and lld, it sets name type "undecorate", which tells the linker to strip the extra suffix when linking to it, so that the DLL import table only contains the undecorated name. This is what is currently broken though, the name type doesn't get set correctly, that I try to fix here and in D36545.)

However, when creating an import library from scratch without linking a DLL, the information about what suffixes to use must come from somewhere, so they need to be in the def file. I'm not sure if it's even possible to create a proper import library for stdcall functions from scratch from a def file with MS link.exe/lib.exe, without having corresponding object files. (If someone knows how to do that, please tell me.) So the extra dlltool behaviour is warranted IMO.

Technically, the only thing this patch really would need to achieve is to tell writeImportLibrary to use ImportNameType = IMPORT_NAME_UNDECORATE. Currently it's only possible to signal that by having E.SymbolName != E.Name - so it would have the same effect just by setting E.Name to some dummy value. For consistency with the case when called from lld, I'm trying to make it produce the right, undecorated symbol name though.

lib/Object/COFFModuleDefinition.cpp
232 ↗(On Diff #110465)

Sure, I can hook this up behind that flag (which mingw-w64 passes).

@martell - to further show the point I'm trying to explain, have a look at the testcase I'm adding in this patch - it shows exactly what should happen. A def file with a decorated name produces an import library with the symbols still decorated, but with the undecorate name type.

mstorsjo updated this revision to Diff 110530.Aug 10 2017, 1:50 AM
mstorsjo retitled this revision from [llvm-dlltool] Fix creating stdcall import libraries for MinGW/i386 to [llvm-dlltool] Fix creating stdcall/fastcall import libraries for i386.
mstorsjo edited the summary of this revision. (Show Details)

Changed the undecorate action into something that should work for all of cdecl/stdcall/fastcall/vectorcall. Vectorcall doesn't work here yet since the def parser always prepends an underscore though (and fixing that is a separate topic that involves quite a bit of lld as well), but at least this particular line should be mostly fine now.

Extended the testcase to test both stdcall and fastcall (mingw has def files containing both). In order to handle decorated fastcall symbols in the def file, the parser had to get a minor adjustment as well.

Once done, this should hopefully also go into 5.0, since without it, llvm-dlltool doesn't work for mingw-w64 for i386. It's not a regression but a tool that is new for 5.0.

mstorsjo added a subscriber: hans.Aug 10 2017, 1:57 AM

When creating an import library as a side effect of linking a DLL, it's fine that the .def file only has got the undecorated name without the @4 suffix, the linker will map it to the real symbol name and export the decorated >symbol name in the import library. (And for link.exe and lld, it sets name type "undecorate", which tells the linker to strip the extra suffix when linking to it, so that the DLL import table only contains the undecorated name. This is what is currently broken though, the name type doesn't get set correctly, that I try to fix here and in D36545.)
However, when creating an import library from scratch without linking a DLL, the information about what suffixes to use must come from somewhere, so they need to be in the def file. I'm not sure if it's even possible to >create a proper import library for stdcall functions from scratch from a def file with MS link.exe/lib.exe, without having corresponding object files. (If someone knows how to do that, please tell me.) So the extra dlltool ?>behaviour is warranted IMO.

I'm confused to the purpose of the def files created at the time of linking a dll in that case.
Essentially the import library has information that the definition file is missing.
Thus we can not re-create the import library with that definition file, making it useless?

I presume Microsoft do not need to create import libraries from definition files because they have the original libraries create when they actually linked the system dll's
I follow now what you mean by this information needs to stores somewhere, it makes sense to have an extra mode in dlltool for this case.

The only alternative I can think of is a tool that directly creates an import library from a dll without using a definition file, this may be in somewhat related to the question I have above but is not useful for our case because we want to also be able to build the mingw-w64 crt on non windows systems( and support versions of windows outside just the one it was built on).

I'm confused to the purpose of the def files created at the time of linking a dll in that case.
Essentially the import library has information that the definition file is missing.
Thus we can not re-create the import library with that definition file, making it useless?

The main reason for a def file when linking a DLL, is to list which symbols are supposed to be exported. There's a difference between the MS and GNU toolchains here: With MS tools, no symbols are exported by default, unless you list them in a def file, have the symbol definition annotated with __declspec(dllexport) or tell the linker to export it with a /export argument. So you need to list your symbols (but without decorations, in the form you'd write them in a C file) in the def file.

(A second, older concern for def files is to specify the ordinal for each exported function, in case a caller wants to link to them by ordinal instead of by name. In those cases, the library developer needs to keep the function <-> ordinal mapping constant between releases.)

With GNU toolchains, all global symbols are exported by default. So for using lld as a linker in a mingw world, we'd need to implement this mode as well, but we can leave that until the main mingw driver frontend has been merged.

rnk accepted this revision.Aug 10 2017, 6:11 PM

However, when creating an import library from scratch without linking a DLL, the information about what suffixes to use must come from somewhere, so they need to be in the def file. I'm not sure if it's even possible to create a proper import library for stdcall functions from scratch from a def file with MS link.exe/lib.exe, without having corresponding object files. (If someone knows how to do that, please tell me.) So the extra dlltool behaviour is warranted IMO.

I guess in the original intended usage model of dlltool, the caller would supply the .o files destined for the final DLL, and we could look up the undecorated names and use the suffixes contained in the object file to fill in the decorated symbol name. However, it sounds like dlltool supports running with a standalone .def file, and in that case, we need some source of decorated symbol name.

This revision is now accepted and ready to land.Aug 10 2017, 6:11 PM
mstorsjo updated this revision to Diff 110674.Aug 10 2017, 10:41 PM

Fixed aliases with stdcall, added a test for that.

mstorsjo updated this revision to Diff 110850.Aug 12 2017, 1:05 PM

Updated to set the right name type for C++ symbols as well, including a testcase for it.

This revision was automatically updated to reflect the committed changes.