diff options
| author | Veikka Tuominen <git@vexu.eu> | 2022-11-29 14:53:54 +0200 |
|---|---|---|
| committer | Veikka Tuominen <git@vexu.eu> | 2022-11-29 21:44:08 +0200 |
| commit | 17ff002bc0ac55850e647fc3a70a43d1d874f6ab (patch) | |
| tree | 700cbc62d0ec95cc2b660cb8d33ab0970e39c9f8 | |
| parent | 6337c04244d9c27cc6535340347d4c127f4742eb (diff) | |
| download | zig-17ff002bc0ac55850e647fc3a70a43d1d874f6ab.tar.gz zig-17ff002bc0ac55850e647fc3a70a43d1d874f6ab.zip | |
Sema: improve safety panic for access of inactive union field
| -rw-r--r-- | doc/langref.html.in | 2 | ||||
| -rw-r--r-- | lib/std/builtin.zig | 5 | ||||
| -rw-r--r-- | src/Sema.zig | 119 | ||||
| -rw-r--r-- | test/cases/safety/bad union field access.zig | 2 |
4 files changed, 45 insertions, 83 deletions
diff --git a/doc/langref.html.in b/doc/langref.html.in index 8974ab06cf..dcb15af9f6 100644 --- a/doc/langref.html.in +++ b/doc/langref.html.in @@ -3803,7 +3803,7 @@ test "switch on non-exhaustive enum" { {#link|Accessing the non-active field|Wrong Union Field Access#} is safety-checked {#link|Undefined Behavior#}: </p> - {#code_begin|test_err|inactive union field#} + {#code_begin|test_err|access of union field 'float' while field 'int' is active#} const Payload = union { int: i64, float: f64, diff --git a/lib/std/builtin.zig b/lib/std/builtin.zig index 09833988ae..fcdf43bd31 100644 --- a/lib/std/builtin.zig +++ b/lib/std/builtin.zig @@ -868,6 +868,11 @@ pub fn panicStartGreaterThanEnd(start: usize, end: usize) noreturn { std.debug.panicExtra(null, @returnAddress(), "start index {d} is larger than end index {d}", .{ start, end }); } +pub fn panicInactiveUnionField(active: anytype, wanted: @TypeOf(active)) noreturn { + @setCold(true); + std.debug.panicExtra(null, @returnAddress(), "access of union field '{s}' while field '{s}' is active", .{ @tagName(wanted), @tagName(active) }); +} + pub const panic_messages = struct { pub const unreach = "reached unreachable code"; pub const unwrap_null = "attempt to use null value"; diff --git a/src/Sema.zig b/src/Sema.zig index d35c4e3881..396c5912fa 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -22120,7 +22120,6 @@ pub const PanicId = enum { shr_overflow, divide_by_zero, exact_division_remainder, - /// TODO make this call `std.builtin.panicInactiveUnionField`. inactive_union_field, integer_part_out_of_bounds, corrupt_switch, @@ -22296,90 +22295,40 @@ fn panicUnwrapError( fn panicIndexOutOfBounds( sema: *Sema, parent_block: *Block, - src: LazySrcLoc, index: Air.Inst.Ref, len: Air.Inst.Ref, cmp_op: Air.Inst.Tag, ) !void { assert(!parent_block.is_comptime); const ok = try parent_block.addBinOp(cmp_op, index, len); - const gpa = sema.gpa; - - var fail_block: Block = .{ - .parent = parent_block, - .sema = sema, - .src_decl = parent_block.src_decl, - .namespace = parent_block.namespace, - .wip_capture_scope = parent_block.wip_capture_scope, - .instructions = .{}, - .inlining = parent_block.inlining, - .is_comptime = false, - }; - - defer fail_block.instructions.deinit(gpa); - - { - const this_feature_is_implemented_in_the_backend = - sema.mod.comp.bin_file.options.use_llvm; - - if (!this_feature_is_implemented_in_the_backend) { - // TODO implement this feature in all the backends and then delete this branch - _ = try fail_block.addNoOp(.breakpoint); - _ = try fail_block.addNoOp(.unreach); - } else { - const panic_fn = try sema.getBuiltin("panicOutOfBounds"); - const args: [2]Air.Inst.Ref = .{ index, len }; - _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, &args, null); - } - } - try sema.addSafetyCheckExtra(parent_block, ok, &fail_block); + try sema.safetyPanicFormatted(parent_block, ok, "panicOutOfBounds", &.{ index, len }); } fn panicStartLargerThanEnd( sema: *Sema, parent_block: *Block, - src: LazySrcLoc, start: Air.Inst.Ref, end: Air.Inst.Ref, ) !void { assert(!parent_block.is_comptime); const ok = try parent_block.addBinOp(.cmp_lte, start, end); - const gpa = sema.gpa; - - var fail_block: Block = .{ - .parent = parent_block, - .sema = sema, - .src_decl = parent_block.src_decl, - .namespace = parent_block.namespace, - .wip_capture_scope = parent_block.wip_capture_scope, - .instructions = .{}, - .inlining = parent_block.inlining, - .is_comptime = false, - }; - - defer fail_block.instructions.deinit(gpa); - - { - const this_feature_is_implemented_in_the_backend = - sema.mod.comp.bin_file.options.use_llvm; + try sema.safetyPanicFormatted(parent_block, ok, "panicStartGreaterThanEnd", &.{ start, end }); +} - if (!this_feature_is_implemented_in_the_backend) { - // TODO implement this feature in all the backends and then delete this branch - _ = try fail_block.addNoOp(.breakpoint); - _ = try fail_block.addNoOp(.unreach); - } else { - const panic_fn = try sema.getBuiltin("panicStartGreaterThanEnd"); - const args: [2]Air.Inst.Ref = .{ start, end }; - _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, &args, null); - } - } - try sema.addSafetyCheckExtra(parent_block, ok, &fail_block); +fn panicInactiveUnionField( + sema: *Sema, + parent_block: *Block, + active_tag: Air.Inst.Ref, + wanted_tag: Air.Inst.Ref, +) !void { + assert(!parent_block.is_comptime); + const ok = try parent_block.addBinOp(.cmp_eq, active_tag, wanted_tag); + try sema.safetyPanicFormatted(parent_block, ok, "panicInactiveUnionField", &.{ active_tag, wanted_tag }); } fn panicSentinelMismatch( sema: *Sema, parent_block: *Block, - src: LazySrcLoc, maybe_sentinel: ?Value, sentinel_ty: Type, ptr: Air.Inst.Ref, @@ -22413,9 +22362,20 @@ fn panicSentinelMismatch( else { const panic_fn = try sema.getBuiltin("checkNonScalarSentinel"); const args: [2]Air.Inst.Ref = .{ expected_sentinel, actual_sentinel }; - _ = try sema.analyzeCall(parent_block, panic_fn, src, src, .auto, false, &args, null); + _ = try sema.analyzeCall(parent_block, panic_fn, sema.src, sema.src, .auto, false, &args, null); return; }; + + try sema.safetyPanicFormatted(parent_block, ok, "panicSentinelMismatch", &.{ expected_sentinel, actual_sentinel }); +} + +fn safetyPanicFormatted( + sema: *Sema, + parent_block: *Block, + ok: Air.Inst.Ref, + func: []const u8, + args: []const Air.Inst.Ref, +) CompileError!void { const gpa = sema.gpa; var fail_block: Block = .{ @@ -22440,9 +22400,8 @@ fn panicSentinelMismatch( _ = try fail_block.addNoOp(.breakpoint); _ = try fail_block.addNoOp(.unreach); } else { - const panic_fn = try sema.getBuiltin("panicSentinelMismatch"); - const args: [2]Air.Inst.Ref = .{ expected_sentinel, actual_sentinel }; - _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, &args, null); + const panic_fn = try sema.getBuiltin(func); + _ = try sema.analyzeCall(&fail_block, panic_fn, sema.src, sema.src, .auto, false, args, null); } } try sema.addSafetyCheckExtra(parent_block, ok, &fail_block); @@ -23465,8 +23424,7 @@ fn unionFieldPtr( // TODO would it be better if get_union_tag supported pointers to unions? const union_val = try block.addTyOp(.load, union_ty, union_ptr); const active_tag = try block.addTyOp(.get_union_tag, union_obj.tag_ty, union_val); - const ok = try block.addBinOp(.cmp_eq, active_tag, wanted_tag); - try sema.addSafetyCheck(block, ok, .inactive_union_field); + try sema.panicInactiveUnionField(block, active_tag, wanted_tag); } if (field.ty.zigTypeTag() == .NoReturn) { _ = try block.addNoOp(.unreach); @@ -23537,8 +23495,7 @@ fn unionFieldVal( const wanted_tag_val = try Value.Tag.enum_field_index.create(sema.arena, enum_field_index); const wanted_tag = try sema.addConstant(union_obj.tag_ty, wanted_tag_val); const active_tag = try block.addTyOp(.get_union_tag, union_obj.tag_ty, union_byval); - const ok = try block.addBinOp(.cmp_eq, active_tag, wanted_tag); - try sema.addSafetyCheck(block, ok, .inactive_union_field); + try sema.panicInactiveUnionField(block, active_tag, wanted_tag); } if (field.ty.zigTypeTag() == .NoReturn) { _ = try block.addNoOp(.unreach); @@ -23849,7 +23806,7 @@ fn elemValArray( if (maybe_index_val == null) { const len_inst = try sema.addIntUnsigned(Type.usize, array_len); const cmp_op: Air.Inst.Tag = if (array_sent != null) .cmp_lte else .cmp_lt; - try sema.panicIndexOutOfBounds(block, elem_index_src, elem_index, len_inst, cmp_op); + try sema.panicIndexOutOfBounds(block, elem_index, len_inst, cmp_op); } } return block.addBinOp(.array_elem_val, array, elem_index); @@ -23910,7 +23867,7 @@ fn elemPtrArray( if (block.wantSafety() and offset == null) { const len_inst = try sema.addIntUnsigned(Type.usize, array_len); const cmp_op: Air.Inst.Tag = if (array_sent) .cmp_lte else .cmp_lt; - try sema.panicIndexOutOfBounds(block, elem_index_src, elem_index, len_inst, cmp_op); + try sema.panicIndexOutOfBounds(block, elem_index, len_inst, cmp_op); } return block.addPtrElemPtr(array_ptr, elem_index, elem_ptr_ty); @@ -23966,7 +23923,7 @@ fn elemValSlice( else try block.addTyOp(.slice_len, Type.usize, slice); const cmp_op: Air.Inst.Tag = if (slice_sent) .cmp_lte else .cmp_lt; - try sema.panicIndexOutOfBounds(block, elem_index_src, elem_index, len_inst, cmp_op); + try sema.panicIndexOutOfBounds(block, elem_index, len_inst, cmp_op); } try sema.queueFullTypeResolution(sema.typeOf(slice)); return block.addBinOp(.slice_elem_val, slice, elem_index); @@ -24025,7 +23982,7 @@ fn elemPtrSlice( break :len try block.addTyOp(.slice_len, Type.usize, slice); }; const cmp_op: Air.Inst.Tag = if (slice_sent) .cmp_lte else .cmp_lt; - try sema.panicIndexOutOfBounds(block, elem_index_src, elem_index, len_inst, cmp_op); + try sema.panicIndexOutOfBounds(block, elem_index, len_inst, cmp_op); } return block.addSliceElemPtr(slice, elem_index, elem_ptr_ty); } @@ -28072,7 +28029,7 @@ fn analyzeSlice( if (block.wantSafety() and !block.is_comptime) { // requirement: start <= end - try sema.panicStartLargerThanEnd(block, src, start, end); + try sema.panicStartLargerThanEnd(block, start, end); } const new_len = try sema.analyzeArithmetic(block, .sub, end, start, src, end_src, start_src, false); const opt_new_len_val = try sema.resolveDefinedValue(block, src, new_len); @@ -28116,11 +28073,11 @@ fn analyzeSlice( else end; - try sema.panicIndexOutOfBounds(block, src, actual_end, actual_len, .cmp_lte); + try sema.panicIndexOutOfBounds(block, actual_end, actual_len, .cmp_lte); } // requirement: result[new_len] == slice_sentinel - try sema.panicSentinelMismatch(block, src, slice_sentinel, elem_ty, result, new_len); + try sema.panicSentinelMismatch(block, slice_sentinel, elem_ty, result, new_len); } return result; }; @@ -28184,11 +28141,11 @@ fn analyzeSlice( try sema.analyzeArithmetic(block, .add, end, .one, src, end_src, end_src, true) else end; - try sema.panicIndexOutOfBounds(block, src, actual_end, len_inst, .cmp_lte); + try sema.panicIndexOutOfBounds(block, actual_end, len_inst, .cmp_lte); } // requirement: start <= end - try sema.panicIndexOutOfBounds(block, src, start, end, .cmp_lte); + try sema.panicIndexOutOfBounds(block, start, end, .cmp_lte); } const result = try block.addInst(.{ .tag = .slice, @@ -28202,7 +28159,7 @@ fn analyzeSlice( }); if (block.wantSafety()) { // requirement: result[new_len] == slice_sentinel - try sema.panicSentinelMismatch(block, src, slice_sentinel, elem_ty, result, new_len); + try sema.panicSentinelMismatch(block, slice_sentinel, elem_ty, result, new_len); } return result; } diff --git a/test/cases/safety/bad union field access.zig b/test/cases/safety/bad union field access.zig index 0badf380fb..fbe7718c66 100644 --- a/test/cases/safety/bad union field access.zig +++ b/test/cases/safety/bad union field access.zig @@ -2,7 +2,7 @@ const std = @import("std"); pub fn panic(message: []const u8, stack_trace: ?*std.builtin.StackTrace, _: ?usize) noreturn { _ = stack_trace; - if (std.mem.eql(u8, message, "access of inactive union field")) { + if (std.mem.eql(u8, message, "access of union field 'float' while field 'int' is active")) { std.process.exit(0); } std.process.exit(1); |
