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

Inconsistent @bitCast between packed struct and integer on big endian #9914

Closed
tgschultz opened this issue Oct 7, 2021 · 1 comment · Fixed by #16572
Closed

Inconsistent @bitCast between packed struct and integer on big endian #9914

tgschultz opened this issue Oct 7, 2021 · 1 comment · Fixed by #16572
Labels
bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code. stage1 The process of building from source via WebAssembly and the C backend.
Milestone

Comments

@tgschultz
Copy link
Contributor

tgschultz commented Oct 7, 2021

Zig Version

0.9.0-dev.1324+598db831f

Steps to Reproduce

On big endian hardware:

const S = packed struct {
    .one: u6,
    .two: u1,
};

const s = S{ .one = 0b110101, .two = 0b1};

const u = @bitCast(u7, s);
const s2 = @bitCast(S, u);

try testing.expectEqual(s.one, s2.one); //will fail
try testing.expectEqual(s.two, s2.two);

Expected Behavior

I believe the correct behavior for big endian is:

Given a packed struct with one field as defined below:

const S = packed struct {
    .one: u7,
};

its representation in memory should have all 7 bits at the high end of the containing byte.

Packed structs containing more than one field require no changes.

@bitCast from a packed struct to an int requires no changes.

@bitCast from an int to a packed struct should place the bits at the high end. Thus:

const s = S{ .one = 0b110101, .two = 0b1};
// s byte = 0b11010110

const u = @bitCast(u7, s);
// u byte = 0b01101011

const s2 = @bitCast(S, u);
// s2 byte = 0b11010110
// s2 = .{.one = 0b110101, .two = 0b1}

Actual Behavior

On big endian hardware, given a packed struct as defined below:

const S = packed struct {
    .one: u7,
};

its representation in memory will be exactly identical that of a u7 integer of the same value. That is to say, the low 7 bits of the byte occupied by our S instance contain the value. However, when the struct has more than one field:

const S = packed struct {
    .one: u6,
    .two: u1,
};

its representation in memory now occupies the high 7 bits of the byte. When @bitCasting this struct to a u7, the value is converted such that the high 7 bits of the struct byte now occupy the low 7 bits of the integer byte. However, when @bitCasting the u7 back to S the high 7 bits are kept and the low 1 is discarded.

examples with binary:

const s = S{ .one = 0b110101, .two = 0b1};
// s byte = 0b11010110

const u = @bitCast(u7, s);
// u byte = 0b01101011

const s2 = @bitCast(S, u);
// s2 byte = 0b01101010
// s2 = .{.one = 0b011010, .two = 0b1}
@tgschultz tgschultz added the bug Observed behavior contradicts documented or intended behavior label Oct 7, 2021
@tgschultz
Copy link
Contributor Author

After further experimentation, I am not convinced my analysis above holds for all cases. Specifically, it would seem that @bitCast from a packed struct to an int does not always produce a sensical result.

@andrewrk andrewrk added stage1 The process of building from source via WebAssembly and the C backend. miscompilation The compiler reports success but produces semantically incorrect code. labels Nov 20, 2021
@andrewrk andrewrk added this to the 0.10.0 milestone Nov 20, 2021
@andrewrk andrewrk modified the milestones: 0.14.0, 0.11.0 Jul 22, 2023
andrewrk added a commit that referenced this issue Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code. stage1 The process of building from source via WebAssembly and the C backend.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants