Skip to content

Nested property access of packed struct loads wrong value #3091

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
evangrayk opened this issue Aug 18, 2019 · 2 comments · Fixed by #12637
Closed

Nested property access of packed struct loads wrong value #3091

evangrayk opened this issue Aug 18, 2019 · 2 comments · Fixed by #12637
Labels
bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code.
Milestone

Comments

@evangrayk
Copy link
Contributor

Accessing nested properties of packed structs seems to silently load from the wrong offset within the struct, without a compile error or runtime crash.

const std = @import("std");
const assert = std.debug.assert;
const warn = std.debug.warn;

const Foo = packed struct {
    v: Vec3,
};
const Vec3 = packed struct {
    x: f32,
    y: f32,
    z: f32,
};

test "packed struct access bug" {
    var foo = Foo{
        .v = Vec3{ .x = 1, .y = 2, .z = 3 },
    };
    var v = foo.v;

    warn("{d}\n", v.x); // 1
    warn("{d}\n", v.y); // 2
    warn("{d}\n", v.z); // 3

    warn("{d}\n", foo.v.x); // 1
    warn("{d}\n", foo.v.y); // 1
    warn("{d}\n", foo.v.z); // 1

    assert(v.y == 2);
    assert(foo.v.y == 2);
}

This only fails if Foo and Vec3 are packed, and it seems to be sensitive to the size/contents/alignment of the structs: for a Vec2 or Vec4, this works fine.

This is likely related to #2731 or #2784, but the issue here is incorrect behavior at runtime.

@andrewrk andrewrk added this to the 0.5.0 milestone Aug 19, 2019
@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Aug 19, 2019
@andrewrk andrewrk added the miscompilation The compiler reports success but produces semantically incorrect code. label Sep 25, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 30, 2019
@gustavolsson
Copy link
Contributor

gustavolsson commented Oct 27, 2019

Heh, I discovered a similar bug when trying to nest my own packed Vec3 struct into another packed struct. A difference is that I only see the problem if the var is set via a pointer. Nesting a packed Vec2 works as @evangrayk says.

Another example:

const std = @import("std");

const Vec2 = packed struct {
    x: f32,
    y: f32,
};

const NestedVec2 = packed struct {
    nested: Vec2,
};

test "packed vec2 set" {
    const v = Vec2{
        .x = 1.0,
        .y = 2.0,
    };

    std.testing.expectEqual(v.x, 1.0);
    std.testing.expectEqual(v.y, 2.0);
}

test "packed vec2 ptr set" {
    const v = Vec2{
        .x = 1.0,
        .y = 2.0,
    };

    var o: Vec2 = undefined;
    const o_ptr: *Vec2 = &o;
    o_ptr.* = v;

    std.testing.expectEqual(o.x, 1.0);
    std.testing.expectEqual(o.y, 2.0);
}

test "nested packed vec2 set" {
    const a = NestedVec2{
        .nested = Vec2{
            .x = 1.0,
            .y = 2.0,
        },
    };

    std.testing.expectEqual(a.nested.x, 1.0);
    std.testing.expectEqual(a.nested.y, 2.0);
}

test "nested packed vec2 ptr set" {
    const a = NestedVec2{
        .nested = Vec2{
            .x = 1.0,
            .y = 2.0,
        },
    };

    var o: NestedVec2 = undefined;
    const o_ptr: *NestedVec2 = &o;
    o_ptr.* = a;

    std.testing.expectEqual(o.nested.x, 1.0);
    std.testing.expectEqual(o.nested.y, 2.0);
}

const Vec3 = packed struct {
    x: f32,
    y: f32,
    z: f32,
};

const NestedVec3 = packed struct {
    nested: Vec3,
};

test "packed vec3 set" {
    const v = Vec3{
        .x = 1.0,
        .y = 2.0,
        .z = 3.0,
    };

    std.testing.expectEqual(v.x, 1.0);
    std.testing.expectEqual(v.y, 2.0);
    std.testing.expectEqual(v.z, 3.0);
}

test "packed vec3 ptr set" {
    const v = Vec3{
        .x = 1.0,
        .y = 2.0,
        .z = 3.0,
    };

    var o: Vec3 = undefined;
    const o_ptr: *Vec3 = &o;
    o_ptr.* = v;

    std.testing.expectEqual(o.x, 1.0);
    std.testing.expectEqual(o.y, 2.0);
    std.testing.expectEqual(o.z, 3.0);
}

