-
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
make windows compat_fn (crudely) work on Miri #95775
Conversation
r? @kennytm (rust-highfive has picked a reviewer for you, use r? to override) |
For a hack this looks fine. Though maybe I should ultimately look into better ways of doing lazy imports that don't rely on linker shenanigans. |
While at it I also fixed a Strict Provenance violation; this code has been round-tripping a function pointer through an integer. |
528f885
to
c599a4c
Compare
Oh I think I got a bit confused here because this is a muddy case of #95489, I think? |
@bors r=ChrisDenton |
📌 Commit c599a4c has been approved by |
…isDenton make windows compat_fn (crudely) work on Miri With rust-lang#95469, Windows `compat_fn!` now has to be supported by Miri to even make stdout work. Unfortunately, it relies on some outside-of-Rust linker hacks (`#[link_section = ".CRT$XCU"]`) that are rather hard to make work in Miri. So I came up with this crude hack to make this stuff work in Miri regardless. It should come at no cost for regular executions, so I hope this is okay. Cc rust-lang#95627 `@ChrisDenton`
⌛ Testing commit c599a4c with merge e16f44ed4c7ed8613eab77fe561e085fae95a2fa... |
💔 Test failed - checks-actions |
The job Click to see the possible cause of the failure (guessed by this bot)
|
Hm, that looks suspiciously like #95759 and not something caused by this PR, doesn't it? |
It does. @bors retry |
⌛ Testing commit c599a4c with merge c077ccf869dccf708e3a906e78a4e0500187b1a2... |
💔 Test failed - checks-actions |
Hmm, same problem. @bors retry |
The job Click to see the possible cause of the failure (guessed by this bot)
|
☀️ Test successful - checks-actions |
Finished benchmarking commit (1a4b9a8): comparison url. Summary:
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
Looks like this wasn't actually a performance improvement just a reversion to normal after a blip in the last run. |
That, or we should implement rust-lang/miri#450. |
With #95469, Windows
compat_fn!
now has to be supported by Miri to even make stdout work. Unfortunately, it relies on some outside-of-Rust linker hacks (#[link_section = ".CRT$XCU"]
) that are rather hard to make work in Miri. So I came up with this crude hack to make this stuff work in Miri regardless. It should come at no cost for regular executions, so I hope this is okay.Cc #95627 @ChrisDenton