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

Invalid expected alignment for fixed-size array #1958

Closed
kvark opened this issue Jan 5, 2022 · 5 comments
Closed

Invalid expected alignment for fixed-size array #1958

kvark opened this issue Jan 5, 2022 · 5 comments

Comments

@kvark
Copy link

kvark commented Jan 5, 2022

This is from miri testing naga, aka:

git clone https://github.com/gfx-rs/naga
cd naga
cargo +nightly miri test

Have miri complaining about this code:

/// Additional information about the glsl shader
///
/// Stores additional information about the glsl shader which might not be
/// stored in the shader [`Module`](Module).
#[derive(Debug)]
pub struct ShaderMetadata {
    /// The glsl version specified in the shader trought the use of the
    /// `#version` preprocessor directive.
    pub version: u16,
    /// The glsl profile specified in the shader trought the use of the
    /// `#version` preprocessor directive.
    pub profile: Profile,
    /// The shader stage in the pipeline, passed to the [`parse`](Parser::parse)
    /// method via the [`Options`](Options) struct.
    pub stage: ShaderStage,

    /// The workgroup size for compute shaders, defaults to `[1; 3]` for
    /// compute shaders and `[0; 3]` for non compute shaders.
    pub workgroup_size: [u32; 3],
    /// Wether or not early fragment tests where requested by the shader,
    /// defaults to `false`.
    pub early_fragment_tests: bool,

    /// The shader can request extensions via the
    /// `#extension` preprocessor directive, in the directive a behavior
    /// parameter is used to control whether the extension should be disabled,
    /// warn on usage, enabled if possible or required.
    ///
    /// This field only stores extensions which were required or requested to
    /// be enabled if possible and they are supported.
    pub extensions: FastHashSet<String>,
}

impl ShaderMetadata {
    fn reset(&mut self, stage: ShaderStage) {
        self.version = 0;
        self.profile = Profile::Core;
        self.stage = stage;
        self.workgroup_size = [if stage == ShaderStage::Compute { 1 } else { 0 }; 3];
        self.early_fragment_tests = false;
        self.extensions.clear();
    }
}
test front::glsl::parser_tests::constants ... error: Undefined Behavior: accessing memory with alignment 4, but alignment 8 is required
   --> src/front/glsl/mod.rs:102:9
    |
102 |         self.workgroup_size = [if stage == ShaderStage::Compute { 1 } else { 0 }; 3];
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ accessing memory with alignment 4, but alignment 8 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

First problem - this is just safe Rust code, there is no room for UB.
Second problem - it says the required alignment of 8 is required, while I'd expect 4 to be required.

@RalfJung
Copy link
Member

RalfJung commented Jan 5, 2022

We recently fixed a bug wrt alignment checks during array initialization, see #1925, #1919. This looks like a duplicate. Are you using the very latest Miri? The bug should be fixed there.

@kvark
Copy link
Author

kvark commented Jan 6, 2022

I used the latest as of today - from cargo 1.58.0-nightly (2e2a16e98 2021-11-08)

@RalfJung
Copy link
Member

RalfJung commented Jan 6, 2022

That's almost 2 months old -- that's definitely not the latest version.

@kvark
Copy link
Author

kvark commented Jan 6, 2022

Damn you are right, sorry about the noise!
Tested on 1.59.0-nightly (f1ce0e6a0 2022-01-05),and it's all good.

@kvark kvark closed this as completed Jan 6, 2022
@RalfJung
Copy link
Member

RalfJung commented Jan 6, 2022

No worries, I'm glad it's working now :)

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

No branches or pull requests

2 participants