Skip to content

Miri on Windows: run .CRT$XLB linker section on thread-end #123937

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Apr 15, 2024

Conversation

RalfJung
Copy link
Member

Hopefully fixes #123583

First commit is originally by @bjorn3

r? @oli-obk
Cc @ChrisDenton

@rustbot rustbot added O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 14, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 14, 2024

The Miri subtree was changed

cc @rust-lang/miri

@RalfJung RalfJung force-pushed the miri-link-section branch from e3c50f7 to 5934aaa Compare April 14, 2024 18:09
@RalfJung RalfJung changed the title Miri: run .CRT$XLB linker section on thread-end Miri on Windows: run .CRT$XLB linker section on thread-end Apr 14, 2024
@ChrisDenton
Copy link
Member

ChrisDenton commented Apr 15, 2024

Rather than #[cfg_attr(miri, used)], could we maybe have like a #[rustc_windows_run_tls_dtors] attribute that backends can choose what to do with? Currently it would be ignored by most backends but that could change in the future.

@RalfJung
Copy link
Member Author

I don't have the time to design and implement such a feature, sorry.

@bjorn3
Copy link
Member

bjorn3 commented Apr 15, 2024

With the first commit you should be able to remove the miri special case at

#[cfg(any(miri, not(all(target_os = "linux", target_env = "gnu"))))]

@RalfJung
Copy link
Member Author

RalfJung commented Apr 15, 2024

I haven't taken your entire commit, only the part that turns link_section into an array. So I can't remove that special case, no -- that could be done in a future PR.

This is intended to be a targeted fix for the Windows issue that makes rustc CI spuriously fail.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 15, 2024

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 15, 2024

📌 Commit 5934aaa has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 15, 2024
@RalfJung
Copy link
Member Author

@bors p=1
hopefully fixes CI failures that break unrelated PRs

@bors
Copy link
Collaborator

bors commented Apr 15, 2024

⌛ Testing commit 5934aaa with merge 0230848...

@bors
Copy link
Collaborator

bors commented Apr 15, 2024

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 0230848 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 15, 2024
@bors bors merged commit 0230848 into rust-lang:master Apr 15, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 15, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0230848): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-4.2% [-4.2%, -4.2%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -4.2% [-4.2%, -4.2%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 677.082s -> 676.684s (-0.06%)
Artifact size: 316.00 MiB -> 316.01 MiB (0.00%)

@RalfJung RalfJung deleted the miri-link-section branch April 16, 2024 05:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. O-windows Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Miri sometimes reports memory leak in thread-local storage on *-pc-windows-gnu
7 participants