-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Incorrect compilation / STATUS_ACCESS_VIOLATION when linking with lld with target-cpu set #72145
Comments
I was able to reproduce the errors by specifying (in addition to Possibly related to #64609? |
Possibly related to #71504? |
This was briefly mentioned during today's meeting or if it's just Windows thing: Removing nomination also. |
Hey Cleanup Crew ICE-breakers! This bug has been identified as a good cc @AminArria @camelid @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @senden9 @shekohex @sinato @spastorino @turboladen @woshilapin @yerke |
Second @rustbot ping windows |
This comment has been minimized.
This comment has been minimized.
Hey Windows Group! This bug has been identified as a good "Windows candidate". cc @arlosi @danielframpton @gdr-at-ms @kennykerr @luqmana @lzybkr @retep998 @rylev @sivadeilra @spastorino |
I spent some time trying to reproduce this, but wasn't able to. Can you provide more specific repro steps? Alternately, can you run rustc.exe under WinDbg, and show the stack trace and What model CPU are you running on? Is it possible that the target features you're requesting are not available on that CPU? |
@sivadeilra I'm using a I can still reproduce the Here are the exact reproduction steps, with
There's nothing more I did to reproduce it. However, just like with the broken Meta:
I'm also using the latest Visual Studio Build Tools, if that's relevant. I'll investigate with WinDbg later today and report back. |
Does not reproduce with
LLVM 10 from MSYS2 @rustbot modify labels: -O-windows +O-windows-msvc |
Also reproduces on a different system with a @sivadeilra Here is the WinDbg output and the stack trace. I used the rustc command specified in the cargo error log. Command output
Stack trace
Additionally the WinDbg output and the stack trace of running a miscompiled Command output
Stack trace
|
Looks like it's reading a value from TLS and adding an offset to use as a pointer that is not 32-byte aligned ( 00007ffd`9ac9dfdb 8b0517321700 mov eax, dword ptr [structopt_derive_afd65372c61b620b!_tls_index (00007ffd`9ae111f8)]
00007ffd`9ac9dfe1 65488b0c2558000000 mov rcx, qword ptr gs:[58h]
00007ffd`9ac9dfea 488b0cc1 mov rcx, qword ptr [rcx+rax*8]
00007ffd`9ac9dfee c5fc288120000000 vmovaps ymm0, ymmword ptr [rcx+20h] ds:00000210`4d54ced0=00
00007ffd`9ac9dff6 4883b92000000000 cmp qword ptr [rcx+20h], 0
00007ffd`9ac9dffe c5fc280d5a051100 vmovaps ymm1, ymmword ptr [structopt_derive_afd65372c61b620b!_ymm (00007ffd`9adae560)]
00007ffd`9ac9e006 488b8130000000 mov rax, qword ptr [rcx+30h] This is happening in the thread_local! {
static DUMMY_IMPL: RefCell<Option<TokenStream>> = RefCell::new(None);
} LLVM seems to believe it should be 32 byte aligned, but as far as I can tell when ntdll is allocating the TLS slots they're 16 byte aligned @_ZN16proc_macro_error5dummy10DUMMY_IMPL7__getit5__KEY17h4a7e8c80e38ac6e5E = internal thread_local global <{ [64 x i8] }> zeroinitializer, align 32 Sounds like a similar issue to #44056 When built with lld-link there's no alignment specified for the TLS section in the DLL:
But using the default msvc linker instead results in the correct alignment:
Seems like an LLD bug? |
Ah, that's great that you've found what appears to be the root cause. Are you / have you opened an issue for Is there more support needed from Windows devs? |
I filed a bug: https://bugs.llvm.org/show_bug.cgi?id=46473 |
rustbuild: Build tests with LLD if `use-lld = true` was passed Addresses rust-lang#76127 (comment). Our test suite is generally ready to run with an explicitly specified linker (rust-lang#45191), so LLD specified with `use-lld = true` works as well. Only 4 tests fail (on `x86_64-pc-windows-msvc`): ``` ui/panic-runtime/lto-unwind.rs run-make-fulldeps/debug-assertions run-make-fulldeps/foreign-exceptions run-make-fulldeps/test-harness ``` All of them are legitimate issues with LLD (or at least with combination Rust+LLD) and manifest in segfaults on access to TLS (rust-lang#76127 (comment)). UPD: These issues are caused by rust-lang#72145 and appear because I had `-Ctarget-cpu=native` set. UPD: Further commits build tests with LLD for non-MSVC targets and propagate LLD to more places when `use-lld` is enabled.
Finally got around to digging into LLD. Fix at https://reviews.llvm.org/D88637 |
@luqmana |
@petrochenkov Yes, it does. When the loader (ntdll) encounters a module (exe/dll) using TLS, it looks up the TLS directory in the PE image and uses the alignment specified there for all TLS data in the module. So my patch just makes LLD save the max alignment of all .tls sections it encounters in the final binary. So that means as long as you're annotating your In terms of allowed values, it's powers of 2 up to 8192 bytes with 16 as the default if unspecified. |
The compilation of crates fails in various ways when a certain combination of rustflags is set.
With this configuration I encountered the following issues:
sd
crate (version 0.7.5), rustc crashes with aSTATUS_ACCESS_VIOLATION
. See the detailled error output below.starship
, the resulting binary doesn't work correctly (see the issue here).Using each rustflag individually does not result in this issue. I could reproduce this both on stable and the recent nightly, and with
target-cpu
set to eithersandybridge
,haswell
orznver2
(without trying more).Meta
rustc --version --verbose
:Error during cargo install --force sd
Also possibly relevant for #71520.
The text was updated successfully, but these errors were encountered: