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

interpret/allocation: fix aliasing issue in interpreter and refactor getters a bit #122537

Merged
merged 1 commit into from
Mar 22, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Mar 15, 2024

That new raw getter will be needed to let Miri pass pointers to natively executed FFI code ("extern-so" mode).

While doing that I realized our get_bytes_mut are named less scary than get_bytes_unchecked so I rectified that. Also I realized mem_copy_repeatedly would break if we called it for multiple overlapping copies so I made sure this does not happen.

And I realized that we are actually violating Stacked Borrows in the interpreter.^^ That was introduced in #87777.

r? @oli-obk

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 15, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 15, 2024

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@RalfJung RalfJung force-pushed the interpret-allocation branch from d42d826 to e36e63e Compare March 15, 2024 11:26
@rustbot
Copy link
Collaborator

rustbot commented Mar 15, 2024

The Miri subtree was changed

cc @rust-lang/miri

@RalfJung RalfJung force-pushed the interpret-allocation branch 2 times, most recently from b65ca27 to 21b8f06 Compare March 15, 2024 11:30
@RalfJung RalfJung changed the title interpret/allocation: rename mutating functions to be more scary; add a new raw bytes getter interpret/allocation: fix aliasing issue in interpreter and refactor getters a bit Mar 15, 2024
@RalfJung RalfJung force-pushed the interpret-allocation branch from 21b8f06 to d4643de Compare March 15, 2024 13:29
@rust-log-analyzer

This comment has been minimized.

…getters a bit

- rename mutating functions to be more scary
- add a new raw bytes getter
@RalfJung RalfJung force-pushed the interpret-allocation branch from d4643de to 7be47b2 Compare March 15, 2024 14:58
@oli-obk
Copy link
Contributor

oli-obk commented Mar 22, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 22, 2024

📌 Commit 7be47b2 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 Mar 22, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 22, 2024
…li-obk

interpret/allocation: fix aliasing issue in interpreter and refactor getters a bit

That new raw getter will be needed to let Miri pass pointers to natively executed FFI code ("extern-so" mode).

While doing that I realized our get_bytes_mut are named less scary than get_bytes_unchecked so I rectified that. Also I realized `mem_copy_repeatedly` would break if we called it for multiple overlapping copies so I made sure this does not happen.

And I realized that we are actually [violating Stacked Borrows in the interpreter](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/I.20think.20Miri.20violates.20Stacked.20Borrows.20.F0.9F.99.88).^^ That was introduced in rust-lang#87777.

r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 22, 2024
…li-obk

interpret/allocation: fix aliasing issue in interpreter and refactor getters a bit

That new raw getter will be needed to let Miri pass pointers to natively executed FFI code ("extern-so" mode).

While doing that I realized our get_bytes_mut are named less scary than get_bytes_unchecked so I rectified that. Also I realized `mem_copy_repeatedly` would break if we called it for multiple overlapping copies so I made sure this does not happen.

And I realized that we are actually [violating Stacked Borrows in the interpreter](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/I.20think.20Miri.20violates.20Stacked.20Borrows.20.F0.9F.99.88).^^ That was introduced in rust-lang#87777.

r? ``@oli-obk``
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#121619 (Experimental feature postfix match)
 - rust-lang#122370 (Gracefully handle `AnonConst` in `diagnostic_hir_wf_check()`)
 - rust-lang#122537 (interpret/allocation: fix aliasing issue in interpreter and refactor getters a bit)
 - rust-lang#122542 (coverage: Clean up marker statements that aren't needed later)
 - rust-lang#122800 (Add `NonNull::<[T]>::is_empty`.)
 - rust-lang#122820 (Stop using `<DefId as Ord>` in various diagnostic situations)
 - rust-lang#122847 (Suggest `RUST_MIN_STACK` workaround on overflow)
 - rust-lang#122855 (Fix Itanium mangling usizes)
 - rust-lang#122863 (add more ice tests )

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 84e55be into rust-lang:master Mar 22, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 22, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2024
Rollup merge of rust-lang#122537 - RalfJung:interpret-allocation, r=oli-obk

interpret/allocation: fix aliasing issue in interpreter and refactor getters a bit

That new raw getter will be needed to let Miri pass pointers to natively executed FFI code ("extern-so" mode).

While doing that I realized our get_bytes_mut are named less scary than get_bytes_unchecked so I rectified that. Also I realized `mem_copy_repeatedly` would break if we called it for multiple overlapping copies so I made sure this does not happen.

And I realized that we are actually [violating Stacked Borrows in the interpreter](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/I.20think.20Miri.20violates.20Stacked.20Borrows.20.F0.9F.99.88).^^ That was introduced in rust-lang#87777.

r? ```@oli-obk```
@RalfJung RalfJung deleted the interpret-allocation branch March 23, 2024 18:13
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 4, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#121619 (Experimental feature postfix match)
 - rust-lang#122370 (Gracefully handle `AnonConst` in `diagnostic_hir_wf_check()`)
 - rust-lang#122537 (interpret/allocation: fix aliasing issue in interpreter and refactor getters a bit)
 - rust-lang#122542 (coverage: Clean up marker statements that aren't needed later)
 - rust-lang#122800 (Add `NonNull::<[T]>::is_empty`.)
 - rust-lang#122820 (Stop using `<DefId as Ord>` in various diagnostic situations)
 - rust-lang#122847 (Suggest `RUST_MIN_STACK` workaround on overflow)
 - rust-lang#122855 (Fix Itanium mangling usizes)
 - rust-lang#122863 (add more ice tests )

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants