CTFE: throw unsupported error when partially overwriting a pointer by RalfJung · Pull Request #87248 · rust-lang/rust

RalfJung · 2021-07-18T09:19:00Z

Currently, during CTFE, when a write to memory would overwrite parts of a pointer, we make the remaining parts of that pointer "uninitialized". This is probably not what users expect, so if this ever happens they will be quite confused about why some of the data just vanishes for seemingly no good reason.
So I propose we change this to abort CTFE when that happens, to at last avoid silently doing the wrong thing.
Cc #87184

Our CTFE test suite still seems to pass. However, we should probably crater this, and I want to do some tests with Miri as well.

rust-highfive · 2021-07-18T09:19:03Z

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 · 2021-07-18T09:19:04Z

r? @michaelwoerister

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

RalfJung · 2021-07-18T09:19:10Z

r? @oli-obk

RalfJung · 2021-07-18T10:14:43Z

Ah turns out Miri really needs this:

error: unsupported operation: unable to overwrite parts of a pointer in memory at alloc1390
   --> /home/r/src/rust/rustc/library/std/src/panicking.rs:401:13
    |
401 |             data.r = ManuallyDrop::new(f());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unable to overwrite parts of a pointer in memory at alloc1390
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support

    = note: inside `std::panicking::try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/r/src/rust/rustc/library/std/src/panicking.rs:401:13
    = note: inside `std::panicking::try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/r/src/rust/rustc/library/std/src/panicking.rs:365:19

RalfJung · 2021-07-18T10:49:20Z

@bors try

bors · 2021-07-18T10:49:29Z

⌛ Trying commit 4725c8543fc218bcafef8c60a9f397019bc5e1aa with merge 3e59a8c2f14152ccc92a6c079f592dc2dc6fa93f...

bors · 2021-07-18T11:39:16Z

☀️ Try build successful - checks-actions
Build commit: 3e59a8c2f14152ccc92a6c079f592dc2dc6fa93f (3e59a8c2f14152ccc92a6c079f592dc2dc6fa93f)

RalfJung · 2021-07-19T09:03:31Z

@craterbot check

craterbot · 2021-07-19T09:03:39Z

👌 Experiment pr-87248 created and queued.
🤖 Automatically detected try build 3e59a8c2f14152ccc92a6c079f592dc2dc6fa93f
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2021-07-25T03:17:01Z

🚧 Experiment pr-87248 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2021-07-30T00:51:07Z

🎉 Experiment pr-87248 is completed!
📊 10 regressed and 6 fixed (173576 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

oli-obk · 2021-07-30T11:55:59Z

All of the regressions are spurious (some python network error and some file not found)

RalfJung · 2021-07-30T18:44:36Z

Nice, that's what I hoped for. :)

So how do we proceed here -- what are your thoughts on this PR, @oli-obk?
(I sure need to add a test before landing.)

oli-obk

I like how this allows us to experiment on miri first!

lgtm modulo some tests

Co-authored-by: Oli Scherer <[email protected]>

RalfJung · 2021-07-31T09:53:08Z

Added a test, please have a look. :)

oli-obk · 2021-08-02T12:00:25Z

@bors r+

bors · 2021-08-02T12:00:27Z

📌 Commit 2a9b44d has been approved by oli-obk

bors · 2021-08-02T13:31:08Z

⌛ Testing commit 2a9b44d with merge 3227e35...

bors · 2021-08-02T15:54:04Z

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

rust-highfive · 2021-08-02T15:54:31Z

📣 Toolstate changed by #87248!

Tested on commit 3227e35.
Direct link to PR: #87248

💔 miri on windows: test-pass → build-fail (cc @eddyb @RalfJung @oli-obk).
💔 miri on linux: test-pass → build-fail (cc @eddyb @RalfJung @oli-obk).

@eddyb

Tested on commit rust-lang/rust@3227e35. Direct link to PR: <rust-lang/rust#87248> 💔 miri on windows: test-pass → build-fail (cc @eddyb @RalfJung @oli-obk). 💔 miri on linux: test-pass → build-fail (cc @eddyb @RalfJung @oli-obk).

adjust for ERR_ON_PARTIAL_PTR_OVERWRITE The Miri side of rust-lang/rust#87248

rust-highfive assigned michaelwoerister Jul 18, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 18, 2021

rust-highfive assigned oli-obk and unassigned michaelwoerister Jul 18, 2021

RalfJung force-pushed the ctfe-partial-overwrite branch from 3b4e57e to 0624ccb Compare July 18, 2021 10:04

RalfJung force-pushed the ctfe-partial-overwrite branch from 0624ccb to 4725c85 Compare July 18, 2021 10:41

This comment has been minimized.

Sign in to view

RalfJung closed this Jul 18, 2021

RalfJung reopened this Jul 18, 2021

RalfJung mentioned this pull request Jul 18, 2021

adjust for ERR_ON_PARTIAL_PTR_OVERWRITE rust-lang/miri#1854

Merged

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 19, 2021

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jul 30, 2021

oli-obk reviewed Jul 30, 2021

View reviewed changes

Comment thread compiler/rustc_middle/src/mir/interpret/error.rs Outdated

CTFE: throw unsupported error when partially overwriting a pointer

14de6ec

typo

b7b5091

Co-authored-by: Oli Scherer <[email protected]>

RalfJung force-pushed the ctfe-partial-overwrite branch from 6efe7fe to b7b5091 Compare July 31, 2021 09:30

RalfJung mentioned this pull request Jul 31, 2021

const-eval: loading or overwriting parts of a pointer is not supported #87184

Closed

add a test

2a9b44d

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 Aug 2, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 2, 2021

bors merged commit 3227e35 into rust-lang:master Aug 2, 2021

rustbot added this to the 1.56.0 milestone Aug 2, 2021

rust-highfive mentioned this pull request Aug 2, 2021

miri no longer builds after rust-lang/rust#87248 #87703

Closed

RalfJung deleted the ctfe-partial-overwrite branch August 2, 2021 15:59

bors added a commit to rust-lang/miri that referenced this pull request Aug 2, 2021

Auto merge of #1854 - RalfJung:partial-ptr-overwrite, r=RalfJung

99ec9c1

adjust for ERR_ON_PARTIAL_PTR_OVERWRITE The Miri side of rust-lang/rust#87248

Uh oh!

Conversation

RalfJung commented Jul 18, 2021

Uh oh!

rust-highfive commented Jul 18, 2021

Uh oh!

rust-highfive commented Jul 18, 2021

Uh oh!

RalfJung commented Jul 18, 2021

Uh oh!

RalfJung commented Jul 18, 2021

Uh oh!

This comment has been minimized.

RalfJung commented Jul 18, 2021

Uh oh!

bors commented Jul 18, 2021

Uh oh!

bors commented Jul 18, 2021

Uh oh!

RalfJung commented Jul 19, 2021

Uh oh!

craterbot commented Jul 19, 2021

Uh oh!

craterbot commented Jul 25, 2021

Uh oh!

craterbot commented Jul 30, 2021

Uh oh!

oli-obk commented Jul 30, 2021

Uh oh!

RalfJung commented Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oli-obk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RalfJung commented Jul 31, 2021

Uh oh!

oli-obk commented Aug 2, 2021

Uh oh!

bors commented Aug 2, 2021

Uh oh!

bors commented Aug 2, 2021

Uh oh!

bors commented Aug 2, 2021

Uh oh!

rust-highfive commented Aug 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

RalfJung commented Jul 30, 2021 •

edited

Loading