Skip to content

fix deallocation of immutable allocations #85599

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

Merged
merged 5 commits into from
May 23, 2021
Merged

Conversation

RalfJung
Copy link
Member

As part of rust-lang/miri#1814, I realized that we currently allow deallocating immutable allocations. This PR fixes that, and also adds some new APIs that are required to still support the existing Miri backtrace support.

r? @oli-obk

@rust-highfive
Copy link
Contributor

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 23, 2021
@RalfJung RalfJung changed the title Immut allocs fix deallocation of immutable allocations May 23, 2021
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@@ -175,6 +176,7 @@ pub(crate) fn codegen_const_value<'tcx>(
let mut alloc = Allocation::from_bytes(
std::iter::repeat(0).take(size.bytes_usize()).collect::<Vec<u8>>(),
align,
Mutability::Not,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bjorn3 on the last PR I developed, ./x.py check compiler/rustc checked the cranelift files -- but now that does not seem to be the case any more, I only got the cranelift build errors from PR CI.

Copy link
Member

@bjorn3 bjorn3 May 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you previously used ./x.py check compiler/rustc? That explicitly says that compiler/rustc and its dependencies should be checked, but nothing else. cg_clif is not a dependency. ./x.py check compiler/rustc compiler/rustc_codegen_cranelift should work though. I think compiler/rustc is even unnecessary as it is marked as dep of cg_clif.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I am pretty sure -- I had that set in my vscode config.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and I think I saw the errors in vscode... though there is a slim chance that I am hallucinating this and actually did ./x.py check on the console and copy-pasted the paths into vscode. I think I stopped doing that a while ago but maybe I misremember the timeline here.)

Co-authored-by: bjorn3 <[email protected]>
@oli-obk
Copy link
Contributor

oli-obk commented May 23, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented May 23, 2021

📌 Commit f9b36b4 has been approved by oli-obk

@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 May 23, 2021
@bors
Copy link
Collaborator

bors commented May 23, 2021

⌛ Testing commit f9b36b4 with merge 0f8cd43...

@bors
Copy link
Collaborator

bors commented May 23, 2021

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 23, 2021
@bors bors merged commit 0f8cd43 into rust-lang:master May 23, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 23, 2021
bors added a commit to rust-lang/miri that referenced this pull request May 23, 2021
avoid unnecessary RefCell calls

Blocked on rust-lang/rust#85599
@RalfJung RalfJung deleted the immut-allocs branch May 23, 2021 17:57
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