From 0a7f3be42e96361ab8a9a567a11782fb81ea17da Mon Sep 17 00:00:00 2001 From: Veikka Tuominen Date: Sat, 23 Apr 2022 11:25:06 +0300 Subject: [PATCH] Sema: improve index out of bounds panic message --- lib/std/builtin.zig | 6 +++ src/Sema.zig | 110 ++++++++++++++++++++++---------------------- 2 files changed, 60 insertions(+), 56 deletions(-) diff --git a/lib/std/builtin.zig b/lib/std/builtin.zig index 02aa7239f5..894103707c 100644 --- a/lib/std/builtin.zig +++ b/lib/std/builtin.zig @@ -847,9 +847,15 @@ pub fn default_panic(msg: []const u8, error_return_trace: ?*StackTrace) noreturn } pub fn panicUnwrapError(st: ?*StackTrace, err: anyerror) noreturn { + @setCold(true); std.debug.panicExtra(st, "attempt to unwrap error: {s}", .{@errorName(err)}); } +pub fn panicOutOfBounds(index: usize, len: usize) noreturn { + @setCold(true); + std.debug.panic("attempt to index out of bound: index {d}, len {d}", .{ index, len }); +} + pub noinline fn returnError(maybe_st: ?*StackTrace) void { @setCold(true); const st = maybe_st orelse return; diff --git a/src/Sema.zig b/src/Sema.zig index 76888834d3..c45079eaa7 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -16857,7 +16857,6 @@ pub const PanicId = enum { cast_to_null, incorrect_alignment, invalid_error_code, - index_out_of_bounds, cast_truncated_data, integer_overflow, shl_overflow, @@ -16886,6 +16885,17 @@ fn addSafetyCheck( _ = try sema.safetyPanic(&fail_block, .unneeded, panic_id); + try sema.addSafetyCheckExtra(parent_block, ok, &fail_block); +} + +fn addSafetyCheckExtra( + sema: *Sema, + parent_block: *Block, + ok: Air.Inst.Ref, + fail_block: *Block, +) !void { + const gpa = sema.gpa; + try parent_block.instructions.ensureUnusedCapacity(gpa, 1); try sema.air_extra.ensureUnusedCapacity(gpa, @typeInfo(Air.Block).Struct.fields.len + @@ -16994,7 +17004,6 @@ fn panicUnwrapError( { const this_feature_is_implemented_in_the_backend = - sema.mod.comp.bin_file.options.object_format == .c or sema.mod.comp.bin_file.options.use_llvm; if (!this_feature_is_implemented_in_the_backend) { @@ -17009,52 +17018,48 @@ fn panicUnwrapError( _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, &args); } } + try sema.addSafetyCheckExtra(parent_block, ok, &fail_block); +} - try parent_block.instructions.ensureUnusedCapacity(gpa, 1); +fn panicIndexOutOfBounds( + sema: *Sema, + parent_block: *Block, + src: LazySrcLoc, + index: Air.Inst.Ref, + len: Air.Inst.Ref, + cmp_op: Air.Inst.Tag, +) !void { + const ok = try parent_block.addBinOp(cmp_op, index, len); + const gpa = sema.gpa; - try sema.air_extra.ensureUnusedCapacity(gpa, @typeInfo(Air.Block).Struct.fields.len + - 1 + // The main block only needs space for the cond_br. - @typeInfo(Air.CondBr).Struct.fields.len + - 1 + // The ok branch of the cond_br only needs space for the br. - fail_block.instructions.items.len); + 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 = parent_block.is_comptime, + }; - try sema.air_instructions.ensureUnusedCapacity(gpa, 3); - const block_inst = @intCast(Air.Inst.Index, sema.air_instructions.len); - const cond_br_inst = block_inst + 1; - const br_inst = cond_br_inst + 1; - sema.air_instructions.appendAssumeCapacity(.{ - .tag = .block, - .data = .{ .ty_pl = .{ - .ty = .void_type, - .payload = sema.addExtraAssumeCapacity(Air.Block{ - .body_len = 1, - }), - } }, - }); - sema.air_extra.appendAssumeCapacity(cond_br_inst); + defer fail_block.instructions.deinit(gpa); - sema.air_instructions.appendAssumeCapacity(.{ - .tag = .cond_br, - .data = .{ .pl_op = .{ - .operand = ok, - .payload = sema.addExtraAssumeCapacity(Air.CondBr{ - .then_body_len = 1, - .else_body_len = @intCast(u32, fail_block.instructions.items.len), - }), - } }, - }); - sema.air_extra.appendAssumeCapacity(br_inst); - sema.air_extra.appendSliceAssumeCapacity(fail_block.instructions.items); + { + const this_feature_is_implemented_in_the_backend = + sema.mod.comp.bin_file.options.use_llvm; - sema.air_instructions.appendAssumeCapacity(.{ - .tag = .br, - .data = .{ .br = .{ - .block_inst = block_inst, - .operand = .void_value, - } }, - }); - - parent_block.instructions.appendAssumeCapacity(block_inst); + 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(&fail_block, src, "panicOutOfBounds"); + const args: [2]Air.Inst.Ref = .{ index, len }; + _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, &args); + } + } + try sema.addSafetyCheckExtra(parent_block, ok, &fail_block); } fn safetyPanic( @@ -17069,7 +17074,6 @@ fn safetyPanic( .cast_to_null => "cast causes pointer to be null", .incorrect_alignment => "incorrect alignment", .invalid_error_code => "invalid error code", - .index_out_of_bounds => "attempt to index out of bounds", .cast_truncated_data => "integer cast truncated bits", .integer_overflow => "integer overflow", .shl_overflow => "left shift overflowed bits", @@ -18261,8 +18265,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) .cmp_lte else .cmp_lt; - const is_in_bounds = try block.addBinOp(cmp_op, elem_index, len_inst); - try sema.addSafetyCheck(block, is_in_bounds, .index_out_of_bounds); + try sema.panicIndexOutOfBounds(block, elem_index_src, elem_index, len_inst, cmp_op); } } return block.addBinOp(.array_elem_val, array, elem_index); @@ -18317,8 +18320,7 @@ fn elemPtrArray( if (maybe_index_val == 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; - const is_in_bounds = try block.addBinOp(cmp_op, elem_index, len_inst); - try sema.addSafetyCheck(block, is_in_bounds, .index_out_of_bounds); + try sema.panicIndexOutOfBounds(block, elem_index_src, elem_index, len_inst, cmp_op); } } return block.addPtrElemPtr(array_ptr, elem_index, elem_ptr_ty); @@ -18371,8 +18373,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; - const is_in_bounds = try block.addBinOp(cmp_op, elem_index, len_inst); - try sema.addSafetyCheck(block, is_in_bounds, .index_out_of_bounds); + try sema.panicIndexOutOfBounds(block, elem_index_src, elem_index, len_inst, cmp_op); } try sema.queueFullTypeResolution(sema.typeOf(slice)); return block.addBinOp(.slice_elem_val, slice, elem_index); @@ -18425,8 +18426,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; - const is_in_bounds = try block.addBinOp(cmp_op, elem_index, len_inst); - try sema.addSafetyCheck(block, is_in_bounds, .index_out_of_bounds); + try sema.panicIndexOutOfBounds(block, elem_index_src, elem_index, len_inst, cmp_op); } return block.addSliceElemPtr(slice, elem_index, elem_ptr_ty); } @@ -21111,13 +21111,11 @@ fn analyzeSlice( break :blk try sema.analyzeArithmetic(block, .add, slice_len_inst, .one, src, end_src, end_src); } else null; if (opt_len_inst) |len_inst| { - const end_is_in_bounds = try block.addBinOp(.cmp_lte, end, len_inst); - try sema.addSafetyCheck(block, end_is_in_bounds, .index_out_of_bounds); + try sema.panicIndexOutOfBounds(block, src, end, len_inst, .cmp_lte); } // requirement: start <= end - const start_is_in_bounds = try block.addBinOp(.cmp_lte, start, end); - try sema.addSafetyCheck(block, start_is_in_bounds, .index_out_of_bounds); + try sema.panicIndexOutOfBounds(block, src, start, end, .cmp_lte); } return block.addInst(.{ .tag = .slice,