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

Fix tests crashing with miri due to misalignment. #14

Merged
merged 1 commit into from
Sep 30, 2022
Merged

Fix tests crashing with miri due to misalignment. #14

merged 1 commit into from
Sep 30, 2022

Conversation

yotamofek
Copy link
Contributor

Currently, three tests are failing with the latest nightly version of miri: test_eq, test_ne, test_ord.

Example output:

➜  zerocopy git:(main) ✗ cargo miri test -- test_ord
Preparing a sysroot for Miri (target: x86_64-unknown-linux-gnu)... done
    Finished test [unoptimized + debuginfo] target(s) in 0.01s
     Running unittests src/lib.rs (target/miri/x86_64-unknown-linux-gnu/debug/deps/zerocopy-3f8b570858445ddd)

running 1 test
test tests::test_ord ... FAILED

failures:

---- tests::test_ord stdout ----
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/lib.rs:2945:60
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    tests::test_ord

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 31 filtered out

error: test failed, to rerun pass `--lib`
➜  zerocopy git:(main) ✗ 

This happens because of these lines:

let buf1 = [0u8; 8];
let lv1 = LayoutVerified::<_, u64>::new(&buf1[..]).unwrap();

Since a [u8; N] array is unaligned, its stack addr isn't necessarily aligned to 8 (as u64 requires). I'm not sure where the exact semantics are documented, but I think miri randomized the layout of data to catch these types of errors? Not sure, but it seems that the tests are passing when run not under miri "by accident", i.e. they rely on the fact that the memory happens to be aligned correctly.

My fix simply stores u64s on the stack, instead of [u8; 8]s, and uses AsBytes::as_bytes() to use them as a buffer for the creation of the LayoutVerifieds. Since the stack memory is now guaranteed to be properly aligned, this seems to appease miri.

@google-cla
Copy link

google-cla bot commented Sep 30, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@joshlf
Copy link
Member

joshlf commented Sep 30, 2022

Thanks for the catch! Small style nit: can you remove the trailing period from your commit message? I know we don't have a style guide yet (#16); hopefully we'll have one soon.

@yotamofek
Copy link
Contributor Author

Done :)

@joshlf joshlf merged commit f9b159b into google:main Sep 30, 2022
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