stage2 x86_64 struct self pointer passed badly into function?

[david-vanderson]

Zig Version

0.10.0-dev.4448+687a7d38a

Steps to Reproduce

const std = @import("std");

const size = 5;
const val: f32 = 5.0;

const FixedBufferList = struct {
    const Self = @This();

    buffer: [size]f32 = undefined,

    pub fn items(self: Self) []const f32 {
        //std.debug.print("self {*}\n", .{&self});
        //std.debug.print("self.buffer {*}\n", .{&self.buffer});
        return self.buffer[0..size];
    }
};

test "return const slice" {
    var tmp = FixedBufferList{};

    var i: usize = 0;
    while (i < size) : (i += 1) {
        tmp.buffer[i] = val;
    }

    //std.debug.print("\ntmp {*}\n", .{&tmp});
    //std.debug.print("tmp.buffer {*}\n", .{&tmp.buffer});
    const slice = tmp.items();
    //std.debug.print("slice {*}\n", .{slice.ptr});
    for (slice) |p| {
        std.debug.print("p {}\n", .{p}); // this is neccesary to trip the bug
        try std.testing.expect(p == val);
    }
}

Expected Behavior

$ zig test test.zig
Test [1/1] test "return const slice"... p 5.0e+00
p 5.0e+00
p 5.0e+00
p 5.0e+00
p 5.0e+00
All 1 tests passed.

Actual Behavior

$ zig test test.zig
Test [1/1] test.return const slice... p 5.0e+00
p 5.0e+00
p 2.80259692e-45
Test [1/1] test.return const slice... FAIL (TestUnexpectedResult)
/home/dvanderson/apps/zig-linux-x86_64-0.10.0-dev.4448+687a7d38a/lib/std/testing.zig:347:14: 0x22841f in expect (test)
    if (!ok) return error.TestUnexpectedResult;
             ^
./test.zig:32:9: 0x228685 in test.return const slice (test)
        try std.testing.expect(p == val);
        ^
0 passed; 0 skipped; 1 failed.

Uncommenting the other debug prints shows that the self pointer passed into items() goes wrong, leading to the returned slice pointing to the wrong memory.

I'm trying to dig into it but it's my first time looking at zig compiler stuff. Any pointers would be very helpful. Thanks!

[nektro]

try

-    pub fn items(self: Self) []const f32 {
+    pub fn items(self: *const Self) []const f32 {

[nektro]

as self: Self arguments are passed by value so the temporary for self.buffer likely points to stack memory which gets cleaned up when .items() returns

whereas *const Self makes it passed by reference and therefor self.buffer to point to the proper stack memory in main/test

[david-vanderson]

Thanks for the quick explanation!

[david-vanderson]

Language proposals are currently disabled, so I'm writing this here to point to in upcoming pull requests and bug reports:

Any member function that accepts the struct by-val as the first argument is a foot-gun:

pub fn foo(self: Self) bar {
    // is self a const pointer or a copy?
    return self.internal_stuff.func();  // sometimes works, other times silently fails due to copied self
}

You want this instead:

pub fn foo(self: *const Self) bar {
    // self is pointer (with or without const as needed)
    return self.internal_stuff.func();  // always works
}

I've made this mistake myself, seen it in zig libs, and now even many places in the zig std lib. I'll send PRs for the ones I've found.

When language proposals open back up, I'll suggest making it a compile error to call any function with the var.foo() syntax if the first parameter of foo is not a pointer.

[Vexu]

You are still allowed to make proposals, especially ones aimed at removing footguns. The issue template is just meant to discourage drive-by proposals.

[david-vanderson]

Haha - it fooled me! Thanks!