Skip to content

Compiler discards const stack variable declarations and generates segfaulting code #19011

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

Open
mikastiv opened this issue Feb 20, 2024 · 4 comments
Labels
backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code.
Milestone

Comments

@mikastiv
Copy link

Zig Version

0.12.0-dev.2811+3cafb9655

Steps to Reproduce and Observed Behavior

I'm on Windows 10

The code pasted below will segfault on try vertices.append(v0); in one of the ArrayList grows. This seems to be related to parameter-reference-optimization because the code will pass the arguments by pointer, which will be freed when it's time to copy them. The strange thing here is that by changing the variable declarations from const to var, the generated code is now correct and passes the arguments by value. There is also another workaround where we call try vertices.appendSlice(&.{ v0, v1 }); This happens in both debug and release fast

const std = @import("std");

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    defer _ = gpa.deinit();

    var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator);
    defer arena.deinit();

    const allocator = gpa.allocator();
    // Using arena allocator also is a workaround
    // const allocator = arena.allocator();

    var vertices = std.ArrayList([3]f32).init(allocator);
    defer vertices.deinit();

    for (0..1000) |i| {
        if (i > 2) {
            // Generates segfault
            const v0 = vertices.items[vertices.items.len - 3];
            const v1 = vertices.items[vertices.items.len - 1];
            try vertices.append(v0);
            try vertices.append(v1);

            // 1st workaround
            // var v0 = vertices.items[vertices.items.len - 3];
            // var v1 = vertices.items[vertices.items.len - 1];
            // _ = &v0;
            // _ = &v1;
            // try vertices.append(v0);
            // try vertices.append(v1);

            // 2nd workaround
            // const v0 = vertices.items[vertices.items.len - 3];
            // const v1 = vertices.items[vertices.items.len - 1];
            // try vertices.appendSlice(&.{ v0, v1 });
        }

        try vertices.append(.{ 0, 0, 0 });
    }

    std.log.err("count: {d}", .{vertices.items.len});
}

Expected Behavior

No segfault in the generated code

@mikastiv mikastiv added the bug Observed behavior contradicts documented or intended behavior label Feb 20, 2024
@mikastiv
Copy link
Author

Potential duplicates #5973, #5455

@Vexu Vexu added the backend-llvm The LLVM backend outputs an LLVM IR Module. label Feb 20, 2024
@Vexu Vexu added this to the 0.13.0 milestone Feb 20, 2024
@Vexu
Copy link
Member

Vexu commented Feb 20, 2024

Using -fno-llvm -fno-lld doesn't segfault so this is likely caused by an optimization in the LLVM backend.

@jacobly0
Copy link
Member

v0 is passed by reference to the actual element in the items slice, which is invalidated by the array list resize, causing the actual append to crash.

@Vexu Vexu added the miscompilation The compiler reports success but produces semantically incorrect code. label May 10, 2024
@andrewrk andrewrk modified the milestones: 0.14.0, 0.14.1 Mar 1, 2025
@andrewrk andrewrk modified the milestones: 0.14.1, 0.15.0 Apr 15, 2025
@joek13
Copy link

joek13 commented May 18, 2025

I ran into the same issue. Here's a slightly more concise repro:

const std = @import("std");

const Foo = struct {
    inner: u8
};

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    defer _ = gpa.deinit();
    const allocator = gpa.allocator();

    var list = std.ArrayList(Foo).init(allocator);
    defer list.deinit();

    try list.append(Foo { .inner = 42 });

    for(0..1000) |_| {
        const v = list.items[list.items.len - 1];
        try list.append(v);
    }
}
% zig version
0.14.0
% zig build-exe repro.zig && ./repro
Segmentation fault at address 0x10288007f
aborting due to recursive panic
zsh: abort      ./repro

I was able to work around the segfault by declaring v as a var:

var v: Foo = undefined;
for(0..1000) |_| {
   v = list.items[list.items.len - 1];
   try list.append(v);
}

Here's a link to Compiler Explorer that shows the generated IR in each case. In the example, the compiler seems to elide the copy and directly pass a pointer to append.

Block2:
  %23 = extractvalue { ptr, i64 } %14, 0
  %24 = getelementptr inbounds %example.Foo, ptr %23, i64 %20
  %25 = call fastcc i16 @"array_list.ArrayListAligned(example.Foo,null).append"(ptr %0, ptr nonnull align 8 %3, ptr nonnull readonly align 1 %24)
  %26 = icmp ne i16 %25, 0
  br i1 %26, label %TryRet1, label %TryCont1

The workaround causes us to copy the value, as desired:

Block2:
  %34 = extractvalue { ptr, i64 } %25, 0
  %35 = getelementptr inbounds %example.Foo, ptr %34, i64 %31
  call void @llvm.memcpy.p0.p0.i64(ptr align 1 %3, ptr align 1 %35, i64 1, i1 false)
  %36 = call fastcc i16 @"array_list.ArrayListAligned(example.Foo,null).append"(ptr %0, ptr nonnull align 8 %5, ptr nonnull readonly align 1 %3)
  %37 = icmp ne i16 %36, 0
  br i1 %37, label %TryRet1, label %TryCont1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior miscompilation The compiler reports success but produces semantically incorrect code.
Projects
None yet
Development

No branches or pull requests

5 participants