From 1c223819098e5621e2e5ad526879cd33852144e1 Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Tue, 22 Mar 2022 23:07:46 -0700 Subject: [PATCH 1/3] stage2: Properly "flatten" elem_ptrs before deref Sema.pointerDeref() assumes that elem_ptrs have been "flattened" when they were created, so that you an elem_ptr will never be the array_ptr of another elem_ptr when they share the same type. Value.elemPtr does this already, but a couple of places in Sema were bypassing this logic. --- src/Sema.zig | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index 03a8a89054..7b5296dd78 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -17045,17 +17045,9 @@ fn elemVal( const indexable_val = maybe_indexable_val orelse break :rs indexable_src; const index_val = maybe_index_val orelse break :rs elem_index_src; const index = @intCast(usize, index_val.toUnsignedInt(target)); - const elem_ty = indexable_ty.elemType2(); - - var payload: Value.Payload.ElemPtr = .{ .data = .{ - .array_ptr = indexable_val, - .elem_ty = elem_ty, - .index = index, - } }; - const elem_ptr_val = Value.initPayload(&payload.base); - + const elem_ptr_val = try indexable_val.elemPtr(indexable_ty, sema.arena, index, target); if (try sema.pointerDeref(block, indexable_src, elem_ptr_val, indexable_ty)) |elem_val| { - return sema.addConstant(elem_ty, elem_val); + return sema.addConstant(indexable_ty.elemType2(), elem_val); } break :rs indexable_src; }; @@ -17310,12 +17302,7 @@ fn elemValSlice( const sentinel_label: []const u8 = if (slice_sent) " +1 (sentinel)" else ""; return sema.fail(block, elem_index_src, "index {d} outside slice of length {d}{s}", .{ index, slice_len, sentinel_label }); } - var elem_ptr_pl: Value.Payload.ElemPtr = .{ .data = .{ - .array_ptr = slice_val.slicePtr(), - .elem_ty = elem_ty, - .index = index, - } }; - const elem_ptr_val = Value.initPayload(&elem_ptr_pl.base); + const elem_ptr_val = try slice_val.elemPtr(slice_ty, sema.arena, index, target); if (try sema.pointerDeref(block, slice_src, elem_ptr_val, slice_ty)) |elem_val| { return sema.addConstant(elem_ty, elem_val); } From 5374e245c50fde8bbb133bbff2db15efa3604721 Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Tue, 22 Mar 2022 23:32:57 -0700 Subject: [PATCH 2/3] stage2: Remove premature elem_val index check We were enforcing bounds on the index of an elem_ptr in pointerDeref, but we want to support out-of-bounds accesses by reinterpreting memory. This removes that check, so that the deref falls back to bitcasting, as usual. This was masked by another bug that was forcing bitcasts incorrectly, which is why this wasn't noticed earlier. --- src/Sema.zig | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index 7b5296dd78..fe10810d57 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -18752,6 +18752,11 @@ fn beginComptimePtrLoad( const elem_ty = elem_ptr.elem_ty; var deref = try beginComptimePtrLoad(sema, block, src, elem_ptr.array_ptr, null); + // This code assumes that elem_ptrs have been "flattened" in order for direct dereference + // to succeed, meaning that elem ptrs of the same elem_ty are coalesced. Here we check that + // our parent is not an elem_ptr with the same elem_ty, since that would be "unflattened" + if (elem_ptr.array_ptr.castTag(.elem_ptr)) |parent_elem_ptr| assert(!(parent_elem_ptr.data.elem_ty.eql(elem_ty, target))); + if (elem_ptr.index != 0) { if (elem_ty.hasWellDefinedLayout()) { if (deref.parent) |*parent| { @@ -18780,13 +18785,6 @@ fn beginComptimePtrLoad( var array_tv = deref.pointee.?; const check_len = array_tv.ty.arrayLenIncludingSentinel(); - if (elem_ptr.index >= check_len) { - // TODO have the deref include the decl so we can say "declared here" - return sema.fail(block, src, "comptime load of index {d} out of bounds of array length {d}", .{ - elem_ptr.index, check_len, - }); - } - if (maybe_array_ty) |load_ty| { // It's possible that we're loading a [N]T, in which case we'd like to slice // the pointee array directly from our parent array. @@ -18800,10 +18798,10 @@ fn beginComptimePtrLoad( } } - deref.pointee = .{ + deref.pointee = if (elem_ptr.index < check_len) TypedValue{ .ty = elem_ty, .val = try array_tv.val.elemValue(sema.arena, elem_ptr.index), - }; + } else null; break :blk deref; }, From 41e300adf17bc4056c574b32de4e07f129f2bd24 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 23 Mar 2022 13:46:06 -0700 Subject: [PATCH 3/3] add behavior test to cover bug fix in previous commit --- test/behavior/eval.zig | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/behavior/eval.zig b/test/behavior/eval.zig index 03a7a511f0..c4c6f47981 100644 --- a/test/behavior/eval.zig +++ b/test/behavior/eval.zig @@ -1,5 +1,6 @@ const builtin = @import("builtin"); const std = @import("std"); +const assert = std.debug.assert; const expect = std.testing.expect; const expectEqual = std.testing.expectEqual; @@ -830,3 +831,23 @@ test "const type-annotated local initialized with function call has correct type try expect(@TypeOf(x) == u64); try expect(x == 1234); } + +test "comptime pointer load through elem_ptr" { + const S = struct { + x: usize, + }; + + comptime { + var array: [10]S = undefined; + for (array) |*elem, i| { + elem.* = .{ + .x = i, + }; + } + var ptr = @ptrCast([*]S, &array); + var x = ptr[0].x; + assert(x == 0); + ptr += 1; + assert(ptr[1].x == 2); + } +}