[win][aarch64] Fix linking statics on Arm64EC, take 2

[dpaoliello]

Arm64EC builds recently started to fail due to the linker not finding a symbol:

symbols.o : error LNK2001: unresolved external symbol #_ZN3std9panicking11EMPTY_PANIC17hc8d2b903527827f1E (EC Symbol)
          C:\Code\hello-world\target\arm64ec-pc-windows-msvc\debug\deps\hello_world.exe : fatal error LNK1120: 1 unresolved externals

It turns out that EMPTY_PANIC is a new static variable that was being exported then imported from the standard library, but when exporting LLVM didn't prepend the name with # (as only functions are prefixed with this character), whereas Rust was prefixing with # when attempting to import it.

The fix is to have Rust not prefix statics with # when importing.

Adding tests discovered another issue: we need to correctly mark static exported from dylibs with DATA, otherwise MSVC's linker assumes they are functions and complains that there is no exit thunk for them.

Fixes #138541

Resurrects #140176 now that #141061 is merged, which removes the incompatibility with __rust_no_alloc_shim_is_unstable.

r? @wesleywiser

CC @bjorn3

[rustbot]

This PR modifies run-make tests.

cc @jieyouxu

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

[bjorn3]

@bors2 try jobs=x86_64-msvc-,x86_64-mingw-,dist-aarch64-msvc

[rust-bors]

⌛ Trying commit fec0e2a with merge 9559cca…

To cancel the try build, run the command @bors2 try cancel.

[rust-bors]

💔 Test failed

• CI ❌

[bjorn3]

@bors2 try jobs=x86_64-msvc-*,x86_64-mingw-*,dist-aarch64-msvc

Edit: Seems to be rust-lang/bors#314

[rust-bors]

Unknown value for argument "jobs".

[jieyouxu]

You'll need to use the try-job-in-PR-description form

[bjorn3]

@bors2 try

[rust-bors]

⌛ Trying commit fec0e2a with merge bdee317…

To cancel the try build, run the command @bors2 try cancel.

[rust-bors]

☀️ Try build successful (CI)
Build commit: bdee317 (bdee3175f59dea0de778f1bf1f8bb597716403ed, parent: 18491d5be00eb3ed2f1ccee2ac5b792694f2a7a0)

[bjorn3]

r=me with the above change

[bjorn3]

@bors r+

[bors]

📌 Commit 2602653 has been approved by bjorn3

It is now in the queue for this repository.

[ladipro]

Adding tests discovered another issue: we need to correctly mark static exported from dylibs with DATA, otherwise MSVC's linker assumes they are functions and complains that there is no exit thunk for them.

This change broke dynamic linking, as far as I can tell. With DATA, the linker doesn't put the bare static name into the import lib, only the __imp_-prefixed one.

For example, the import lib for tests\ui\statics\auxiliary\check_static_recursion_foreign_helper.rs previously had:

    7 public symbols

      3A0 __IMPORT_DESCRIPTOR_check_static_recursion_foreign_helper
      642 __NULL_IMPORT_DESCRIPTOR
      796 check_static_recursion_foreign_helper_NULL_THUNK_DATA
      9EC __imp_test_static
      9EC test_static
      92C __imp_rust_metadata_check_static_recursion_foreign_helper_31b3b31070780c5b
      92C rust_metadata_check_static_recursion_foreign_helper_31b3b31070780c5b

now it has only:

    5 public symbols

      2F4 __IMPORT_DESCRIPTOR_check_static_recursion_foreign_helper
      596 __NULL_IMPORT_DESCRIPTOR
      6EA check_static_recursion_foreign_helper_NULL_THUNK_DATA
      940 __imp_test_static
      880 __imp_rust_metadata_check_static_recursion_foreign_helper_31b3b31070780c5b

The test happens to pass because it doesn't really access the static cross-crate. When it's modified to read test_static from its main function, it fails to link (x64 MSVC linker):

a.check_recursion_foreign.55e77c91ee99656d-cgu.0.rcgu.o : error LNK2019: unresolved external symbol test_static referenced in function _ZN23check_recursion_foreign4main17h519e10ac4c6da3d1E

The broken scenario doesn't look all that niche to me so maybe this should be fixed ASAP or reverted?
cc @bjorn3 @wesleywiser

[bjorn3]

As I understand it this PR is fundamentally necessary to support arm64ec because the non-__imp symbol will cause a thunk to be generated, which is wrong for non-function symbols. I believe not exporting the non-__imp symbol is technically correct for statics and would work for imports using __dllimport. Rust however doesn't have an equivalent to __dllimport, so rustc makes guesses about whether to use it or not and in this case guessed wrong. Rustc currently sets __dllimport for statics imported from another crate and for statics imported through #[link(name = "foo", kind = "dylib")] extern "C" {}. This test does neither.

[ladipro]

Do I understand it correctly then that the test is written in a way that's not guaranteed to work and likely does not represent a real world scenario? For what it's worth, it looks like the helper crate was perhaps intended to be linked statically, given the crate_type attribute:

https://github.com/rust-lang/rust/blob/4b3ba5844e8831c9b3ee5a5643cdff5da0677426/tests/ui/statics/auxiliary/check_static_recursion_foreign_helper.rs#L6C1-L6C23

[bjorn3]

For what it's worth, it looks like the helper crate was perhaps intended to be linked statically, given the crate_type attribute:

Yeah, how did that even end up getting dynamically linked?

[ladipro]

compiletest overrides the attribute with a rustc crate-type command line option. I have posted #148064 as a candidate fix.