From bac132bc8fb320620ca5ad554ce515ccb7e4dad1 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 7 Jun 2022 17:48:53 -0700 Subject: introduce std.debug.Trace And use it to debug a LazySrcLoc in stage2 that is set to a bogus value. The actual fix in this commit is: ```diff - try sema.emitBackwardBranch(&child_block, call_src); + try sema.emitBackwardBranch(block, call_src); ``` --- src/Module.zig | 93 +++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 30 deletions(-) (limited to 'src/Module.zig') diff --git a/src/Module.zig b/src/Module.zig index 12d311046a..724fa2ebc4 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -659,7 +659,7 @@ pub const Decl = struct { } pub fn nodeSrcLoc(decl: Decl, node_index: Ast.Node.Index) LazySrcLoc { - return .{ .node_offset = decl.nodeIndexToRelative(node_index) }; + return LazySrcLoc.nodeOffset(decl.nodeIndexToRelative(node_index)); } pub fn srcLoc(decl: Decl) SrcLoc { @@ -670,7 +670,7 @@ pub const Decl = struct { return .{ .file_scope = decl.getFileScope(), .parent_decl_node = decl.src_node, - .lazy = .{ .node_offset = node_offset }, + .lazy = LazySrcLoc.nodeOffset(node_offset), }; } @@ -861,7 +861,7 @@ pub const ErrorSet = struct { return .{ .file_scope = owner_decl.getFileScope(), .parent_decl_node = owner_decl.src_node, - .lazy = .{ .node_offset = self.node_offset }, + .lazy = LazySrcLoc.nodeOffset(self.node_offset), }; } @@ -947,7 +947,7 @@ pub const Struct = struct { return .{ .file_scope = owner_decl.getFileScope(), .parent_decl_node = owner_decl.src_node, - .lazy = .{ .node_offset = s.node_offset }, + .lazy = LazySrcLoc.nodeOffset(s.node_offset), }; } @@ -1066,7 +1066,7 @@ pub const EnumSimple = struct { return .{ .file_scope = owner_decl.getFileScope(), .parent_decl_node = owner_decl.src_node, - .lazy = .{ .node_offset = self.node_offset }, + .lazy = LazySrcLoc.nodeOffset(self.node_offset), }; } }; @@ -1097,7 +1097,7 @@ pub const EnumNumbered = struct { return .{ .file_scope = owner_decl.getFileScope(), .parent_decl_node = owner_decl.src_node, - .lazy = .{ .node_offset = self.node_offset }, + .lazy = LazySrcLoc.nodeOffset(self.node_offset), }; } }; @@ -1131,7 +1131,7 @@ pub const EnumFull = struct { return .{ .file_scope = owner_decl.getFileScope(), .parent_decl_node = owner_decl.src_node, - .lazy = .{ .node_offset = self.node_offset }, + .lazy = LazySrcLoc.nodeOffset(self.node_offset), }; } }; @@ -1197,7 +1197,7 @@ pub const Union = struct { return .{ .file_scope = owner_decl.getFileScope(), .parent_decl_node = owner_decl.src_node, - .lazy = .{ .node_offset = self.node_offset }, + .lazy = LazySrcLoc.nodeOffset(self.node_offset), }; } @@ -1404,7 +1404,7 @@ pub const Opaque = struct { return .{ .file_scope = owner_decl.getFileScope(), .parent_decl_node = owner_decl.src_node, - .lazy = .{ .node_offset = self.node_offset }, + .lazy = LazySrcLoc.nodeOffset(self.node_offset), }; } @@ -2105,7 +2105,17 @@ pub const SrcLoc = struct { const token_starts = tree.tokens.items(.start); return token_starts[tok_index]; }, - .node_offset, .node_offset_bin_op => |node_off| { + .node_offset => |traced_off| { + const node_off = traced_off.x; + const tree = try src_loc.file_scope.getTree(gpa); + const node = src_loc.declRelativeToNodeIndex(node_off); + assert(src_loc.file_scope.tree_loaded); + const main_tokens = tree.nodes.items(.main_token); + const tok_index = main_tokens[node]; + const token_starts = tree.tokens.items(.start); + return token_starts[tok_index]; + }, + .node_offset_bin_op => |node_off| { const tree = try src_loc.file_scope.getTree(gpa); const node = src_loc.declRelativeToNodeIndex(node_off); assert(src_loc.file_scope.tree_loaded); @@ -2515,6 +2525,17 @@ pub const SrcLoc = struct { } }; +/// This wraps a simple integer in debug builds so that later on we can find out +/// where in semantic analysis the value got set. +const TracedOffset = struct { + x: i32, + trace: Trace = trace_init, + + const want_tracing = builtin.mode == .Debug; + const trace_init = if (want_tracing) std.debug.Trace(1, 3){} else {}; + const Trace = @TypeOf(trace_init); +}; + /// Resolving a source location into a byte offset may require doing work /// that we would rather not do unless the error actually occurs. /// Therefore we need a data structure that contains the information necessary @@ -2555,7 +2576,7 @@ pub const LazySrcLoc = union(enum) { /// The source location points to an AST node, which is this value offset /// from its containing Decl node AST index. /// The Decl is determined contextually. - node_offset: i32, + node_offset: TracedOffset, /// The source location points to two tokens left of the first token of an AST node, /// which is this value offset from its containing Decl node AST index. /// The Decl is determined contextually. @@ -2705,6 +2726,18 @@ pub const LazySrcLoc = union(enum) { /// The Decl is determined contextually. node_offset_array_type_elem: i32, + pub const nodeOffset = if (TracedOffset.want_tracing) nodeOffsetDebug else nodeOffsetRelease; + + noinline fn nodeOffsetDebug(node_offset: i32) LazySrcLoc { + var result: LazySrcLoc = .{ .node_offset = .{ .x = node_offset } }; + result.node_offset.trace.addAddr(@returnAddress(), "init"); + return result; + } + + fn nodeOffsetRelease(node_offset: i32) LazySrcLoc { + return .{ .node_offset = .{ .x = node_offset } }; + } + /// Upgrade to a `SrcLoc` based on the `Decl` provided. pub fn toSrcLoc(lazy: LazySrcLoc, decl: *Decl) SrcLoc { return switch (lazy) { @@ -4014,7 +4047,7 @@ fn semaDecl(mod: *Module, decl_index: Decl.Index) !bool { const body = zir.extra[extra.end..][0..extra.data.body_len]; const result_ref = (try sema.analyzeBodyBreak(&block_scope, body)).?.operand; try wip_captures.finalize(); - const src: LazySrcLoc = .{ .node_offset = 0 }; + const src = LazySrcLoc.nodeOffset(0); const decl_tv = try sema.resolveInstValue(&block_scope, src, result_ref); const decl_align: u32 = blk: { const align_ref = decl.zirAlignRef(); @@ -5044,7 +5077,7 @@ pub fn analyzeFnBody(mod: *Module, func: *Fn, arena: Allocator) SemaError!Air { // Crucially, this happens *after* we set the function state to success above, // so that dependencies on the function body will now be satisfied rather than // result in circular dependency errors. - const src: LazySrcLoc = .{ .node_offset = 0 }; + const src = LazySrcLoc.nodeOffset(0); sema.resolveFnTypes(&inner_block, src, fn_ty_info) catch |err| switch (err) { error.NeededSourceLocation => unreachable, error.GenericPoison => unreachable, @@ -5338,7 +5371,7 @@ pub const SwitchProngSrc = union(enum) { log.warn("unable to load {s}: {s}", .{ decl.getFileScope().sub_file_path, @errorName(err), }); - return LazySrcLoc{ .node_offset = 0 }; + return LazySrcLoc.nodeOffset(0); }; const switch_node = decl.relativeToNodeIndex(switch_node_offset); const main_tokens = tree.nodes.items(.main_token); @@ -5367,17 +5400,17 @@ pub const SwitchProngSrc = union(enum) { node_tags[case.ast.values[0]] == .switch_range; switch (prong_src) { - .scalar => |i| if (!is_multi and i == scalar_i) return LazySrcLoc{ - .node_offset = decl.nodeIndexToRelative(case.ast.values[0]), - }, + .scalar => |i| if (!is_multi and i == scalar_i) return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(case.ast.values[0]), + ), .multi => |s| if (is_multi and s.prong == multi_i) { var item_i: u32 = 0; for (case.ast.values) |item_node| { if (node_tags[item_node] == .switch_range) continue; - if (item_i == s.item) return LazySrcLoc{ - .node_offset = decl.nodeIndexToRelative(item_node), - }; + if (item_i == s.item) return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(item_node), + ); item_i += 1; } else unreachable; }, @@ -5387,15 +5420,15 @@ pub const SwitchProngSrc = union(enum) { if (node_tags[range] != .switch_range) continue; if (range_i == s.item) switch (range_expand) { - .none => return LazySrcLoc{ - .node_offset = decl.nodeIndexToRelative(range), - }, - .first => return LazySrcLoc{ - .node_offset = decl.nodeIndexToRelative(node_datas[range].lhs), - }, - .last => return LazySrcLoc{ - .node_offset = decl.nodeIndexToRelative(node_datas[range].rhs), - }, + .none => return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(range), + ), + .first => return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(node_datas[range].lhs), + ), + .last => return LazySrcLoc.nodeOffset( + decl.nodeIndexToRelative(node_datas[range].rhs), + ), }; range_i += 1; } else unreachable; @@ -5450,7 +5483,7 @@ pub const PeerTypeCandidateSrc = union(enum) { log.warn("unable to load {s}: {s}", .{ decl.getFileScope().sub_file_path, @errorName(err), }); - return LazySrcLoc{ .node_offset = 0 }; + return LazySrcLoc.nodeOffset(0); }; const node = decl.relativeToNodeIndex(node_offset); const node_datas = tree.nodes.items(.data); -- cgit v1.2.3 From af909f6c93f06e409e98cb90a9896aa5216f1563 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 8 Jun 2022 12:55:24 -0700 Subject: std.debug.Trace: improve API Now `std.debug.Trace` is a concrete type with pre-chosen defaults. `std.debug.ConfigurableTrace` can be used for more advanced cases. --- lib/std/debug.zig | 37 +++++++++++++++++++++++++++++++------ src/Module.zig | 6 ++---- 2 files changed, 33 insertions(+), 10 deletions(-) (limited to 'src/Module.zig') diff --git a/lib/std/debug.zig b/lib/std/debug.zig index f4ca0f0354..ba45b16d1b 100644 --- a/lib/std/debug.zig +++ b/lib/std/debug.zig @@ -1944,19 +1944,42 @@ noinline fn showMyTrace() usize { return @returnAddress(); } -pub fn Trace(comptime size: usize, comptime stack_frame_count: usize) type { +/// This API helps you track where a value originated and where it was mutated, +/// or any other points of interest. +/// In debug mode, it adds a small size penalty (104 bytes on 64-bit architectures) +/// to the aggregate that you add it to. +/// In release mode, it is size 0 and all methods are no-ops. +/// This is a pre-made type with default settings. +/// For more advanced usage, see `ConfigurableTrace`. +pub const Trace = ConfigurableTrace(2, 4, builtin.mode == .Debug); + +pub fn ConfigurableTrace(comptime size: usize, comptime stack_frame_count: usize, comptime enabled: bool) type { return struct { - addrs: [size][stack_frame_count]usize = undefined, - notes: [size][]const u8 = undefined, - index: usize = 0, + addrs: [actual_size][stack_frame_count]usize = undefined, + notes: [actual_size][]const u8 = undefined, + index: Index = 0, - const frames_init = [1]usize{0} ** stack_frame_count; + const actual_size = if (enabled) size else 0; + const Index = if (enabled) usize else u0; - pub noinline fn add(t: *@This(), note: []const u8) void { + pub const enabled = enabled; + + pub const add = if (enabled) addNoInline else addNoOp; + + pub noinline fn addNoInline(t: *@This(), note: []const u8) void { + comptime assert(enabled); return addAddr(t, @returnAddress(), note); } + pub inline fn addNoOp(t: *@This(), note: []const u8) void { + _ = t; + _ = note; + comptime assert(!enabled); + } + pub fn addAddr(t: *@This(), addr: usize, note: []const u8) void { + if (!enabled) return; + if (t.index < size) { t.notes[t.index] = note; t.addrs[t.index] = [1]usize{0} ** stack_frame_count; @@ -1972,6 +1995,8 @@ pub fn Trace(comptime size: usize, comptime stack_frame_count: usize) type { } pub fn dump(t: @This()) void { + if (!enabled) return; + const tty_config = detectTTYConfig(); const stderr = io.getStdErr().writer(); const end = @maximum(t.index, size); diff --git a/src/Module.zig b/src/Module.zig index 724fa2ebc4..602b91a5ba 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -2529,11 +2529,9 @@ pub const SrcLoc = struct { /// where in semantic analysis the value got set. const TracedOffset = struct { x: i32, - trace: Trace = trace_init, + trace: std.debug.Trace = .{}, - const want_tracing = builtin.mode == .Debug; - const trace_init = if (want_tracing) std.debug.Trace(1, 3){} else {}; - const Trace = @TypeOf(trace_init); + const want_tracing = std.debug.Trace.enabled; }; /// Resolving a source location into a byte offset may require doing work -- cgit v1.2.3 From 2bf532fc23ec014201bb1e69cb3b44e37e23f14c Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 9 Jun 2022 15:33:04 -0700 Subject: stage2: use std.debug.Trace only when explicitly enabled Because it bumps up the stack space requirements, which is making a test case fail on aarch64 drone CI. --- build.zig | 3 +++ ci/azure/build.zig | 2 ++ src/Module.zig | 2 +- src/config.zig.in | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) (limited to 'src/Module.zig') diff --git a/build.zig b/build.zig index 3ce7cc24c2..0afbe9171a 100644 --- a/build.zig +++ b/build.zig @@ -131,6 +131,7 @@ pub fn build(b: *Builder) !void { const link_libc = b.option(bool, "force-link-libc", "Force self-hosted compiler to link libc") orelse enable_llvm; const strip = b.option(bool, "strip", "Omit debug information") orelse false; const use_zig0 = b.option(bool, "zig0", "Bootstrap using zig0") orelse false; + const value_tracing = b.option(bool, "value-tracing", "Enable extra state tracking to help troubleshoot bugs in the compiler (using the std.debug.Trace API)") orelse false; const mem_leak_frames: u32 = b.option(u32, "mem-leak-frames", "How many stack frames to print when a memory leak occurs. Tests get 2x this amount.") orelse blk: { if (strip) break :blk @as(u32, 0); @@ -353,6 +354,7 @@ pub fn build(b: *Builder) !void { exe_options.addOption(bool, "enable_tracy", tracy != null); exe_options.addOption(bool, "enable_tracy_callstack", tracy_callstack); exe_options.addOption(bool, "enable_tracy_allocation", tracy_allocation); + exe_options.addOption(bool, "value_tracing", value_tracing); exe_options.addOption(bool, "is_stage1", is_stage1); exe_options.addOption(bool, "omit_stage2", omit_stage2); if (tracy) |tracy_path| { @@ -402,6 +404,7 @@ pub fn build(b: *Builder) !void { test_cases_options.addOption(bool, "enable_rosetta", b.enable_rosetta); test_cases_options.addOption(bool, "enable_darling", b.enable_darling); test_cases_options.addOption(u32, "mem_leak_frames", mem_leak_frames * 2); + test_cases_options.addOption(bool, "value_tracing", value_tracing); test_cases_options.addOption(?[]const u8, "glibc_runtimes_dir", b.glibc_runtimes_dir); test_cases_options.addOption([:0]const u8, "version", try b.allocator.dupeZ(u8, version)); test_cases_options.addOption(std.SemanticVersion, "semver", semver); diff --git a/ci/azure/build.zig b/ci/azure/build.zig index 4596233bd8..12197fdf07 100644 --- a/ci/azure/build.zig +++ b/ci/azure/build.zig @@ -99,6 +99,7 @@ pub fn build(b: *Builder) !void { const force_gpa = b.option(bool, "force-gpa", "Force the compiler to use GeneralPurposeAllocator") orelse false; const link_libc = b.option(bool, "force-link-libc", "Force self-hosted compiler to link libc") orelse enable_llvm; const strip = b.option(bool, "strip", "Omit debug information") orelse false; + const value_tracing = b.option(bool, "value-tracing", "Enable extra state tracking to help troubleshoot bugs in the compiler (using the std.debug.Trace API)") orelse false; const mem_leak_frames: u32 = b.option(u32, "mem-leak-frames", "How many stack frames to print when a memory leak occurs. Tests get 2x this amount.") orelse blk: { if (strip) break :blk @as(u32, 0); @@ -303,6 +304,7 @@ pub fn build(b: *Builder) !void { exe_options.addOption(bool, "enable_tracy", tracy != null); exe_options.addOption(bool, "enable_tracy_callstack", tracy_callstack); exe_options.addOption(bool, "enable_tracy_allocation", tracy_allocation); + exe_options.addOption(bool, "value_tracing", value_tracing); exe_options.addOption(bool, "is_stage1", is_stage1); exe_options.addOption(bool, "omit_stage2", omit_stage2); if (tracy) |tracy_path| { diff --git a/src/Module.zig b/src/Module.zig index 602b91a5ba..f03ba77a39 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -2531,7 +2531,7 @@ const TracedOffset = struct { x: i32, trace: std.debug.Trace = .{}, - const want_tracing = std.debug.Trace.enabled; + const want_tracing = build_options.value_tracing; }; /// Resolving a source location into a byte offset may require doing work diff --git a/src/config.zig.in b/src/config.zig.in index f193fddb20..104c3ed8eb 100644 --- a/src/config.zig.in +++ b/src/config.zig.in @@ -8,6 +8,7 @@ pub const semver = @import("std").SemanticVersion.parse(version) catch unreachab pub const enable_logging: bool = @ZIG_ENABLE_LOGGING_BOOL@; pub const enable_link_snapshots: bool = false; pub const enable_tracy = false; +pub const value_tracing = false; pub const is_stage1 = true; pub const skip_non_native = false; pub const omit_stage2: bool = @ZIG_OMIT_STAGE2_BOOL@; -- cgit v1.2.3