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

False Positive - incorrect alignment #1925

Closed
elichai opened this issue Nov 27, 2021 · 5 comments · Fixed by rust-lang/rust#91303
Closed

False Positive - incorrect alignment #1925

elichai opened this issue Nov 27, 2021 · 5 comments · Fixed by rust-lang/rust#91303

Comments

@elichai
Copy link
Contributor

elichai commented Nov 27, 2021

Miri complained about alignment bugs in blake2_simd and I managed to make this minimum reproducible example:

use core::mem::size_of;
fn main() {
    let mut a = Params::new();
    a.key_block = [0; BLOCKBYTES];
}

#[derive(Clone)]
pub struct Params {
    hash_length: u8,
    key_length: u8,
    key_block: [u8; BLOCKBYTES],
    max_leaf_length: u32,
}

pub const OUTBYTES: usize = 8 * size_of::<u64>();
pub const KEYBYTES: usize = 8 * size_of::<u64>();
pub const BLOCKBYTES: usize = 16 * size_of::<u64>();

impl Params {
    pub fn new() -> Self {
        Self {
            hash_length: OUTBYTES as u8,
            key_length: 0,
            key_block: [0; BLOCKBYTES],
            max_leaf_length: 0,
        }
    }
}

Miri output:

error: Undefined Behavior: accessing memory with alignment 1, but alignment 2 is required
 --> src/main.rs:4:5
  |
4 |     a.key_block = [0; BLOCKBYTES];
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ accessing memory with alignment 1, but alignment 2 is required
  |
  = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
  = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
          
  = note: inside `main` at src/main.rs:4:5
  = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
  = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:123:18
  = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:145:18
  = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
  = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
  = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
  = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
  = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:128:48
  = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
  = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
  = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
  = note: inside `std::rt::lang_start_internal` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:128:20
  = note: inside `std::rt::lang_start::<()>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:144:17

playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2e1749d0115ff35f9c763312fdc449f8

It's obvious that there's no bug in the rust code itself, so it's either in miri or in the MIR lowering code of current nightly?

@bjorn3
Copy link
Member

bjorn3 commented Nov 27, 2021

This looks a lot like #1919 except that this doesn't use async. Maybe the cause of that other issue is unrelated to async?

@camelid
Copy link
Member

camelid commented Nov 27, 2021

Hmm, given that two people reported similar issues at similar times ... maybe this is a regression?

@RalfJung
Copy link
Member

Well, having an example without async is certainly going to be extremely helpful in figuring that out, so thanks a ton for minimizing this, @elichai!

@RalfJung
Copy link
Member

I am pretty sure I know what is causing this... yes it is a regression but it happened more than half a year ago. It just seemingly needs very special circumstances to be triggered.

The regression is on this line introduced by rust-lang/rust#85376. I think I will have a patch shortly.

@RalfJung
Copy link
Member

rust-lang/rust#91303 should fix this.

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 a pull request may close this issue.

4 participants