Skip to content

Miri: make size/align_of_val work for dangling raw ptrs#80491

Merged
bors merged 2 commits intorust-lang:masterfrom
RalfJung:dangling-of-val
Dec 31, 2020
Merged

Miri: make size/align_of_val work for dangling raw ptrs#80491
bors merged 2 commits intorust-lang:masterfrom
RalfJung:dangling-of-val

Conversation

@RalfJung
Copy link
Member

@RalfJung RalfJung commented Dec 29, 2020

This is needed for #80365 (comment).

r? @oli-obk

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 29, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Dec 29, 2020

Is this testable within this repo or do we need miri for it?

r=me with a test if possible

@RalfJung
Copy link
Member Author

RalfJung commented Dec 30, 2020

Good point, I added a test.

@bors r=oli-obk

@bors
Copy link
Collaborator

bors commented Dec 30, 2020

📌 Commit f76bae9 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 Dec 30, 2020
@m-ou-se m-ou-se mentioned this pull request Dec 30, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 30, 2020
@bors
Auto merge of rust-lang#80530 - m-ou-se:rollup-zit69ko, r=m-ou-se
9775ffe 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#78934 (refactor: removing library/alloc/src/vec/mod.rs ignore-tidy-filelength)
 - rust-lang#79479 (Add `Iterator::intersperse`)
 - rust-lang#80128 (Edit rustc_ast::ast::FieldPat docs)
 - rust-lang#80424 (Don't give an error when creating a file for the first time)
 - rust-lang#80458 (Some Promotion Refactoring)
 - rust-lang#80488 (Do not create dangling &T in Weak<T>::drop)
 - rust-lang#80491 (Miri: make size/align_of_val work for dangling raw ptrs)
 - rust-lang#80495 (Rename kw::Invalid -> kw::Empty)
 - rust-lang#80513 (Add regression test for rust-lang#80062)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 067f1b7 into rust-lang:master Dec 31, 2020
@rustbot rustbot added this to the 1.51.0 milestone Dec 31, 2020
@RalfJung RalfJung deleted the dangling-of-val branch December 31, 2020 13:03
@matthieu-m
Copy link
Contributor

matthieu-m commented Jan 8, 2021

Is that a guarantee that we will want to make going forward?

I tried to create a *const dyn Debug from a *const DynamicallySized<[usize]> and failed (see playground) as apparently it's not possible to create a pointer to trait from a pointer to DST right now.

It's unclear whether this is a restriction that will stay, or not. If it doesn't, however, then size_of_val_raw may, on certain types, require accessing the data-pointer of the trait.

As such, it seems that if we wish to keep our options open regarding the conversion from DST to trait, then we should enforce that size_of_val_raw require a valid data-pointer.

@RalfJung
Copy link
Member Author

RalfJung commented Jan 8, 2021

Is that a guarantee that we will want to make going forward?

The guarantee is what is documented for align_of_val_raw_/size_of_val_raw. In particular, for sized types, the pointers may dangle. So what Miri did previously was definitely wrong.

I think Miri also implements the rest of those docs correctly, but if there's a bug, please let me know. Note the last clause in the documentation:

otherwise, it is conservatively not allowed to call this function.

Currently, this is impossible to reach (there are no other kinds of DST), but that is not a stable assumptions -- so if one has no clue what the unsized tail is, one has to use size_of_val still.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@oli-obk oli-obk

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.51.0

Development

Successfully merging this pull request may close these issues.

6 participants

@RalfJung @oli-obk @bors @matthieu-m @rust-highfive @rustbot