Skip to content

Commit

Permalink
compiler: add intcast_safe AIR instruction
Browse files Browse the repository at this point in the history
This instruction is like `intcast`, but includes two safety checks:

* Checks that the int is in range of the destination type
* If the destination type is an exhaustive enum, checks that the int
  is a named enum value

This instruction is locked behind the `safety_checked_instructions`
backend feature; if unsupported, Sema will emit a fallback, as with
other safety-checked instructions.

This instruction is used to add a missing safety check for `@enumFromInt`
truncating bits. This check also has a fallback for backends which do
not yet support `safety_checked_instructions`.

Resolves: #21946
  • Loading branch information
mlugg committed Jan 30, 2025
1 parent c5e34df commit b01d6b1
Show file tree
Hide file tree
Showing 17 changed files with 249 additions and 29 deletions.
8 changes: 8 additions & 0 deletions src/Air.zig
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,12 @@ pub const Inst = struct {
/// See `trunc` for integer truncation.
/// Uses the `ty_op` field.
intcast,
/// Like `intcast`, but includes two safety checks:
/// * triggers a safety panic if the cast truncates bits
/// * triggers a safety panic if the destination type is an exhaustive enum
/// and the operand is not a valid value of this type; i.e. equivalent to
/// a safety check based on `.is_named_enum_value`
intcast_safe,
/// Truncate higher bits from an integer, resulting in an integer with the same
/// sign but an equal or smaller number of bits.
/// Uses the `ty_op` field.
Expand Down Expand Up @@ -1463,6 +1469,7 @@ pub fn typeOfIndex(air: *const Air, inst: Air.Inst.Index, ip: *const InternPool)
.fpext,
.fptrunc,
.intcast,
.intcast_safe,
.trunc,
.optional_payload,
.optional_payload_ptr,
Expand Down Expand Up @@ -1712,6 +1719,7 @@ pub fn mustLower(air: Air, inst: Air.Inst.Index, ip: *const InternPool) bool {
.add_safe,
.sub_safe,
.mul_safe,
.intcast_safe,
=> true,

.add,
Expand Down
1 change: 1 addition & 0 deletions src/Air/types_resolved.zig
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ fn checkBody(air: Air, body: []const Air.Inst.Index, zcu: *Zcu) bool {
.fptrunc,
.fpext,
.intcast,
.intcast_safe,
.trunc,
.optional_payload,
.optional_payload_ptr,
Expand Down
2 changes: 2 additions & 0 deletions src/Liveness.zig
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,7 @@ pub fn categorizeOperand(
.fpext,
.fptrunc,
.intcast,
.intcast_safe,
.trunc,
.optional_payload,
.optional_payload_ptr,
Expand Down Expand Up @@ -977,6 +978,7 @@ fn analyzeInst(
.fpext,
.fptrunc,
.intcast,
.intcast_safe,
.trunc,
.optional_payload,
.optional_payload_ptr,
Expand Down
1 change: 1 addition & 0 deletions src/Liveness/Verify.zig
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ fn verifyBody(self: *Verify, body: []const Air.Inst.Index) Error!void {
.fpext,
.fptrunc,
.intcast,
.intcast_safe,
.trunc,
.optional_payload,
.optional_payload_ptr,
Expand Down
63 changes: 43 additions & 20 deletions src/Sema.zig
Original file line number Diff line number Diff line change
Expand Up @@ -8798,11 +8798,12 @@ fn zirEnumFromInt(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError
const operand_src = block.builtinCallArgSrc(inst_data.src_node, 0);
const dest_ty = try sema.resolveDestType(block, src, extra.lhs, .remove_eu_opt, "@enumFromInt");
const operand = try sema.resolveInst(extra.rhs);
const operand_ty = sema.typeOf(operand);

if (dest_ty.zigTypeTag(zcu) != .@"enum") {
return sema.fail(block, src, "expected enum, found '{}'", .{dest_ty.fmt(pt)});
}
_ = try sema.checkIntType(block, operand_src, sema.typeOf(operand));
_ = try sema.checkIntType(block, operand_src, operand_ty);

if (try sema.resolveValue(operand)) |int_val| {
if (dest_ty.isNonexhaustiveEnum(zcu)) {
Expand Down Expand Up @@ -8830,23 +8831,39 @@ fn zirEnumFromInt(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError
}

if (try sema.typeHasOnePossibleValue(dest_ty)) |opv| {
const result = Air.internedToRef(opv.toIntern());
// The operand is runtime-known but the result is comptime-known. In
// this case we still need a safety check.
// TODO add a safety check here. we can't use is_named_enum_value -
// it needs to convert the enum back to int and make sure it equals the operand int.
return result;
if (block.wantSafety()) {
// The operand is runtime-known but the result is comptime-known. In
// this case we still need a safety check.
const expect_int_val = switch (zcu.intern_pool.indexToKey(opv.toIntern())) {
.enum_tag => |enum_tag| enum_tag.int,
else => unreachable,
};
const expect_int_coerced = try pt.getCoerced(.fromInterned(expect_int_val), operand_ty);
const ok = try block.addBinOp(.cmp_eq, operand, Air.internedToRef(expect_int_coerced.toIntern()));
try sema.addSafetyCheck(block, src, ok, .invalid_enum_value);
}
return Air.internedToRef(opv.toIntern());
}

try sema.requireRuntimeBlock(block, src, operand_src);
const result = try block.addTyOp(.intcast, dest_ty, operand);
if (block.wantSafety() and !dest_ty.isNonexhaustiveEnum(zcu) and
zcu.backendSupportsFeature(.is_named_enum_value))
{
const ok = try block.addUnOp(.is_named_enum_value, result);
try sema.addSafetyCheck(block, src, ok, .invalid_enum_value);
if (block.wantSafety()) {
if (zcu.backendSupportsFeature(.safety_checked_instructions)) {
_ = try sema.preparePanicId(src, .invalid_enum_value);
return block.addTyOp(.intcast_safe, dest_ty, operand);
} else {
// Slightly silly fallback case...
const int_tag_ty = dest_ty.intTagType(zcu);
// Use `intCast`, since it'll set up the Sema-emitted safety checks for us!
const int_val = try sema.intCast(block, src, int_tag_ty, src, operand, src, true, true);
const result = try block.addBitCast(dest_ty, int_val);
if (zcu.backendSupportsFeature(.is_named_enum_value)) {
const ok = try block.addUnOp(.is_named_enum_value, result);
try sema.addSafetyCheck(block, src, ok, .invalid_enum_value);
}
return result;
}
}
return result;
return block.addTyOp(.intcast, dest_ty, operand);
}

/// Pointer in, pointer out.
Expand Down Expand Up @@ -10192,7 +10209,7 @@ fn zirIntCast(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air
const dest_ty = try sema.resolveDestType(block, src, extra.lhs, .remove_eu_opt, "@intCast");
const operand = try sema.resolveInst(extra.rhs);

return sema.intCast(block, block.nodeOffset(inst_data.src_node), dest_ty, src, operand, operand_src, true);
return sema.intCast(block, block.nodeOffset(inst_data.src_node), dest_ty, src, operand, operand_src, true, false);
}

fn intCast(
Expand All @@ -10204,6 +10221,7 @@ fn intCast(
operand: Air.Inst.Ref,
operand_src: LazySrcLoc,
runtime_safety: bool,
safety_panics_are_enum: bool,
) CompileError!Air.Inst.Ref {
const pt = sema.pt;
const zcu = pt.zcu;
Expand Down Expand Up @@ -10242,7 +10260,7 @@ fn intCast(
const is_in_range = try block.addBinOp(.cmp_lte, operand, zero_inst);
break :ok is_in_range;
};
try sema.addSafetyCheck(block, src, ok, .cast_truncated_data);
try sema.addSafetyCheck(block, src, ok, if (safety_panics_are_enum) .invalid_enum_value else .cast_truncated_data);
}
}

Expand All @@ -10251,6 +10269,11 @@ fn intCast(

try sema.requireRuntimeBlock(block, src, operand_src);
if (runtime_safety and block.wantSafety()) {
if (zcu.backendSupportsFeature(.safety_checked_instructions)) {
_ = try sema.preparePanicId(src, .negative_to_unsigned);
_ = try sema.preparePanicId(src, .cast_truncated_data);
return block.addTyOp(.intcast_safe, dest_ty, operand);
}
const actual_info = operand_scalar_ty.intInfo(zcu);
const wanted_info = dest_scalar_ty.intInfo(zcu);
const actual_bits = actual_info.bits;
Expand Down Expand Up @@ -10305,7 +10328,7 @@ fn intCast(
break :ok is_in_range;
};
// TODO negative_to_unsigned?
try sema.addSafetyCheck(block, src, ok, .cast_truncated_data);
try sema.addSafetyCheck(block, src, ok, if (safety_panics_are_enum) .invalid_enum_value else .cast_truncated_data);
} else {
const ok = if (is_vector) ok: {
const is_in_range = try block.addCmpVector(operand, dest_max, .lte);
Expand All @@ -10321,7 +10344,7 @@ fn intCast(
const is_in_range = try block.addBinOp(.cmp_lte, operand, dest_max);
break :ok is_in_range;
};
try sema.addSafetyCheck(block, src, ok, .cast_truncated_data);
try sema.addSafetyCheck(block, src, ok, if (safety_panics_are_enum) .invalid_enum_value else .cast_truncated_data);
}
} else if (actual_info.signedness == .signed and wanted_info.signedness == .unsigned) {
// no shrinkage, yes sign loss
Expand All @@ -10344,7 +10367,7 @@ fn intCast(
const is_in_range = try block.addBinOp(.cmp_gte, operand, zero_inst);
break :ok is_in_range;
};
try sema.addSafetyCheck(block, src, ok, .negative_to_unsigned);
try sema.addSafetyCheck(block, src, ok, if (safety_panics_are_enum) .invalid_enum_value else .negative_to_unsigned);
}
}
return block.addTyOp(.intcast, dest_ty, operand);
Expand Down Expand Up @@ -14149,7 +14172,7 @@ fn zirShl(
{
const max_int = Air.internedToRef((try lhs_ty.maxInt(pt, lhs_ty)).toIntern());
const rhs_limited = try sema.analyzeMinMax(block, rhs_src, .min, &.{ rhs, max_int }, &.{ rhs_src, rhs_src });
break :rhs try sema.intCast(block, src, lhs_ty, rhs_src, rhs_limited, rhs_src, false);
break :rhs try sema.intCast(block, src, lhs_ty, rhs_src, rhs_limited, rhs_src, false, false);
} else {
break :rhs rhs;
}
Expand Down
3 changes: 2 additions & 1 deletion src/Zcu.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3313,7 +3313,7 @@ pub fn addGlobalAssembly(zcu: *Zcu, unit: AnalUnit, source: []const u8) !void {

pub const Feature = enum {
/// When this feature is enabled, Sema will emit calls to
/// `std.builtin.Panic` functions for things like safety checks and
/// `std.builtin.panic` functions for things like safety checks and
/// unreachables. Otherwise traps will be emitted.
panic_fn,
/// When this feature is enabled, Sema will insert tracer functions for gathering a stack
Expand All @@ -3329,6 +3329,7 @@ pub const Feature = enum {
/// * `Air.Inst.Tag.add_safe`
/// * `Air.Inst.Tag.sub_safe`
/// * `Air.Inst.Tag.mul_safe`
/// * `Air.Inst.Tag.intcast_safe`
/// The motivation for this feature is that it makes AIR smaller, and makes it easier
/// to generate better machine code in the backends. All backends should migrate to
/// enabling this feature.
Expand Down
1 change: 1 addition & 0 deletions src/arch/aarch64/CodeGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,7 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
.add_safe,
.sub_safe,
.mul_safe,
.intcast_safe,
=> return self.fail("TODO implement safety_checked_instructions", .{}),

.is_named_enum_value => return self.fail("TODO implement is_named_enum_value", .{}),
Expand Down
1 change: 1 addition & 0 deletions src/arch/arm/CodeGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,7 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
.add_safe,
.sub_safe,
.mul_safe,
.intcast_safe,
=> return self.fail("TODO implement safety_checked_instructions", .{}),

.is_named_enum_value => return self.fail("TODO implement is_named_enum_value", .{}),
Expand Down
1 change: 1 addition & 0 deletions src/arch/riscv64/CodeGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1517,6 +1517,7 @@ fn genBody(func: *Func, body: []const Air.Inst.Index) InnerError!void {
.add_safe,
.sub_safe,
.mul_safe,
.intcast_safe,
=> return func.fail("TODO implement safety_checked_instructions", .{}),

.cmp_lt,
Expand Down
1 change: 1 addition & 0 deletions src/arch/sparc64/CodeGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,7 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void {
.add_safe,
.sub_safe,
.mul_safe,
.intcast_safe,
=> @panic("TODO implement safety_checked_instructions"),

.is_named_enum_value => @panic("TODO implement is_named_enum_value"),
Expand Down
1 change: 1 addition & 0 deletions src/arch/wasm/CodeGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2084,6 +2084,7 @@ fn genInst(cg: *CodeGen, inst: Air.Inst.Index) InnerError!void {
.add_safe,
.sub_safe,
.mul_safe,
.intcast_safe,
=> return cg.fail("TODO implement safety_checked_instructions", .{}),

.work_item_id,
Expand Down
1 change: 1 addition & 0 deletions src/arch/x86_64/CodeGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2549,6 +2549,7 @@ fn genBody(cg: *CodeGen, body: []const Air.Inst.Index) InnerError!void {
.add_safe,
.sub_safe,
.mul_safe,
.intcast_safe,
=> return cg.fail("TODO implement safety_checked_instructions", .{}),

.add_optimized => try cg.airBinOp(inst, .add),
Expand Down
1 change: 1 addition & 0 deletions src/codegen/c.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3436,6 +3436,7 @@ fn genBodyInner(f: *Function, body: []const Air.Inst.Index) error{ AnalysisFail,
.add_safe,
.sub_safe,
.mul_safe,
.intcast_safe,
=> return f.fail("TODO implement safety_checked_instructions", .{}),

.is_named_enum_value => return f.fail("TODO: C backend: implement is_named_enum_value", .{}),
Expand Down
Loading

0 comments on commit b01d6b1

Please sign in to comment.