Create _imp__ symbols also when doing ThinLTO

[Zoxc]

When generating a rlib crate on Windows we create dllimport / _imp__ symbols for each global. This effectively makes the rlib contain an import library for itself and allows them to both be dynamically and statically linked. However when doing ThinLTO we do not generate these and thus we end up with missing symbols. Microsoft's link can fix these up (and emits warnings), but lld seems to currently be unable to.

This PR also does this generation for ThinLTO avoiding those issues with lld and also avoids the warnings on link.

This is an workaround for #81408.

cc @lqd

[rustbot]

r? @compiler-errors

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[wesleywiser]

Can we add a simple test case to demonstrate that the _imp__ symbols are generated now? Asm or llvm-ir test would be sufficient if it's too complex to do an end-to-end test.

[Zoxc]

I think we'd need to inspect the symbols in the linker inputs to test this.

[lqd]

if it's too complex to do an end-to-end test

@wesleywiser what would you want to see in an end-to-end test? #81408 will pass before and after this PR, until we revert #103353 -- is lqd@f9fb8e9 what you have in mind? (note it doesn't duplicate cargo's use of -Clinker-plugin-lto as it wasn't needed to reproduce the failure, but easily can be added if need be of course)

[ChrisDenton]

Note that merged this fixes #111480

[wesleywiser]

Talked with @lqd offline about his comment and I'm fine with merging this as-is.

[wesleywiser]

Thanks @Zoxc!

@bors r+ rollup

[bors]

📌 Commit 562c0d1 has been approved by wesleywiser

It is now in the queue for this repository.

[lqd]

@ChrisDenton as you've done the most investigation on #111480, do we need a test for that issue, or is the one added in this PR good for it as well? That is, it seems like #111480 is a duplicate of #81408?

[ChrisDenton]

Fully minimizing #111480 seems to be near enough the test case here. I guess there is a difference in that #111480 does compiles a cdylib as well.

[lqd]

IIRC there are a few issues when multiple crate types are used with cargo including cdylibs, and LTO enabled, like rust-lang/cargo#9672; other issues happen with PGO as well #117220

(I hope these issues don't emulate -Zdylib-lto in practice...)

I guess we can leave #111480 as E-needs-test and people can use this one as a base and also test a cdylib in there if they want. The missing _imp symbols should be the root cause in both types of libraries anyways.

[ChrisDenton]

Yeah I don't feel strongly either way. I'm happy with the test here as it does exercise the underlying problem.