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

Validate more things #477

Merged
merged 2 commits into from
Oct 12, 2018
Merged

Validate more things #477

merged 2 commits into from
Oct 12, 2018

Conversation

RalfJung
Copy link
Member

By whitelisting one method that currently fails to validate (std::ptr::read), we can enable validation for plenty more things. Seems like a good deal!

Also, enable validation in compile-fail and finally unignore some of the tests there :)

"std::ptr::read",
];
for frame in ecx.stack().iter()
.rev().take(3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol, the last 3 stackframes!?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah well, I had to pick some number. I didn't want to manually whitelist all callees, but also didn't want to walk up the entire stack.

It's a hack to even have a whitelist, so... whatever?^^

@@ -0,0 +1,8 @@
fn main() {
assert!(std::char::from_u32(-1_i32 as u32).is_none());
let _ = match unsafe { std::mem::transmute::<i32, char>(-1) } { //~ ERROR encountered 4294967295, but expected something in the range 0..=1114111
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

char has a range? what's going on here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it does? It must be a valid unicode codepoint.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought char's value ranges had gaps and were generally totally insane. 1114111 seems a little low, too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has gaps. Remember we do first layout-based validation and then type-based validation. This value is so out of range it is already caught by the first.

But if that's too low, our layout code would be wrong and that would be BAD (TM).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The maximal value currently seems to be 0x10ffff. In decimal that's 1114111.

Maybe we should make validation print the range in hexadecimal...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah seems fine. Or we could print both. It's not really clear what is more readable...


let g: fn(*const i32) = unsafe { std::mem::transmute(f as fn(&i32)) };

g(0usize as *const i32) //~ ERROR encountered 0, but expected something greater or equal to 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sweet

@RalfJung
Copy link
Member Author

(Btw, this is a PR against the rustup branch, to become part of the other PR, because it all waits for rust-lang/rust#54955 to land.)

@oli-obk oli-obk merged commit 10244b5 into rust-lang:rustup Oct 12, 2018
@RalfJung RalfJung deleted the miri-validate branch November 8, 2018 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants