ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.83k stars 2.48k forks source link

index out of bounds when accessing when accessing an element in an array #21152

Open samy-00007 opened 1 month ago

samy-00007 commented 1 month ago

Zig Version

0.14.0-dev.1217+dffc8c44f

Steps to Reproduce and Observed Behavior

I apologize for the poor issue title. Here is a minimal code that produces the error:

const std = @import("std");

 var gpa = std.heap.GeneralPurposeAllocator(.{}){};
 const allocator = gpa.allocator();
 var list = std.ArrayList(usize).init(allocator);

 pub fn main() !void {
     std.debug.print("{}\n", .{list.items[try addToList(2)]});
 }

 fn addToLtist(x: usize) !usize {
     const result = list.items.len;
     try list.append(x);
     return result;
 }

Observed behavior: thread 144618: index out of bounds: index 0, len 0

Expected Behavior

The array access should not fail, since the element exists before it is accessed.

It is even possible to log the length of the array before the panic by putting a block (for instance blk: { const r = try addToList(2); std.debug.print("len: {}", .{list.items.len}); break :blk r; } ). I am guessing the out of bound detection code takes the length of the array before compute the result of addToList

nektro commented 1 month ago

I think whats happening is that list.items in main is a stale pointer because addToList causes the backing allocation to be reallocated in order to grow in size

nektro commented 1 month ago

more specifically:

not sure this is a bug.

nektro commented 1 month ago

its exhibiting the same failure as

pub fn main() !void {
    const x = list.items; // { ptr: undefined, len: 0 } is on the stack
    const y = try addToList(2); // original list.items is updated
    std.debug.print("{}\n", .{x[y]}); // access and print old value
}
alexrp commented 1 month ago

The order of evaluation is working the way I'd expect it to here.

rohlem commented 1 month ago

I think I can see arguments for both evaluation orders: Reading list.items before it was updated is left-to-right reading order, reading it after it was updated might conserve stack space. As-is the behavior of the code is underdefined, just as I believe the equivalent C code would be (those 2 expressions share the same sequence point -> are indeterminately sequenced). If Zig's language specification is going to keep the evaluation order as loose as C (doesn't enforce more sequence points), then I think this is unpredictable behavior that we might want to point out via a safety check (panic in safe build modes) - see #1966 and https://github.com/ziglang/zig/issues/2301 .