Skip to content
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

[stable] probe-stack=call everywhere again, for now. #83412

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Mar 23, 2021

To buy time on issue 83139, revert effect of PR 77885: We will not conditionally
enable probe-stack=inline-asm on LLVM 11+ anymore on any of our targets that
opted into doing so on PR #77885 (and were subsequently configured to do so in a
fine grained manner on PR #80838).

After we resolve 83139 (potentially by backporting a fix to LLVM, or potentially
by deciding that one cannot rely on the quality of our DWARF output in the
manner described in issue 83139), we can change this back.

cc #83139

@rust-highfive
Copy link
Collaborator

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against stable. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 23, 2021
To buy time on issue 83139, revert effect of PR 77885: We will not conditionally
enable probe-stack=inline-asm on LLVM 11+ anymore on any of our targets that
opted into doing so on PR rust-lang#77885 (and were subsequently configured to do so in a
fine grained manner on PR rust-lang#80838).

After we resolve 83139 (potentially by backporting a fix to LLVM, or potentially
by deciding that one cannot rely on the quality of our DWARF output in the
manner described in issue 83139), we can change this back.

(Update: fixed formatting issue.)
@Mark-Simulacrum Mark-Simulacrum assigned nagisa and unassigned lcnr Mar 23, 2021
@Mark-Simulacrum Mark-Simulacrum changed the title On stable, probe-stack=call everywhere again, for now. [stable] probe-stack=call everywhere again, for now. Mar 23, 2021
@nagisa
Copy link
Member

nagisa commented Mar 23, 2021

You may need to adjust some of the tests, I think, at least in src/test/assembly/stack-probes.rs. Otherwise LGTM.

@pnkfelix pnkfelix force-pushed the stable-revert-effect-of-pr-77885-for-issue-83139 branch from e35125b to 1aa1785 Compare March 23, 2021 15:37
@pnkfelix
Copy link
Member Author

pnkfelix commented Mar 23, 2021

You may need to adjust some of the tests, I think, at least in src/test/assembly/stack-probes.rs. Otherwise LGTM.

Oh yes, let me look at those. I got so wrapped up in the #80838 stuff that I forgot to double-check what else was in PR #77885

@pnkfelix
Copy link
Member Author

@bors r=nagisa p=999

@bors
Copy link
Contributor

bors commented Mar 23, 2021

📌 Commit b75894e has been approved by nagisa

@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 Mar 23, 2021
@bors
Copy link
Contributor

bors commented Mar 23, 2021

⌛ Testing commit b75894e with merge 2fd73fa...

@bors
Copy link
Contributor

bors commented Mar 23, 2021

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 2fd73fa to stable...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 23, 2021
@bors bors merged commit 2fd73fa into rust-lang:stable Mar 23, 2021
@rustbot rustbot added this to the 1.51.0 milestone Mar 23, 2021
wesleywiser added a commit to wesleywiser/rust that referenced this pull request Mar 24, 2021
This change was backed out in rust-lang#83412 so we should remove the reference
to it from the release notes.
@wesleywiser wesleywiser mentioned this pull request Mar 24, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 25, 2021
…-Simulacrum

Update RELEASES.md

This change was backed out in rust-lang#83412 so we should remove the reference
to it from the release notes.
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Apr 29, 2021
…in, for now.

We had already reverted the change on stable back in PR rust-lang#83412.

Since then, we've had some movement on issue rust-lang#83139, but not a 100% fix.

But also since then, we had bug reported, issue rust-lang#84667, that looks like outright
codegen breakage, rather than problems confined to debuginfo issues.

So we are reverting PR rust-lang#77885 on stable and beta. We'll reland PR rust-lang#77885 (or some
variant) switching back to an LLVM-dependent selection of out-of-line call vs
inline-asm, after these other issues have been resolved.
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 29, 2021
…ark-Simulacrum

Revert PR 77885 everywhere

Change to probe-stack=call (instead of inline-or-call) everywhere again, for now.

We had already reverted the change on stable back in PR rust-lang#83412.

Since then, we've had some movement on issue rust-lang#83139, but not a 100% fix.

But also since then, we had bug reported, issue rust-lang#84667, that looks like outright codegen breakage, rather than problems confined to debuginfo issues.    So we are reverting PR rust-lang#77885 on stable and beta. We'll reland PR rust-lang#77885 (or some    variant) switching back to an LLVM-dependent selection of out-of-line call vs    inline-asm, after these other issues have been resolved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants