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

Add option to disable alignment check #1332

Merged
merged 5 commits into from
Apr 14, 2020

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Apr 13, 2020

Requires rust-lang/rust#71101
Fixes #1326

@RalfJung RalfJung added the S-blocked-on-rust Status: Blocked on landing a Rust PR label Apr 13, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 13, 2020
…static-morse

Miri: let machine hook dynamically decide about alignment checks

This is needed for rust-lang/miri#1332.
@thomcc
Copy link
Member

thomcc commented Apr 14, 2020

Will miri still report actual loads from unaligned addresses?

Either way this is great -- it's been frustrating to disable tests for miri because they were too thorough and checked that every possible alignment and size combination didn't result in a misaligned load or OOB read.

@RalfJung
Copy link
Member Author

Will miri still report actual loads from unaligned addresses?

That is the exact check this disables. I am confused by your question.

Either way this is great -- it's been frustrating to disable tests for miri because they were too thorough and checked that every possible alignment and size combination didn't result in a misaligned load or OOB read.

Except for #1074, I do not know of a case where Miri is too thorough. Alignment errors otherwise are UB and indicate a bug in the program.

@thomcc
Copy link
Member

thomcc commented Apr 14, 2020

Except for #1074, I do not know of a case where Miri is too thorough. Alignment errors otherwise are UB and indicate a bug in the program.

I believe that's the case I mean. Basically I have code that manually aligns a pointer using pointer arithmetic. I test this with a very large number of possible input alignments and lengths to ensure it's sane, but miri assumes that any manually-aligned pointer is still invalid. As a result, I can't run my exhaustive tests under miri (and only some of the other less thorough ones).

I had thought this was for disabling miri complaints about that case -- I guess it's about all cases wrt alignment. That doesn't bother me that much, since I generally have have a wrapper around ptr::read which asserts the alignment is correct for stuff like this, but yeah, I must have misunderstood.

@RalfJung
Copy link
Member Author

I had thought this was for disabling miri complaints about that case

No. That would be resolving #1332. This does not resolve that issue, but it offers a work-around (at the expense of possibly missing bugs).

@RalfJung
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented Apr 14, 2020

📌 Commit 8e73db6 has been approved by RalfJung

bors added a commit that referenced this pull request Apr 14, 2020
@bors
Copy link
Contributor

bors commented Apr 14, 2020

⌛ Testing commit 8e73db6 with merge e7ab430...

@RalfJung
Copy link
Member Author

@bors retry r+

@bors
Copy link
Contributor

bors commented Apr 14, 2020

📌 Commit f4a1544 has been approved by RalfJung

@bors
Copy link
Contributor

bors commented Apr 14, 2020

⌛ Testing commit f4a1544 with merge 0371590...

@bors
Copy link
Contributor

bors commented Apr 14, 2020

☀️ Test successful - checks-travis, status-appveyor
Approved by: RalfJung
Pushing 0371590 to master...

@bors bors merged commit 0371590 into rust-lang:master Apr 14, 2020
@RalfJung RalfJung deleted the disable-alignment-check branch April 14, 2020 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked-on-rust Status: Blocked on landing a Rust PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow ignoring alignment violations
3 participants