zig icon indicating copy to clipboard operation
zig copied to clipboard

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

Open samy-00007 opened this issue 1 year ago • 5 comments

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

samy-00007 avatar Aug 20 '24 22:08 samy-00007

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 avatar Aug 20 '24 23:08 nektro

more specifically:

  • list.items in main is &.{}
  • addToList causes it to be allocator.alloc(1) since list.capacity is 0
  • list.items.len is now 1 but the temporary had already been created

not sure this is a bug.

nektro avatar Aug 20 '24 23:08 nektro

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
}

nektro avatar Aug 20 '24 23:08 nektro

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

alexrp avatar Aug 21 '24 16:08 alexrp

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 .

rohlem avatar Aug 21 '24 18:08 rohlem