This is an archive of the discontinued LLVM Phabricator instance.

[LLD] [COFF] Support MinGW automatic dllimport of data
ClosedPublic

Authored by mstorsjo on Aug 17 2018, 1:01 PM.

Details

Summary

Normally, in order to reference exported data symbols from a different DLL, the declarations need to have the dllimport attribute, in order to use the __imp_<var> symbol (which contains an address to the actual variable) instead of the variable itself directly. This isn't an issue in the same way for functions, since any reference to the function without the dllimport attribute will end up as a reference to a thunk which loads the actual target function from the import address table (IAT).

GNU ld, in MinGW environments, supports automatically importing data symbols from DLLs, even if the references didn't have the appropriate dllimport attribute. Since the PE/COFF format doesn't support the kind of relocations that this would require, the MinGW's CRT startup code has an custom framework of their own for manually fixing the missing relocations once module is loaded and the target addresses in the IAT are known.

For this to work, the linker (originall in GNU ld) creates a list of remaining references needing fixup, which the runtime processes on startup before handing over control to user code.

While this feature is rather controversial, it's one of the main features allowing unix style libraries to be used on windows without any extra porting effort.

Some sort of automatic fixing of data imports is also necessary for the itanium C++ ABI on windows (as clang implements it right now) for importing vtable pointers in certain cases, see D43184 for some discussion on that.

The runtime pseudo relocation handler supports 8/16/32/64 bit addresses, either PC relative references (like IMAGE_REL_*_REL32*) or absolute references (IMAGE_REL_AMD64_ADDR32, IMAGE_REL_AMD64_ADDR32, IMAGE_REL_I386_DIR32). On linking, the relocation is handled as a relocation against the corresponding IAT slot. For the absolute references, a normal base relocation is created, to update the embedded address in case the image is loaded at a different address.

The list of runtime pseudo relocations contains the RVA of the imported symbol (the IAT slot), the RVA of the location the relocation should be applied to, and a size of the memory location. When the relocations are fixed at runtime, the difference between the actual IAT slot value and the IAT slot address is added to the reference, doing the right thing for both absolute and relative references.

With this patch alone, things work fine for i386 binaries, and mostly for x86_64 binaries, with feature parity with GNU ld. Despite this,
there are a few gotchas:

  • References to data from within code works fine on both x86 architectures, since their relocations consist of plain 32 or 64 bit absolute/relative references. On ARM and AArch64, references to data doesn't consist of a plain 32 or 64 bit embedded address or offset in the code. On ARMNT, it's usually a MOVW+MOVT instruction pair represented by a IMAGE_REL_ARM_MOV32T relocation, each instruction containing 16 bit of the target address), on AArch64, it's usually an ADRP+ADD/LDR/STR instruction pair with an even more complex encoding, storing a PC relative address (with a range of +/- 4 GB). This could theoretically be remedied by extending the runtime pseudo relocation handler with new relocation types, to support these instruction encodings. This isn't an issue for GCC/GNU ld since they don't support windows on ARMNT/AArch64.
  • For x86_64, if references in code are encoded as 32 bit PC relative offsets, the runtime relocation will fail if the target turns out to be out of range for a 32 bit offset.
  • Fixing up the relocations at runtime requires making sections writable if necessary, with the VirtualProtect function. In Windows Store/UWP apps, this function is forbidden.

These limitations will be addressed by a few later patches in lld and llvm.

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.Aug 17 2018, 1:01 PM
rnk added a comment.Aug 20 2018, 10:22 AM

Sigh. ELF supports too much and lets people do too much, and then we have to go reimplement that functionality badly for other object file formats that tried to be simple.

Anyway, we should do something. I understand that for many developers, Windows is a corner case platform and they are going to spend approximately zero effort porting their code to it, so this feature is valuable to them.

Will subsequent LLVM patches make the compiler use more general code sequences for global variable access, like an IAT load? That would address the list of architectural limitations you outlined.

