Skip to content

Fix Sema unreachable when shfting by vector containing undef elems #24396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions src/Sema.zig
Original file line number Diff line number Diff line change
Expand Up @@ -13572,8 +13572,8 @@ fn zirShl(
const maybe_rhs_val = try sema.resolveValueResolveLazy(rhs);

if (maybe_rhs_val) |rhs_val| {
if (rhs_val.isUndef(zcu)) {
return pt.undefRef(sema.typeOf(lhs));
if (rhs_val.anyScalarIsUndef(zcu)) {
return sema.failWithUseOfUndef(block, rhs_src);
}
// If rhs is 0, return lhs without doing any calculations.
if (try rhs_val.compareAllWithZeroSema(.eq, pt)) {
Expand Down Expand Up @@ -13621,7 +13621,9 @@ fn zirShl(
}

const runtime_src = if (maybe_lhs_val) |lhs_val| rs: {
if (lhs_val.isUndef(zcu)) return pt.undefRef(lhs_ty);
if (lhs_val.anyScalarIsUndef(zcu)) {
return sema.failWithUseOfUndef(block, lhs_src);
}
const rhs_val = maybe_rhs_val orelse {
if (scalar_ty.zigTypeTag(zcu) == .comptime_int) {
return sema.fail(block, src, "LHS of shift must be a fixed-width integer type, or RHS must be comptime-known", .{});
Expand Down Expand Up @@ -13753,8 +13755,10 @@ fn zirShr(
const maybe_rhs_val = try sema.resolveValueResolveLazy(rhs);

const runtime_src = if (maybe_rhs_val) |rhs_val| rs: {
if (rhs_val.isUndef(zcu)) {
return pt.undefRef(lhs_ty);
switch (air_tag) {
.shr => if (rhs_val.isUndefDeep(zcu)) return pt.undefRef(lhs_ty),
.shr_exact => if (rhs_val.anyScalarIsUndef(zcu)) return sema.failWithUseOfUndef(block, rhs_src),
else => unreachable,
}
// If rhs is 0, return lhs without doing any calculations.
if (try rhs_val.compareAllWithZeroSema(.eq, pt)) {
Expand All @@ -13766,6 +13770,7 @@ fn zirShr(
var i: usize = 0;
while (i < rhs_ty.vectorLen(zcu)) : (i += 1) {
const rhs_elem = try rhs_val.elemValue(pt, i);
if (rhs_elem.isUndef(zcu)) continue;
if (rhs_elem.compareHetero(.gte, bit_value, zcu)) {
return sema.fail(block, rhs_src, "shift amount '{f}' at index '{d}' is too large for operand type '{f}'", .{
rhs_elem.fmtValueSema(pt, sema),
Expand All @@ -13785,6 +13790,7 @@ fn zirShr(
var i: usize = 0;
while (i < rhs_ty.vectorLen(zcu)) : (i += 1) {
const rhs_elem = try rhs_val.elemValue(pt, i);
if (rhs_elem.isUndef(zcu)) continue;
if (rhs_elem.compareHetero(.lt, try pt.intValue(rhs_ty.childType(zcu), 0), zcu)) {
return sema.fail(block, rhs_src, "shift by negative amount '{f}' at index '{d}'", .{
rhs_elem.fmtValueSema(pt, sema),
Expand All @@ -13798,15 +13804,17 @@ fn zirShr(
});
}
if (maybe_lhs_val) |lhs_val| {
if (lhs_val.isUndef(zcu)) {
return pt.undefRef(lhs_ty);
}
if (air_tag == .shr_exact) {
// Detect if any ones would be shifted out.
const truncated = try lhs_val.intTruncBitsAsValue(lhs_ty, sema.arena, .unsigned, rhs_val, pt);
if (!(try truncated.compareAllWithZeroSema(.eq, pt))) {
return sema.fail(block, src, "exact shift shifted out 1 bits", .{});
}
switch (air_tag) {
.shr => if (lhs_val.isUndefDeep(zcu)) return pt.undefRef(lhs_ty),
.shr_exact => {
if (lhs_val.anyScalarIsUndef(zcu)) return sema.failWithUseOfUndef(block, lhs_src);
// Detect if any ones would be shifted out.
const truncated = try lhs_val.intTruncBitsAsValue(lhs_ty, sema.arena, .unsigned, rhs_val, pt);
if (!(try truncated.compareAllWithZeroSema(.eq, pt))) {
return sema.fail(block, src, "exact shift shifted out 1 bits", .{});
}
},
else => unreachable,
}
const val = try lhs_val.shr(rhs_val, lhs_ty, sema.arena, pt);
return Air.internedToRef(val.toIntern());
Expand Down
19 changes: 15 additions & 4 deletions src/Value.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1386,11 +1386,22 @@ pub fn isUndef(val: Value, zcu: *const Zcu) bool {
return zcu.intern_pool.isUndef(val.toIntern());
}

/// TODO: check for cases such as array that is not marked undef but all the element
/// values are marked undef, or struct that is not marked undef but all fields are marked
/// undef, etc.
pub fn isUndefDeep(val: Value, zcu: *const Zcu) bool {
return val.isUndef(zcu);
if (val.toIntern() == .undef) return true;
switch (zcu.intern_pool.indexToKey(val.toIntern())) {
.undef => return true,
.int, .float => return false,
.aggregate => |agg| {
const elems = agg.storage.values();
if (elems.len == 0) return false;
for (elems) |elem_val| {
if (!Value.fromInterned(elem_val).isUndefDeep(zcu)) return false;
}
return true;
},
.un => |un| return if (un.tag == .none) Value.fromInterned(un.val).isUndefDeep(zcu) else false,
else => return false,
}
}

/// `val` must have a numeric or vector type.
Expand Down
6 changes: 0 additions & 6 deletions test/behavior/bit_shifting.zig
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,6 @@ test "Saturating Shift Left where lhs is of a computed type" {
try expect(value.exponent == 0);
}

comptime {
var image: [1]u8 = undefined;
_ = &image;
_ = @shlExact(@as(u16, image[0]), 8);
}

test "Saturating Shift Left" {
if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
if (builtin.zig_backend == .stage2_c) return error.SkipZigTest;
Expand Down
Loading
Loading