Skip to content

x86 backend miscompilation w/ packed structs and bitwise operation #20581

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

Closed
MasonRemaley opened this issue Jul 10, 2024 · 2 comments · Fixed by #23256
Closed

x86 backend miscompilation w/ packed structs and bitwise operation #20581

MasonRemaley opened this issue Jul 10, 2024 · 2 comments · Fixed by #23256
Labels
arch-x86_64 64-bit x86 backend-self-hosted bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code.
Milestone

Comments

@MasonRemaley
Copy link
Contributor

MasonRemaley commented Jul 10, 2024

Zig Version

0.14.0-dev.616+ab4eeb770

Steps to Reproduce and Observed Behavior

I know the x86 backend isn't passing all tests yet anyway, so I'm not sure if documenting these sorts of bugs is useful or not--if not let me know! I'm just very excited about how fast my project compiles under this backend haha.

Here's a reproduction of the problem:

const std = @import("std");
pub fn main() void {
    // Create a packed struct
    const Packed = packed struct {
        should_be_true: bool,
    };

    // Create a variable and then set it to zero. If we set it immediately to zero the bug
    // doesn't occur, presumably because the bitwise math can then be trivially optimized out.
    var zero: u32 = 5;
    zero -= 5;
    std.debug.assert(zero == 0); // <-- All good

    // This bitwise math results in true as expected
    const should_be_true = (zero & 1) != 1;
    std.debug.assert(should_be_true); // <-- All good

    // Store the same bitwise math in this packed struct. Now the value is true instead of
    // false.
    const result: Packed = .{ .should_be_true = (zero & 1) != 1 };
    std.debug.assert(result.should_be_true); // <-- This fails
}

Expected Behavior

I expected the result of the bitwise operation to be the same in both cases.

@MasonRemaley MasonRemaley added the bug Observed behavior contradicts documented or intended behavior label Jul 10, 2024
@MasonRemaley MasonRemaley changed the title x86 backend miscompilation x86 backend miscompilation w/ packed structs and bitwise operation Jul 10, 2024
@mlugg mlugg added arch-x86_64 64-bit x86 miscompilation The compiler reports success but produces semantically incorrect code. backend-self-hosted labels Jul 11, 2024
@mlugg mlugg added this to the 0.14.0 milestone Jul 11, 2024
@mlugg
Copy link
Member

mlugg commented Jul 11, 2024

Yeah, we're at a point where it's worth reporting bugs in x86_64 self-hosted; thanks!

@jacobly0 jacobly0 modified the milestones: 0.14.0, 0.14.1, 0.15.0 Mar 1, 2025
xtexx added a commit to xtexx/zig that referenced this issue Mar 15, 2025
Fixes ziglang#20113 and ziglang#20581.

First, postpone operand resolve from airStore into packedStore/store.
Then, spill EFLAGS before AND in packedStore as AND instructions
clobber EFLAGS, which may break EFLAGS operands.

Bug: ziglang#20113
Bug: ziglang#20581
Signed-off-by: Bingwu Zhang <[email protected]>
@xtexx
Copy link
Contributor

xtexx commented Mar 15, 2025

This seems to be the same bug as #20113. It looks like that #23256 also fixed this.

@jacobly0 jacobly0 modified the milestones: 0.15.0, 0.14.1 Mar 16, 2025
xtexx added a commit to xtexx/zig that referenced this issue Mar 16, 2025
Fixes ziglang#20113 and ziglang#20581.

AND instructions in packedStore clobbers EFLAGS.

Bug: ziglang#20113
Bug: ziglang#20581
Signed-off-by: Bingwu Zhang <[email protected]>
xtexx added a commit to xtexx/zig that referenced this issue Mar 22, 2025
Fixes ziglang#20113 and ziglang#20581.

AND instructions in packedStore clobbers EFLAGS.

Bug: ziglang#20113
Bug: ziglang#20581
Signed-off-by: Bingwu Zhang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-x86_64 64-bit x86 backend-self-hosted bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants