Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 2 commits into from
Dec 31, 2020
Merged

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

merged 2 commits into from
Dec 31, 2020

Conversation

RalfJung
Copy link
Member

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

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

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.

# for free to join this conversation on GitHub. Already have an account? # 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