-
Notifications
You must be signed in to change notification settings - Fork 56
ci: run miri + adjustments for miri #128
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
Conversation
9e479ad
to
e122ad5
Compare
ee2f968
to
bb84406
Compare
Could I ask you @nicholasbishop please for help? You do have more experience with miri than I have, I think. I don't quite understand why miri fails and if this is valid or how I should either fix or ignore it. Do you have some advice? You can find a failed run here: https://github.com/rust-osdev/multiboot2/actions/runs/4526183512/jobs/7971182214 |
I think the error in |
At this state, miri is somewhat useful. All tests that use the A possible solution might be to model the whole parsing logic differently:
I need to think, if this should be merged in this state. I do not like the need for the (feature incomplete) |
Hm. In #155 (comment) I could at least address some of the issues. |
I decided so as they only make things more complicated for miri. As we have mature builder and parser structs guaranteeing alignment, we do not have to create additional confusion. These align(8) markers do not have value-add here besides being informational. They are only useful if no crazy pointer magic is involved and Rust constructs these types "in the Rust-typical way".
This MR now introduces basic miri support. Most tests are skipped by miri. There are two major issues:
However, some things now are tracked by miri. For example if |
Closes #117