Correctly handle dllimport on Windows

[alexcrichton]

Currently the compiler makes basically no attempt to correctly use dllimport. As a bit of a refresher, the Windows linker requires that if you're importing symbols from a DLL that they're tagged with dllimport. This helps wire things up correctly at runtime and link-time. To help us out, though, the linker will patch up a few cases where dllimport is missing where it would otherwise be required. If a function in another DLL is linked to without dllimport then the linker will inject a local shim which adds a bit of indirection and runtime overhead but allows the crate to link correctly. For importing constants from other DLLs, however, MSVC linker requires that dllimport is annotated correctly. MinGW linkers can sometimes workaround it (see this commit description.

If we're targeting windows, then the compiler currently puts dllimport on all imported constants from external crates, regardless of whether it's actually being imported from another crate. We rely on the linker fixing up all imports of functions. This ends up meaning that some crates don't link correctly, however (see this comment: #26591 (comment)).

We should fix the compiler's handling of dllimport in a few ways:

• Functions should be tagged with dllimport where appropriate
• FFI functions should also be tagged with dllimport where appropriate
• Constants should not always be tagged with dllimport if they're not actually being imported from a DLL.

I currently have a few thoughts running around in my head for fixing this, but nothing seems plausible enough to push on.

EDIT: Updated as @mati865 requested here.

[vadimcn]

So basically, with MSVC toolchain you are supposed to know whether you are going to be linking with a static lib or a dll version of the upstream library when compiling your crate.

Here's some ideas that come to my mind:

• Add attributes for marking up linking modes of extern crates (e.g. #[link_dll] extern crate foo).
  Cons: since this wart is unique to MSVC toolchain, it is unlikely that crate authors, who are not on Windows, will pay any attention to it. Besides, sometimes you simply can't tell in advance whether you'll need to link with a dll.
• Don't mark anything as dllimport
  • Code refs: let the linker fix them (extra jmp overhead in the case of dll library),
  • Data refs: avoid imported data altogether. Many Windows libraries take this approach using getter/setter functions where having exported data cannot be avoided. Cons: same as above.
• Mark all imported symbols as dllimport:
  • Code refs: let the linker fix them (indirect call overhead for static libraries).
  • Data refs: emit extra _imp__foo = &foo symbols into static libraries (linking succeeds, but adds the overhead of indirect access).
• Since rustc knows which flavor of the library it it going to link, it could take bitcode saved in Rust crates for LTO, add dllimport attribute to the symbols that need it, then re-emit the object file.
  Cons: increases compilation time, won't work for staticlibs (since those are not necessarily linked via rustc)

[alexcrichton]

I agree that strategies like #[link_dll] probably won't work out so hot, whatever we do probably needs to be baked into the compiler as much as possible instead of requiring annotations.

I'm personally a bit up in the air about how to tackle this. I only know of one absolute failure mode today (#26591) and otherwise the drawbacks today are lots of linker warnings and some extra indirection (not so bad). In that sense it doesn't seem incredibly urgent to tackle this, but it's certainly a wart!

[Diggsey]

Here's some links with useful information:
Using dllexport from static libs: https://support.microsoft.com/en-us/kb/141459
dllimport/dllexport details: https://msdn.microsoft.com/en-us/library/aa271769%28v=vs.60%29.aspx
#26591 is not specific to rust: http://blogs.msdn.com/b/oldnewthing/archive/2014/03/21/10509670.aspx

Looks like you can use a module definition file to get the current strategy to at least work in all cases, although it wouldn't remove the unnecessary indirection.

In general it seems impossible to get best performance in all situations, unless you either have two sets of binaries for dynamic vs static linking (as microsoft does with the standard library), or always build from source, and use compiler flags to fine-tune its behaviour. Only cargo really exists at a high enough level to be able to do that automatically.

Is there any danger of actual runtime unsafety here, eg. the msdn article implies that getting this wrong can result in code which uses the address of the constant getting the address of the import table instead?

[alexcrichton]

@Diggsey that MSDN blog post is actually different than #26591 I believe, handling dllexport to the best of my knowledge is done 100% correctly now that #27416 has landed. The bug you referenced, #26591, has to do with applying dllimport to all imported statics, and if they're not actually imported via a DLL then the linker won't include the object file. It's kinda the same problem, but not precisely.

Is there any danger of actual runtime unsafety here, eg. the msdn article implies that getting this wrong can result in code which uses the address of the constant getting the address of the import table instead?

I'm unaware of any impending danger, but which MSDN article were you referencing here? I'm under the impression that the linker fixes up references to functions automatically, and it apparently also fixes up dllimported statics to not use dllimport if not necessary.

[retep998]

@alexcrichton When attempting to link to some statics in the CRT directly from Rust, the code would link fine but the generated code was wrong. It would would follow the pointer to the static, but instead of accessing the static it then treated the static as another pointer and would segfault since the static wasn't supposed to be a pointer. I'm not sure whether this was the fault of dllimport or shoddy LLVM code generation or even rustc itself. We'll probably need some make tests covering all the possibilities.

[vadimcn]

The MS link's behavior to require an object file to be chosen for linking before starting to auto-insert __imp__ stubs for dllimport'ed symbols is pretty strange. I could not find any mention of this on MSDN (which, hopefully, would explain the rationale), but I can confirm that it indeed works this way.
Manually inserting __imp__foo = &foo symbol into the static library, as I proposed in option 3 above, fixed the case of data-only static lib, and seems to have no ill effects for dlls, so maybe that's the way to go?
Here's my test case:

---

Static library:

__declspec(dllexport) int foo = 1;
int* _imp__foo = &foo;

__declspec(dllexport) int bar() { return 2; }
void* _imp__bar = &bar;

Main executable:

__declspec(dllimport) int foo;
__declspec(dllimport) int bar();

int main()
{
    int f = foo;
    int b = bar();
    printf("%d %d\n", f, b);
    return 0;
}

If you comment out both _imp__foo and _imp__bar, you'll end up with an "unresolved external symbol" link error. Commenting out only one of them makes linking succeed with a warning.

[retep998]

@vadimcn That doesn't quite solve the issue if I need to link to statics from a library that I don't control, like a system library.