COFF/Chunks.h
161 ↗(On Diff #161304)

This can be a reference to indicate that it is required and not nullable.

test/COFF/autoimport-arm-code.s
10 ↗(On Diff #161304)

Do we have a table to print relocations symbolically?

In D50917#1206111, @rnk wrote:

Sigh. ELF supports too much and lets people do too much, and then we have to go reimplement that functionality badly for other object file formats that tried to be simple.

Anyway, we should do something. I understand that for many developers, Windows is a corner case platform and they are going to spend approximately zero effort porting their code to it, so this feature is valuable to them.

Indeed... While using this feature to handle libraries public APIs with data symbols is one thing, the fact that it more or less is required for the itanium C++ ABI tipped the scales for me about implementing it (instead of the clang-contained kludge I tried before).

Will subsequent LLVM patches make the compiler use more general code sequences for global variable access, like an IAT load? That would address the list of architectural limitations you outlined.

Yes, exactly. I've got patches that make every global variable access (except those where we do see a definition, making it sure that it really is DSO local) go via a variable stub. And once you have such a stub, you can even replace accesses to it with an access to __imp_<sym> in the linker, avoiding most runtime relocations (except for cases where the pointer is used in initialized data, like in the C++ cases). And finally we can make all sections that still contain runtime relocations writable, avoiding any need for VirtualProtect at runtime.

COFF/Chunks.h
161 ↗(On Diff #161304)

Ok, will change.

test/COFF/autoimport-arm-code.s
10 ↗(On Diff #161304)

Yes, there is something in lib/Object, I'll try to make use of it.

mstorsjo updated this revision to Diff 161555.Aug 20 2018, 1:42 PM

Printing relocs symbolically (depends on D50995), using a vector reference instead of a pointer as parameter.

Any further comments on this, or is this tolerable enough?

pcc added inline comments.Aug 24 2018, 12:49 PM
COFF/Chunks.h
420 ↗(On Diff #161555)

Would it be simpler to have a single PseudoRelocChunk for all the pseudo-relocs instead of creating a separate one for each reloc?

447 ↗(On Diff #161555)

It looks like this function is not used.

COFF/SymbolTable.cpp
208 ↗(On Diff #161555)

It looks like the RVA of the IAT entry is stored in the pseudo-reloc. So does this need to be relocated to the IAT entry? Could you replace it with a DefinedAbsolute of value 0 for example?

mstorsjo added inline comments.Aug 24 2018, 1:01 PM
COFF/Chunks.h
420 ↗(On Diff #161555)

I guess that could work just as well.

447 ↗(On Diff #161555)

Hmm, indeed, will remove.

COFF/SymbolTable.cpp
208 ↗(On Diff #161555)

Yes, this needs to be relocated to the IAT entry - DefinedAbsolute with a zero value won't work.

At runtime, the runtime pseudo reloc mechanism only adds the difference between IAT entry itself and IAT entry value to the relocation address; this makes it work the same both for absolute addresses and RIP relative addressing.

mstorsjo updated this revision to Diff 162463.Aug 24 2018, 1:25 PM

Applied @pcc's suggestions; the pseudo reloc table/list turned out much cleaner/nicer with just one Chunk containing it all, removed the unused method.

ruiu added a comment.Aug 26 2018, 10:41 PM

This patch looks okay to me though it is unfortunate that we need to support it. This patch overall lacks explanation -- at somewhere in the patch, I'd explain what is automatic import and how lld supports that functionality. Once you get an overview, details should hopefully become self-explanatory.

COFF/Chunks.cpp
422 ↗(On Diff #162463)

It's not clear to me what this function is supposed to return in the first place. Could you write a brief function comment?

493 ↗(On Diff #162463)

What this function is doing? Please write a brief function comment.

494 ↗(On Diff #162463)

If Res is always empty at the entry of this function, return a new instance of a std::vector instead of taking an instance of std::vector as a reference.

631 ↗(On Diff #162463)

There's no explanation of "pseudo relocation". Please explain it somewhere.

COFF/Chunks.h
424 ↗(On Diff #162463)

Please add comment to explain what this is.

436–437 ↗(On Diff #162463)

Please explain the motivation of defining these classes in the first place. In particular, I'd mention that we don't really need this for MS compatibility but this is for MinGW.

445 ↗(On Diff #162463)

Ditto

COFF/SymbolTable.cpp
197 ↗(On Diff #162463)

This function is already too long. Please consider splitting to a new function.

COFF/Writer.cpp
1148 ↗(On Diff #162463)

Please add a comment.

mstorsjo added inline comments.Aug 26 2018, 11:46 PM
COFF/Chunks.cpp
422 ↗(On Diff #162463)

Sure, will do - with this explanation:

// Check whether a static relocation of type Type can be deferred and
// handled at runtime as a pseudo relocation (for references to a module
// local variable, which turned out to actually need to be imported from
// another DLL) This returns the size the relocation is supposed to update,  
// in bits, or 0 if the relocation cannot be handled as a runtime pseudo  
// relocation.
493 ↗(On Diff #162463)

Will do, like this:

// Append information to the provided vector about all relocations that
// need to be handled at runtime as runtime pseudo relocations (references
// to a module local variable, which turned out to actually need to be
// imported from another DLL).
494 ↗(On Diff #162463)

It isn't empty at entry; we append entries to an existing vector.

631 ↗(On Diff #162463)

Adding a comment like this above here:

// MinGW specific, for the "automatic import of variables from DLLs" feature.

I added a slightly longer comment about the same class in Chunks.h as well.

COFF/Chunks.h
424 ↗(On Diff #162463)

I'll add a comment like this:

// MinGW specific, for the "automatic import of variables from DLLs" feature.
// This provides the table of runtime pseudo relocations, for variable
// references that turned out to need to be imported from a DLL even though
// the reference didn't use the dllimport attribute. The MinGW runtime will
// process this table after loading, before handling control over to user
// code.
436–437 ↗(On Diff #162463)

Well the EmptyChunk isn't inherently anything MinGW specific at all, I could imagine that it would be useful for any case where you want to attach a Symbol to a specific place in an OutputSection, without actually producing any content.

I can mention that it is used for the MinGW specific __RUNTIME_PSEUDO_RELOC_LIST_END__ symbol if that makes it clearer?

445 ↗(On Diff #162463)

Added a comment like this:

// MinGW specific; information about one individual location in the image
// that needs to be fixed up at runtime after loading. This represents
// one individual element in the PseudoRelocTableChunk table.
COFF/SymbolTable.cpp
197 ↗(On Diff #162463)

Done, split the extra new added code to a separate method.

COFF/Writer.cpp
1148 ↗(On Diff #162463)

I wrote:

// MinGW specific. Gather all relocations that are imported from a DLL even
// though the code didn't expect it to, produce the table that the runtime
// uses for fixing them up, and provide the synthetic symbols that the
// runtime uses for finding the table.
mstorsjo updated this revision to Diff 162618.Aug 26 2018, 11:48 PM
mstorsjo edited the summary of this revision. (Show Details)

Updated according to @ruiu's comments. I also updated the patch summary (the first three paragraphs are updated/rewritten) to introduce the feature a little better.

ruiu added inline comments.Aug 27 2018, 12:26 AM
COFF/Chunks.cpp
428 ↗(On Diff #162618)

I'd add MinGW specific. to all functions that are added in this patch.

525 ↗(On Diff #162618)

SizeInBits is called Flags in RuntimePseudoReloc class. I was little confused because Flags sounds like bit flags. Can you consistently use one name?

COFF/SymbolTable.cpp
163 ↗(On Diff #162618)

nit: add a blank line.

COFF/Writer.cpp
191 ↗(On Diff #162618)

Looks like this variable is used only by one function. Can this be a local variable?

mstorsjo added inline comments.Aug 27 2018, 12:36 AM
COFF/Chunks.cpp
428 ↗(On Diff #162618)

Ok, will do, except for methods where MinGW is part of the name where it otherwise is apparent.

525 ↗(On Diff #162618)

This dualism comes from the MinGW sources itself; the field is called Flags in the struct, but so far the only flags used is the size of the relocation, but I guess they've left it open to use other higher bits. I'll try to clarify this.

COFF/Writer.cpp
191 ↗(On Diff #162618)

Not really, the PseudoRelocTableChunk contains an ArrayRef to this, so it needs to be kept alive until the chunks actually are written.

mstorsjo updated this revision to Diff 162626.Aug 27 2018, 12:37 AM

Clarified the comments further, as requested.

ruiu added inline comments.Aug 27 2018, 12:49 AM
COFF/Writer.cpp
191 ↗(On Diff #162618)

Then maybe you can transfer the ownership of the object?

mstorsjo updated this revision to Diff 162627.Aug 27 2018, 12:57 AM

Transferring ownership of the runtime pseudo relocs vector to the Chunk that will write it.

ruiu accepted this revision.Aug 27 2018, 1:12 AM

LGTM

Thank you for doing this. It looks much less intrusive than I originally thought. Also, thank you for all your code comments. I believe it the new code is easy to understand.

COFF/Writer.cpp
1152 ↗(On Diff #162627)

Since the scope of this variable is much narrower than before, I'd use a shorter name, such as Rels.

This revision is now accepted and ready to land.Aug 27 2018, 1:12 AM

LGTM

Thank you for doing this. It looks much less intrusive than I originally thought. Also, thank you for all your code comments. I believe it the new code is easy to understand.

Thanks for taking the time to review it! I was also initially pleasantly surprised by how easy it was to implement.

COFF/Writer.cpp
1152 ↗(On Diff #162627)

Oh, right, yes. Will change before committing.

This revision was automatically updated to reflect the committed changes.