[PATCH] D36548: [llvm-dlltool] Fix creating stdcall import libraries for MinGW/i386

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D36548#837373, @martell wrote:

> 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 at 4` (or perhaps `__imp__LibraryFunc at 4`). When I link it against the import library, the import library most definitely needs to have a symbol named `_LibraryFunc at 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 https://reviews.llvm.org/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.



================
Comment at: lib/Object/COFFModuleDefinition.cpp:232
+        E.SymbolName = E.Name;
+        E.Name = E.Name.substr(0, E.Name.find('@'));
+      }
----------------
martell wrote:
> mstorsjo wrote:
> > rnk wrote:
> > > mstorsjo wrote:
> > > > rnk wrote:
> > > > > Surely this doesn't work for fastcall functions, though? They look like @foo at 4
> > > > 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.
> > > Unfortunately, vectorcall messes `rfind` up, it does foo@@4: https://msdn.microsoft.com/en-us/library/deaxefa7.aspx
> > 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)?
> 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.
Sure, I can hook this up behind that flag (which mingw-w64 passes).


https://reviews.llvm.org/D36548