test "nested packed vec3 set" {
    const a = NestedVec3{
        .nested = Vec3{
            .x = 1.0,
            .y = 2.0,
            .z = 3.0,
        },
    };

    std.testing.expectEqual(a.nested.x, 1.0);
    std.testing.expectEqual(a.nested.y, 2.0);
    std.testing.expectEqual(a.nested.z, 3.0);
}

test "nested packed vec3 ptr set" {
    const a = NestedVec3{
        .nested = Vec3{
            .x = 1.0,
            .y = 2.0,
            .z = 3.0,
        },
    };

    var o: NestedVec3 = undefined;
    const o_ptr: *NestedVec3 = &o;
    o_ptr.* = a;

    std.testing.expectEqual(o.nested.x, 1.0);
    std.testing.expectEqual(o.nested.y, 2.0);
    std.testing.expectEqual(o.nested.z, 3.0);
}

zig test bug.zig yields:

1/8 test "packed vec2 set"...OK
2/8 test "packed vec2 ptr set"...OK
3/8 test "nested packed vec2 set"...OK
4/8 test "nested packed vec2 ptr set"...OK
5/8 test "packed vec3 set"...OK
6/8 test "packed vec3 ptr set"...OK
7/8 test "nested packed vec3 set"...OK
8/8 test "nested packed vec3 ptr set"...expected 1.0e+00, found 2.0e+00
/usr/local/Cellar/zig/HEAD-dca6e74/lib/zig/std/testing.zig:54:32: 0x107c9fecf in _std.testing.expectEqual (test.o)
                std.debug.panic("expected {}, found {}", expected, actual);
                               ^
/Users/gustav/Documents/Development/zig-ws/engine/src/zig_bugs/nested_packed_struct_bug.zig:130:28: 0x107c9fd3b in _test "nested packed vec3 ptr set" (test.o)
    std.testing.expectEqual(o.nested.y, 2.0);
                           ^
/usr/local/Cellar/zig/HEAD-dca6e74/lib/zig/std/special/test_runner.zig:13:25: 0x107ccb2bd in _std.special.main (test.o)
        if (test_fn.func()) |_| {
                        ^
/usr/local/Cellar/zig/HEAD-dca6e74/lib/zig/std/special/start.zig:204:37: 0x107ccb083 in _main (test.o)
            const result = root.main() catch |err| {
                                    ^
???:?:?: 0x7fff6e3bb014 in ??? (???)

@DorianXGH
Copy link

DorianXGH commented Jan 28, 2020

I have the same bug, but even when not assigned by pointer:

const std = @import("std");
const assert = std.debug.assert;

const hld = packed struct {
    c: u64,
    d: u32,
};

const mld = packed struct {
    h: u64,
    i: u64,
};

const a = packed struct {
    b: hld,
    g: mld,
};

test "c" {
    var arg = a{ .b = hld{ .c = 1, .d = 2 }, .g = mld{ .h = 6, .i = 8 } };
    assert(arg.b.c == 1);
}

test "d" {
    var arg = a{ .b = hld{ .c = 1, .d = 2 }, .g = mld{ .h = 6, .i = 8 } };
    assert(arg.b.d == 2);
}

test "h" {
    var arg = a{ .b = hld{ .c = 1, .d = 2 }, .g = mld{ .h = 6, .i = 8 } };
    assert(arg.g.h == 6);
}

test "i" {
    var arg = a{ .b = hld{ .c = 1, .d = 2 }, .g = mld{ .h = 6, .i = 8 } };
    assert(arg.g.i == 8);
}

zig test test1.zig shows :

Test [2/4] test "d"...reached unreachable code
/usr/lib/zig/std/debug.zig:221:14: 0x20608b in std.debug.assert (test)
    if (!ok) unreachable; // assertion failure
             ^
/home/dorianb/zigtests/test1.zig:28:11: 0x206748 in test "d" (test)
    assert(arg.b.d == 2);
          ^
/usr/lib/zig/std/special/test_runner.zig:22:25: 0x227eeb in std.special.main (test)
        if (test_fn.func()) |_| {
                        ^
/usr/lib/zig/std/start.zig:250:37: 0x206b95 in std.start.posixCallMainAndExit (test)
            const result = root.main() catch |err| {
                                    ^
/usr/lib/zig/std/start.zig:114:5: 0x206a0f in std.start._start (test)
    @call(.{ .modifier = .never_inline }, posixCallMainAndExit, .{});
    ^

But when the outer struct is not packed or both are not, then all tests pass.

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.
Projects
None yet
4 participants