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

Report null fat raw pointers #918

Closed
nico-abram opened this issue Aug 24, 2019 · 5 comments · Fixed by rust-lang/rust#63880
Closed

Report null fat raw pointers #918

nico-abram opened this issue Aug 24, 2019 · 5 comments · Fixed by rust-lang/rust#63880
Labels
A-validation Area: This affects enforcing the validity invariant, and related UB checking C-support Category: Not necessarily a bug, but someone asking for support I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings)

Comments

@nico-abram
Copy link
Contributor

https://play.rust-lang.org/?version=stable&mode=release&edition=2018&gist=37f152716318de72dd678cb4b8906257
As far as I know, this example is ub. I doubt
detecting this is very important since either zeroed or transmute are probably the only way to do this, and it seems to be as insta ub as setting a reference to null (since the vtable ptr Is NonNull iirc) so i doubt any working code does it.

@RalfJung RalfJung added the A-validation Area: This affects enforcing the validity invariant, and related UB checking label Aug 24, 2019
@RalfJung
Copy link
Member

RalfJung commented Aug 24, 2019

Thanks for the report!

This is deliberately not warned against until rust-lang/unsafe-code-guidelines#166 is resolved. I think the likely conclusion of that discussion will be that wide raw pointers are like (usize, usize) have no non-NULL requirements or so -- so I don't want to cause tons of errors in existing code.

Being able to zero-initialize raw pointers is actually a key motivation for not making this UB, IMO.

@RalfJung
Copy link
Member

since the vtable ptr Is NonNull iirc

I don't think it is, do you have a source for this?

@nico-abram
Copy link
Contributor Author

nico-abram commented Aug 24, 2019

The example playground i linked crashes on release. Theres also this (somewhat old) comment rust-lang/rfcs#433 (comment)

The example compiles down to playground::main:
ud2

@RalfJung
Copy link
Member

The example playground i linked crashes on release.

Ouch. That seems like a bug to me, probably a bad Debug impl for raw pointers? Could you report this against rustc, Cc me?

Reopening until we figured out if this is a Miri or rustc bug. Definitely it shouldn't crash on rustc but work fine in Miri.

@RalfJung RalfJung reopened this Aug 24, 2019
@RalfJung RalfJung added the C-support Category: Not necessarily a bug, but someone asking for support label Aug 24, 2019
@nico-abram
Copy link
Contributor Author

The same thing happens without the debug impl: https://play.rust-lang.org/?version=stable&mode=release&edition=2018&gist=7c69493026add62256996d204e1278c0

Sure, will open issue in rust-lang/Rust.

@RalfJung RalfJung added the I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings) label Aug 25, 2019
Centril added a commit to Centril/rust that referenced this issue Aug 29, 2019
Validation: check raw wide pointer metadata

While I was at it, I also added a missing check for slices not to be too big.

r? @oli-obk
Fixes rust-lang/miri#918
Centril added a commit to Centril/rust that referenced this issue Aug 29, 2019
Validation: check raw wide pointer metadata

While I was at it, I also added a missing check for slices not to be too big.

r? @oli-obk
Fixes rust-lang/miri#918
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-validation Area: This affects enforcing the validity invariant, and related UB checking C-support Category: Not necessarily a bug, but someone asking for support I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants