authorgravatar for topolarity@tapscott.meCody Tapscott <topolarity@tapscott.me> 2022-09-13 15:31:07-07:00
committergravatar for topolarity@tapscott.meCody Tapscott <topolarity@tapscott.me> 2022-10-21 10:44:20-07:00
log3007fdde45868142654d0bfa59bc0e17e5f24a1c
tree94611e7212e5050491e72204f7a231cfbfe9a71e
parent0c3a50fe1c1370c975d7de1f2f00458b4a3ec299

stage2: Pop error trace when storing error to var/const

In order to enforce a strict stack discipline for error return traces, we cannot track error return traces that are stored in variables: ```zig const x = errorable(); // errorable()'s error return trace is killed here // v-- error trace starts here instead return x catch error.UnknownError; ``` In order to propagate error return traces, function calls need to be passed directly to an error-handling expression (`if`, `catch`, `try` or `return`): ```zig // When passed directly to `catch`, the return trace is propagated return errorable() catch error.UnknownError; // Using a break also works return blk: { // code here break :blk errorable(); } catch error.UnknownError; ``` Why do we need this restriction? Without it, multiple errors can co-exist with their own error traces. Handling that situation correctly means either: a. Dynamically allocating trace memory and tracking lifetimes, OR b. Allowing the production of one error to interfere with the trace of another (which is the current status quo) This is piece (3/3) of https://github.com/ziglang/zig/issues/1923#issuecomment-1218495574

5 files changed, 147 insertions(+), 42 deletions(-)

lib/test_runner.zig+18-17
......@@ -44,23 +44,24 @@ pub fn main() void {
4444 if (!have_tty) {
4545 std.debug.print("{d}/{d} {s}... ", .{ i + 1, test_fn_list.len, test_fn.name });
4646 }
47 const result = if (test_fn.async_frame_size) |size| switch (io_mode) {
48 .evented => blk: {
49 if (async_frame_buffer.len < size) {
50 std.heap.page_allocator.free(async_frame_buffer);
51 async_frame_buffer = std.heap.page_allocator.alignedAlloc(u8, std.Target.stack_align, size) catch @panic("out of memory");
52 }
53 const casted_fn = @ptrCast(fn () callconv(.Async) anyerror!void, test_fn.func);
54 break :blk await @asyncCall(async_frame_buffer, {}, casted_fn, .{});
55 },
56 .blocking => {
57 skip_count += 1;
58 test_node.end();
59 progress.log("SKIP (async test)\n", .{});
60 continue;
61 },
62 } else test_fn.func();
63 if (result) |_| {
47 if (result: {
48 if (test_fn.async_frame_size) |size| switch (io_mode) {
49 .evented => {
50 if (async_frame_buffer.len < size) {
51 std.heap.page_allocator.free(async_frame_buffer);
52 async_frame_buffer = std.heap.page_allocator.alignedAlloc(u8, std.Target.stack_align, size) catch @panic("out of memory");
53 }
54 const casted_fn = @ptrCast(fn () callconv(.Async) anyerror!void, test_fn.func);
55 break :result await @asyncCall(async_frame_buffer, {}, casted_fn, .{});
56 },
57 .blocking => {
58 skip_count += 1;
59 test_node.end();
60 progress.log("SKIP (async test)\n", .{});
61 continue;
62 },
63 } else break :result test_fn.func();
64 }) |_| {
6465 ok_count += 1;
6566 test_node.end();
6667 if (!have_tty) std.debug.print("OK\n", .{});
src/AstGen.zig+13
......@@ -8545,9 +8545,22 @@ fn callExpr(
85458545 scratch_index += 1;
85468546 }
85478547
8548 // If our result location is a try/catch/error-union-if/return, the error trace propagates.
8549 // Otherwise, it should always be popped (handled in Sema).
8550 const propagate_error_trace = switch (rl) {
8551 .catch_none, .catch_ref => true, // Propagate to try/catch/error-union-if
8552 .ptr, .ty => |ref| b: { // Otherwise, propagate if result loc is a return
8553 const inst = refToIndex(ref) orelse break :b false;
8554 const zir_tags = astgen.instructions.items(.tag);
8555 break :b zir_tags[inst] == .ret_ptr or zir_tags[inst] == .ret_type;
8556 },
8557 else => false,
8558 };
8559
85488560 const payload_index = try addExtra(astgen, Zir.Inst.Call{
85498561 .callee = callee,
85508562 .flags = .{
8563 .pop_error_return_trace = !propagate_error_trace,
85518564 .packed_modifier = @intCast(Zir.Inst.Call.Flags.PackedModifier, @enumToInt(modifier)),
85528565 .args_len = @intCast(Zir.Inst.Call.Flags.PackedArgsLen, call.ast.params.len),
85538566 },
src/Sema.zig+62-24
......@@ -5664,6 +5664,7 @@ fn zirCall(
56645664
56655665 const modifier = @intToEnum(std.builtin.CallOptions.Modifier, extra.data.flags.packed_modifier);
56665666 const ensure_result_used = extra.data.flags.ensure_result_used;
5667 const pop_error_return_trace = extra.data.flags.pop_error_return_trace;
56675668
56685669 var func = try sema.resolveInst(extra.data.callee);
56695670 var resolved_args: []Air.Inst.Ref = undefined;
......@@ -5771,7 +5772,7 @@ fn zirCall(
57715772 resolved_args[arg_index] = resolved;
57725773 }
57735774
5774 return sema.analyzeCall(block, func, func_src, call_src, modifier, ensure_result_used, resolved_args, bound_arg_src);
5775 return sema.analyzeCall(block, func, func_src, call_src, modifier, ensure_result_used, pop_error_return_trace, resolved_args, bound_arg_src);
57755776}
57765777
57775778const GenericCallAdapter = struct {
......@@ -5883,6 +5884,7 @@ fn analyzeCall(
58835884 call_src: LazySrcLoc,
58845885 modifier: std.builtin.CallOptions.Modifier,
58855886 ensure_result_used: bool,
5887 pop_error_return_trace: bool,
58865888 uncasted_args: []const Air.Inst.Ref,
58875889 bound_arg_src: ?LazySrcLoc,
58885890) CompileError!Air.Inst.Ref {
......@@ -6333,19 +6335,55 @@ fn analyzeCall(
63336335 sema.owner_func.?.calls_or_awaits_errorable_fn = true;
63346336 }
63356337
6336 try sema.air_extra.ensureUnusedCapacity(gpa, @typeInfo(Air.Call).Struct.fields.len +
6337 args.len);
6338 const func_inst = try block.addInst(.{
6339 .tag = call_tag,
6340 .data = .{ .pl_op = .{
6341 .operand = func,
6342 .payload = sema.addExtraAssumeCapacity(Air.Call{
6343 .args_len = @intCast(u32, args.len),
6344 }),
6345 } },
6346 });
6347 sema.appendRefsAssumeCapacity(args);
6348 break :res func_inst;
6338 const backend_supports_error_return_tracing = sema.mod.comp.bin_file.options.use_llvm;
6339 const emit_error_trace_save_restore = sema.mod.comp.bin_file.options.error_return_tracing and
6340 backend_supports_error_return_tracing and
6341 pop_error_return_trace and func_ty_info.return_type.isError();
6342
6343 if (emit_error_trace_save_restore) {
6344 // This function call is error-able (and so can generate an error trace), but AstGen determined
6345 // that its result does not go to an error-handling operator (try/catch/return etc.). We need to
6346 // save and restore the error trace index here, effectively "popping" the new entries immediately.
6347
6348 const unresolved_stack_trace_ty = try sema.getBuiltinType(block, call_src, "StackTrace");
6349 const stack_trace_ty = try sema.resolveTypeFields(block, call_src, unresolved_stack_trace_ty);
6350 const ptr_stack_trace_ty = try Type.Tag.single_mut_pointer.create(sema.arena, stack_trace_ty);
6351 const err_return_trace = try block.addTy(.err_return_trace, ptr_stack_trace_ty);
6352 const field_ptr = try sema.structFieldPtr(block, call_src, err_return_trace, "index", call_src, stack_trace_ty, true);
6353
6354 const saved_index = try sema.analyzeLoad(block, call_src, field_ptr, call_src);
6355
6356 try sema.air_extra.ensureUnusedCapacity(gpa, @typeInfo(Air.Call).Struct.fields.len +
6357 args.len);
6358 const func_inst = try block.addInst(.{
6359 .tag = call_tag,
6360 .data = .{ .pl_op = .{
6361 .operand = func,
6362 .payload = sema.addExtraAssumeCapacity(Air.Call{
6363 .args_len = @intCast(u32, args.len),
6364 }),
6365 } },
6366 });
6367 sema.appendRefsAssumeCapacity(args);
6368
6369 try sema.storePtr2(block, call_src, field_ptr, call_src, saved_index, call_src, .store);
6370
6371 break :res func_inst;
6372 } else {
6373 try sema.air_extra.ensureUnusedCapacity(gpa, @typeInfo(Air.Call).Struct.fields.len +
6374 args.len);
6375 const func_inst = try block.addInst(.{
6376 .tag = call_tag,
6377 .data = .{ .pl_op = .{
6378 .operand = func,
6379 .payload = sema.addExtraAssumeCapacity(Air.Call{
6380 .args_len = @intCast(u32, args.len),
6381 }),
6382 } },
6383 });
6384 sema.appendRefsAssumeCapacity(args);
6385 break :res func_inst;
6386 }
63496387 };
63506388
63516389 if (ensure_result_used) {
......@@ -10927,7 +10965,7 @@ fn maybeErrorUnwrap(sema: *Sema, block: *Block, body: []const Zir.Inst.Index, op
1092710965 const panic_fn = try sema.getBuiltin(block, src, "panicUnwrapError");
1092810966 const err_return_trace = try sema.getErrorReturnTrace(block, src);
1092910967 const args: [2]Air.Inst.Ref = .{ err_return_trace, operand };
10930 _ = try sema.analyzeCall(block, panic_fn, src, src, .auto, false, &args, null);
10968 _ = try sema.analyzeCall(block, panic_fn, src, src, .auto, false, false, &args, null);
1093110969 return true;
1093210970 },
1093310971 .panic => {
......@@ -10938,7 +10976,7 @@ fn maybeErrorUnwrap(sema: *Sema, block: *Block, body: []const Zir.Inst.Index, op
1093810976 const panic_fn = try sema.getBuiltin(block, src, "panic");
1093910977 const err_return_trace = try sema.getErrorReturnTrace(block, src);
1094010978 const args: [3]Air.Inst.Ref = .{ msg_inst, err_return_trace, .null_value };
10941 _ = try sema.analyzeCall(block, panic_fn, src, src, .auto, false, &args, null);
10979 _ = try sema.analyzeCall(block, panic_fn, src, src, .auto, false, false, &args, null);
1094210980 return true;
1094310981 },
1094410982 else => unreachable,
......@@ -16141,7 +16179,7 @@ fn retWithErrTracing(
1614116179 const args: [1]Air.Inst.Ref = .{err_return_trace};
1614216180
1614316181 if (!need_check) {
16144 _ = try sema.analyzeCall(block, return_err_fn, src, src, .never_inline, false, &args, null);
16182 _ = try sema.analyzeCall(block, return_err_fn, src, src, .never_inline, false, false, &args, null);
1614516183 _ = try block.addUnOp(ret_tag, operand);
1614616184 return always_noreturn;
1614716185 }
......@@ -16152,7 +16190,7 @@ fn retWithErrTracing(
1615216190
1615316191 var else_block = block.makeSubBlock();
1615416192 defer else_block.instructions.deinit(gpa);
16155 _ = try sema.analyzeCall(&else_block, return_err_fn, src, src, .never_inline, false, &args, null);
16193 _ = try sema.analyzeCall(&else_block, return_err_fn, src, src, .never_inline, false, false, &args, null);
1615616194 _ = try else_block.addUnOp(ret_tag, operand);
1615716195
1615816196 try sema.air_extra.ensureUnusedCapacity(gpa, @typeInfo(Air.CondBr).Struct.fields.len +
......@@ -20369,7 +20407,7 @@ fn zirBuiltinCall(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError
2036920407 }
2037020408 }
2037120409 const ensure_result_used = extra.flags.ensure_result_used;
20372 return sema.analyzeCall(block, func, func_src, call_src, modifier, ensure_result_used, resolved_args, bound_arg_src);
20410 return sema.analyzeCall(block, func, func_src, call_src, modifier, ensure_result_used, false, resolved_args, bound_arg_src);
2037320411}
2037420412
2037520413fn zirFieldParentPtr(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Inst.Ref {
......@@ -21803,7 +21841,7 @@ fn panicWithMsg(
2180321841 Value.@"null",
2180421842 );
2180521843 const args: [3]Air.Inst.Ref = .{ msg_inst, null_stack_trace, .null_value };
21806 _ = try sema.analyzeCall(block, panic_fn, src, src, .auto, false, &args, null);
21844 _ = try sema.analyzeCall(block, panic_fn, src, src, .auto, false, false, &args, null);
2180721845 return always_noreturn;
2180821846}
2180921847
......@@ -21844,7 +21882,7 @@ fn panicUnwrapError(
2184421882 const err = try fail_block.addTyOp(unwrap_err_tag, Type.anyerror, operand);
2184521883 const err_return_trace = try sema.getErrorReturnTrace(&fail_block, src);
2184621884 const args: [2]Air.Inst.Ref = .{ err_return_trace, err };
21847 _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, &args, null);
21885 _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, false, &args, null);
2184821886 }
2184921887 }
2185021888 try sema.addSafetyCheckExtra(parent_block, ok, &fail_block);
......@@ -21885,7 +21923,7 @@ fn panicIndexOutOfBounds(
2188521923 } else {
2188621924 const panic_fn = try sema.getBuiltin(&fail_block, src, "panicOutOfBounds");
2188721925 const args: [2]Air.Inst.Ref = .{ index, len };
21888 _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, &args, null);
21926 _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, false, &args, null);
2188921927 }
2189021928 }
2189121929 try sema.addSafetyCheckExtra(parent_block, ok, &fail_block);
......@@ -21927,7 +21965,7 @@ fn panicSentinelMismatch(
2192721965 else {
2192821966 const panic_fn = try sema.getBuiltin(parent_block, src, "checkNonScalarSentinel");
2192921967 const args: [2]Air.Inst.Ref = .{ expected_sentinel, actual_sentinel };
21930 _ = try sema.analyzeCall(parent_block, panic_fn, src, src, .auto, false, &args, null);
21968 _ = try sema.analyzeCall(parent_block, panic_fn, src, src, .auto, false, false, &args, null);
2193121969 return;
2193221970 };
2193321971 const gpa = sema.gpa;
......@@ -21956,7 +21994,7 @@ fn panicSentinelMismatch(
2195621994 } else {
2195721995 const panic_fn = try sema.getBuiltin(&fail_block, src, "panicSentinelMismatch");
2195821996 const args: [2]Air.Inst.Ref = .{ expected_sentinel, actual_sentinel };
21959 _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, &args, null);
21997 _ = try sema.analyzeCall(&fail_block, panic_fn, src, src, .auto, false, false, &args, null);
2196021998 }
2196121999 }
2196222000 try sema.addSafetyCheckExtra(parent_block, ok, &fail_block);
src/Zir.zig+2-1
......@@ -2825,10 +2825,11 @@ pub const Inst = struct {
28252825 pub const Flags = packed struct {
28262826 /// std.builtin.CallOptions.Modifier in packed form
28272827 pub const PackedModifier = u3;
2828 pub const PackedArgsLen = u28;
2828 pub const PackedArgsLen = u27;
28292829
28302830 packed_modifier: PackedModifier,
28312831 ensure_result_used: bool = false,
2832 pop_error_return_trace: bool,
28322833 args_len: PackedArgsLen,
28332834
28342835 comptime {
test/stack_traces.zig+52
......@@ -208,6 +208,58 @@ pub fn addCases(cases: *tests.StackTracesContext) void {
208208 },
209209 });
210210
211 cases.addCase(.{
212 .name = "stored errors do not contribute to error trace",
213 .source =
214 \\fn foo() !void {
215 \\ return error.TheSkyIsFalling;
216 \\}
217 \\
218 \\pub fn main() !void {
219 \\ // Once an error is stored in a variable, it is popped from the trace
220 \\ var x = foo();
221 \\ x = {};
222 \\
223 \\ // As a result, this error trace will still be clean
224 \\ return error.SomethingUnrelatedWentWrong;
225 \\}
226 ,
227 .Debug = .{
228 .expect =
229 \\error: SomethingUnrelatedWentWrong
230 \\source.zig:11:5: [address] in main (test)
231 \\ return error.SomethingUnrelatedWentWrong;
232 \\ ^
233 \\
234 ,
235 },
236 .ReleaseSafe = .{
237 .exclude_os = .{
238 .windows, // TODO
239 .linux, // defeated by aggressive inlining
240 },
241 .expect =
242 \\error: SomethingUnrelatedWentWrong
243 \\source.zig:11:5: [address] in [function]
244 \\ return error.SomethingUnrelatedWentWrong;
245 \\ ^
246 \\
247 ,
248 },
249 .ReleaseFast = .{
250 .expect =
251 \\error: SomethingUnrelatedWentWrong
252 \\
253 ,
254 },
255 .ReleaseSmall = .{
256 .expect =
257 \\error: SomethingUnrelatedWentWrong
258 \\
259 ,
260 },
261 });
262
211263 cases.addCase(.{
212264 .name = "try return from within catch",
213265 .source =