ziglang / zig

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

Coercion of tuple pointer to slice violates aliasing and lifetime expectations #13334

Open topolarity opened 2 years ago

topolarity commented 2 years ago

Zig Version

0.10.0-dev.1246+e67c756b91

Steps to Reproduce and Observed Behavior

const std = @import("std");
const expect = std.testing.expect;

fn clobberStack() void { var array: [20]u32 = undefined; _ = array[1]; }
fn asSlice(comptime T: type, ptr: anytype) []const T {
    switch (@typeInfo(@TypeOf(ptr))) {
        .Pointer => return @as([]const T, ptr),
        else => unreachable,
    }
}

test "implicit tuple-ptr to array-ptr cast refers to temporary with shorter lifetime" {
    const queue = try std.heap.page_allocator.create(std.meta.Tuple(&[_]type{ u32, u32 }));
    queue.* = .{ 1, 2 };

    // Unlike every other ptr->ptr cast in Zig, slice ends up referring to a temporary 
    // in asSlice rather than to the heap-allocated `queue`
    var slice = asSlice(u32, queue);
    _ = clobberStack();

    // Surely `slice` is still valid here? Nope.
    // This fails:
    try expect(std.mem.eql(u32, slice, &[_]u32{1, 2}));

    //  The conversion in `asSlice` implicitly shortened the lifetime of the cast result
}

test "aliasing not respected" {
    var x: usize = 1;
    var a = .{ x, x + 1, x + 2, x + 3 };
    var b: []const usize = &a;

    a[0] = 7; // Changes to a are not reflected in b

    try expect(a[0] == b[0]); // fails
}

Both tests fail

Expected Behavior

Both of these behaviors are incongruous with how reference semantics typically work in Zig. In every other pointer-to-pointer cast (implicit or explicit), aliasing is respected and the valid lifetime of the created pointer is the same as the pointer being cast from.

topolarity commented 2 years ago

Wanted to file this to follow-up on the discussion in https://github.com/ziglang/zig/pull/12826

A possible solution was proposed in that PR: Make this a question of type inference rather than coercion, by allowing &.{...} to coerce to the type of its result location, but not allowing this for any tuple pointer created after the fact.

In that case, there is no tuple pointer to alias against at all. We are essentially inferring that .{...} should have been an array from the beginning.

Vexu commented 1 year ago

These concerns were brought up when this coercion was initially implemented and if only allowing &.{} gets rid of them then we should go with that since &.{} is the only intended use case in the first place (at least IMO).

mlugg commented 11 months ago

Note that as of #16512, this coercion is not used for the &.{ ... } case. Also note that the accepted proposal #16865 eliminates anonymous struct types entirely, so implicitly removes this coercion.