Skip to content

Conversation

@Justus2308
Copy link
Contributor

@Justus2308 Justus2308 commented Nov 14, 2025

Resolves #22788
Resolves #24320

Removes an incorrect check that allowed pointers to mutable pointers to coerce to any other pointer to a mutable pointer.

I'm 99% sure that the check for !dest_is_mut doesn't make any sense here, however the entire coercion logic is quite dense so please correct me if I'm wrong.
This does fix the issue though, the test I added compiles just fine with master and doesn't anymore with this patch applied.

@alexrp alexrp requested a review from mlugg November 14, 2025 11:55
src/Sema.zig Outdated
if (child != .ok) allow: {
// As a special case, we also allow coercing `*[n:s]T` to `*[n]T`, akin to dropping the sentinel from a slice.
// `*[n:s]T` cannot coerce in memory to `*[n]T` since they have different sizes.
if (src_child.zigTypeTag(zcu) == .array and dest_child.zigTypeTag(zcu) == .array and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (src_child.zigTypeTag(zcu) == .array and dest_child.zigTypeTag(zcu) == .array and
if (!dest_is_mut and src_child.zigTypeTag(zcu) == .array and dest_child.zigTypeTag(zcu) == .array and

Sorry, I see the mistake I made in the original code: the !dest_is_mut was meant to be on the special case allowing an additional coercion, not on the outer error case check.

I haven't tested this, but in theory, the (minor) bug which this suggestion^^ fixes is that the coercion **[n:s]T -> **[n]T incorrectly succeeds:

export fn foo() void {
    var x: [1:42]u8 = .{0};
    var y: [1]u8 = .{0};

    var ptr: *[1:42]u8 = &x;

    const a: **[1:42]u8 = &ptr;
    const b: **[1]u8 = a;
    b.* = &y;

    // 'ptr' is a '*[1:42]u8', but its value is '&y', which should be a '*[1]u8'
    // so we've managed to assign an illegal value to 'ptr'!
}

The point of the !dest_is_mut check is that *[n:s]T -> *[n]T is okay, as is *const *[n:s]T -> *const *[n]T, but not **[n:s]T -> **[n]T.

Don't worry if you find this confusing, because it definitely is; but the point is that dest_is_mut implies that the coercion needs to be valid in both directions, because the in-memory coercion A -> B is actually being checked for a *A -> *B coercion, which (if permitted) effectively allows in-memory-coercing B -> A by storing to the *B and loading from the *A. In other words, dest_is_mut could be implemented by replacing every call to coerceInMemoryAllowed(..., true) with two calls (where the second call has src and dest flipped), but it's implemented this way instead to avoid exponential behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, thanks for the explanation!
**[n:s]T -> **[n]T does indeed succeed without your change, I'll adapt the test a bit

Moves a premature check that allowed pointers to mutable pointers to coerce
to any other pointer to a mutable pointer.
@Justus2308
Copy link
Contributor Author

Justus2308 commented Nov 14, 2025

I've applied your fix and added a somewhat helpful error message for a known actual sentinel and no wanted sentinel (previously: note: array sentinel '42' cannot cast into array sentinel 'unreachable')

@Justus2308 Justus2308 requested a review from mlugg November 14, 2025 13:18
@mlugg mlugg enabled auto-merge (rebase) November 14, 2025 16:10
@mlugg mlugg merged commit 06d08da into ziglang:master Nov 14, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Footgun in slice assignements Slices of incompatible pointer types coerce incorrectly

2 participants