From 627cf6ce482349c172150d058660f7a1646c2aac Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Mon, 31 Jan 2022 17:07:45 +0100 Subject: [PATCH] astgen: clean up source line calculation and management Clarify that `astgen.advanceSourceCursor` already increments absolute values of the line and columns numbers; i.e., `GenZir.calcLine` is thus not only obsolete but wrong by design. Incidentally, this clean up allows for specifying the `FnDecl` line numbers for DWARF use correctly as relative values with respect to the start of the parent `Decl`. This `Decl` in turn has its line number information specified relatively to its parent `Decl`, and so on, until we reach the global scope. --- src/AstGen.zig | 89 ++++++++++++++++++++-------------------- src/Module.zig | 4 +- src/Zir.zig | 4 +- src/arch/x86_64/Emit.zig | 5 ++- src/link/Elf.zig | 21 ++++++++-- 5 files changed, 70 insertions(+), 53 deletions(-) diff --git a/src/AstGen.zig b/src/AstGen.zig index cb6947b7c1..4fddc0a3e2 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -27,9 +27,11 @@ string_bytes: ArrayListUnmanaged(u8) = .{}, /// to avoid starting over the line/column scan for every declaration, which /// would be O(N^2). source_offset: u32 = 0, -/// Tracks the current line of `source_offset`. +/// Tracks the corresponding line of `source_offset`. +/// This value is absolute. source_line: u32 = 0, -/// Tracks the current column of `source_offset`. +/// Tracks the corresponding column of `source_offset`. +/// This value is absolute. source_column: u32 = 0, /// Used for temporary allocations; freed after AstGen is complete. /// The resulting ZIR code has no references to anything in this arena. @@ -2511,7 +2513,7 @@ fn makeDeferScope( const token_starts = tree.tokens.items(.start); const node_start = token_starts[tree.firstToken(expr_node)]; const defer_scope = try block_arena.create(Scope.Defer); - astgen.advanceSourceCursor(tree.source, node_start); + astgen.advanceSourceCursor(node_start); defer_scope.* = .{ .base = .{ .tag = scope_tag }, @@ -2775,14 +2777,9 @@ fn emitDbgNode(gz: *GenZir, node: Ast.Node.Index) !void { if (gz.force_comptime) return; const astgen = gz.astgen; - const tree = astgen.tree; - const source = tree.source; - const token_starts = tree.tokens.items(.start); - const node_start = token_starts[tree.firstToken(node)]; - - astgen.advanceSourceCursor(source, node_start); - const line = @intCast(u32, astgen.source_line); - const column = @intCast(u32, astgen.source_column); + astgen.advanceSourceCursorToNode(node); + const line = astgen.source_line - gz.decl_line; + const column = astgen.source_column; _ = try gz.add(.{ .tag = .dbg_stmt, .data = .{ .dbg_stmt = .{ @@ -3188,12 +3185,13 @@ fn fnDecl( // We insert this at the beginning so that its instruction index marks the // start of the top level declaration. const block_inst = try gz.makeBlockInst(.block_inline, fn_proto.ast.proto_node); + astgen.advanceSourceCursorToNode(decl_node); var decl_gz: GenZir = .{ .force_comptime = true, .in_defer = false, .decl_node_index = fn_proto.ast.proto_node, - .decl_line = gz.calcLine(decl_node), + .decl_line = astgen.source_line, .parent = scope, .astgen = astgen, .instructions = gz.instructions, @@ -3391,11 +3389,9 @@ fn fnDecl( astgen.fn_block = &fn_gz; defer astgen.fn_block = prev_fn_block; - const token_starts = tree.tokens.items(.start); - const lbrace_start = token_starts[tree.firstToken(body_node)]; - astgen.advanceSourceCursor(tree.source, lbrace_start); - const lbrace_line = @intCast(u32, astgen.source_line); - const lbrace_column = @intCast(u32, astgen.source_column); + astgen.advanceSourceCursorToNode(body_node); + const lbrace_line = astgen.source_line - decl_gz.decl_line; + const lbrace_column = astgen.source_column; _ = try expr(&fn_gz, params_scope, .none, body_node); try checkUsed(gz, &fn_gz.base, params_scope); @@ -3465,11 +3461,12 @@ fn globalVarDecl( const name_token = var_decl.ast.mut_token + 1; const name_str_index = try astgen.identAsString(name_token); + astgen.advanceSourceCursorToNode(node); var block_scope: GenZir = .{ .parent = scope, .decl_node_index = node, - .decl_line = gz.calcLine(node), + .decl_line = astgen.source_line, .astgen = astgen, .force_comptime = true, .in_defer = false, @@ -3614,12 +3611,13 @@ fn comptimeDecl( // top-level declaration. const block_inst = try gz.makeBlockInst(.block_inline, node); wip_members.nextDecl(false, false, false, false); + astgen.advanceSourceCursorToNode(node); var decl_block: GenZir = .{ .force_comptime = true, .in_defer = false, .decl_node_index = node, - .decl_line = gz.calcLine(node), + .decl_line = astgen.source_line, .parent = scope, .astgen = astgen, .instructions = gz.instructions, @@ -3668,12 +3666,13 @@ fn usingnamespaceDecl( // top-level declaration. const block_inst = try gz.makeBlockInst(.block_inline, node); wip_members.nextDecl(is_pub, true, false, false); + astgen.advanceSourceCursorToNode(node); var decl_block: GenZir = .{ .force_comptime = true, .in_defer = false, .decl_node_index = node, - .decl_line = gz.calcLine(node), + .decl_line = astgen.source_line, .parent = scope, .astgen = astgen, .instructions = gz.instructions, @@ -3715,12 +3714,13 @@ fn testDecl( const block_inst = try gz.makeBlockInst(.block_inline, node); wip_members.nextDecl(false, false, false, false); + astgen.advanceSourceCursorToNode(node); var decl_block: GenZir = .{ .force_comptime = true, .in_defer = false, .decl_node_index = node, - .decl_line = gz.calcLine(node), + .decl_line = astgen.source_line, .parent = scope, .astgen = astgen, .instructions = gz.instructions, @@ -3756,11 +3756,9 @@ fn testDecl( astgen.fn_block = &fn_block; defer astgen.fn_block = prev_fn_block; - const token_starts = tree.tokens.items(.start); - const lbrace_start = token_starts[tree.firstToken(body_node)]; - astgen.advanceSourceCursor(tree.source, lbrace_start); - const lbrace_line = @intCast(u32, astgen.source_line); - const lbrace_column = @intCast(u32, astgen.source_column); + astgen.advanceSourceCursorToNode(body_node); + const lbrace_line = astgen.source_line - decl_block.decl_line; + const lbrace_column = astgen.source_column; const block_result = try expr(&fn_block, &fn_block.base, .none, body_node); if (fn_block.isEmpty() or !fn_block.refIsNoReturn(block_result)) { @@ -3841,10 +3839,11 @@ fn structDeclInner( // The struct_decl instruction introduces a scope in which the decls of the struct // are in scope, so that field types, alignments, and default value expressions // can refer to decls within the struct itself. + astgen.advanceSourceCursorToNode(node); var block_scope: GenZir = .{ .parent = &namespace.base, .decl_node_index = node, - .decl_line = gz.calcLine(node), + .decl_line = astgen.source_line, .astgen = astgen, .force_comptime = true, .in_defer = false, @@ -3966,10 +3965,11 @@ fn unionDeclInner( // The union_decl instruction introduces a scope in which the decls of the union // are in scope, so that field types, alignments, and default value expressions // can refer to decls within the union itself. + astgen.advanceSourceCursorToNode(node); var block_scope: GenZir = .{ .parent = &namespace.base, .decl_node_index = node, - .decl_line = gz.calcLine(node), + .decl_line = astgen.source_line, .astgen = astgen, .force_comptime = true, .in_defer = false, @@ -4249,10 +4249,11 @@ fn containerDecl( // The enum_decl instruction introduces a scope in which the decls of the enum // are in scope, so that tag values can refer to decls within the enum itself. + astgen.advanceSourceCursorToNode(node); var block_scope: GenZir = .{ .parent = &namespace.base, .decl_node_index = node, - .decl_line = gz.calcLine(node), + .decl_line = astgen.source_line, .astgen = astgen, .force_comptime = true, .in_defer = false, @@ -6980,7 +6981,7 @@ fn builtinCall( const token_starts = tree.tokens.items(.start); const node_start = token_starts[tree.firstToken(node)]; - astgen.advanceSourceCursor(tree.source, node_start); + astgen.advanceSourceCursor(node_start); const result = try gz.addExtendedPayload(.builtin_src, Zir.Inst.LineColumn{ .line = @intCast(u32, astgen.source_line), @@ -9560,18 +9561,6 @@ const GenZir = struct { return false; } - fn calcLine(gz: GenZir, node: Ast.Node.Index) u32 { - const astgen = gz.astgen; - const tree = astgen.tree; - const source = tree.source; - const token_starts = tree.tokens.items(.start); - const node_start = token_starts[tree.firstToken(node)]; - - astgen.advanceSourceCursor(source, node_start); - - return @intCast(u32, gz.decl_line + astgen.source_line); - } - fn nodeIndexToRelative(gz: GenZir, node_index: Ast.Node.Index) i32 { return @bitCast(i32, node_index) - @bitCast(i32, gz.decl_node_index); } @@ -9704,8 +9693,8 @@ const GenZir = struct { assert(node_tags[fn_decl] == .fn_decl or node_tags[fn_decl] == .test_decl); const block = node_datas[fn_decl].rhs; const rbrace_start = token_starts[tree.lastToken(block)]; - astgen.advanceSourceCursor(tree.source, rbrace_start); - const rbrace_line = @intCast(u32, astgen.source_line); + astgen.advanceSourceCursor(rbrace_start); + const rbrace_line = @intCast(u32, astgen.source_line - gz.decl_line); const rbrace_column = @intCast(u32, astgen.source_column); const columns = args.lbrace_column | (rbrace_column << 16); @@ -10736,7 +10725,17 @@ fn detectLocalShadowing( }; } -fn advanceSourceCursor(astgen: *AstGen, source: []const u8, end: usize) void { +/// Advances the source cursor to the beginning of `node`. +fn advanceSourceCursorToNode(astgen: *AstGen, node: Ast.Node.Index) void { + const tree = astgen.tree; + const token_starts = tree.tokens.items(.start); + const node_start = token_starts[tree.firstToken(node)]; + astgen.advanceSourceCursor(node_start); +} + +/// Advances the source cursor to an absolute byte offset `end` in the file. +fn advanceSourceCursor(astgen: *AstGen, end: usize) void { + const source = astgen.tree.source; var i = astgen.source_offset; var line = astgen.source_line; var column = astgen.source_column; diff --git a/src/Module.zig b/src/Module.zig index 35b7efb016..e2e2505927 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -375,6 +375,7 @@ pub const Decl = struct { src_node: Ast.Node.Index, /// Line number corresponding to `src_node`. Stored separately so that source files /// do not need to be loaded into memory in order to compute debug line numbers. + /// This value is absolute. src_line: u32, /// Index to ZIR `extra` array to the entry in the parent's decl structure /// (the part that says "for every decls_len"). The first item at this index is @@ -4122,7 +4123,8 @@ fn scanDecl(iter: *ScanDeclIter, decl_sub_index: usize, flags: u4) SemaError!voi const has_linksection_or_addrspace = (flags & 0b1000) != 0; // zig fmt: on - const line = iter.parent_decl.relativeToLine(zir.extra[decl_sub_index + 4]); + const line_off = zir.extra[decl_sub_index + 4]; + const line = iter.parent_decl.relativeToLine(line_off); const decl_name_index = zir.extra[decl_sub_index + 5]; const decl_index = zir.extra[decl_sub_index + 6]; const decl_block_inst_data = zir.instructions.items(.data)[decl_index].pl_node; diff --git a/src/Zir.zig b/src/Zir.zig index 1ff103a876..dbdaa0f5fb 100644 --- a/src/Zir.zig +++ b/src/Zir.zig @@ -2309,9 +2309,9 @@ pub const Inst = struct { body_len: u32, pub const SrcLocs = struct { - /// Absolute line index in the source file. + /// Line index in the source file relative to the parent decl. lbrace_line: u32, - /// Absolute line index in the source file. + /// Line index in the source file relative to the parent decl. rbrace_line: u32, /// lbrace_column is least significant bits u16 /// rbrace_column is most significant bits u16 diff --git a/src/arch/x86_64/Emit.zig b/src/arch/x86_64/Emit.zig index aa1798c0f6..274d1e86b5 100644 --- a/src/arch/x86_64/Emit.zig +++ b/src/arch/x86_64/Emit.zig @@ -787,12 +787,14 @@ fn mirDbgLine(emit: *Emit, inst: Mir.Inst.Index) InnerError!void { assert(tag == .dbg_line); const payload = emit.mir.instructions.items(.data)[inst].payload; const dbg_line_column = emit.mir.extraData(Mir.DbgLineColumn, payload).data; + log.debug("mirDbgLine", .{}); try emit.dbgAdvancePCAndLine(dbg_line_column.line, dbg_line_column.column); } fn dbgAdvancePCAndLine(emit: *Emit, line: u32, column: u32) InnerError!void { const delta_line = @intCast(i32, line) - @intCast(i32, emit.prev_di_line); const delta_pc: usize = emit.code.items.len - emit.prev_di_pc; + log.debug(" (advance pc={d} and line={d})", .{ delta_line, delta_pc }); switch (emit.debug_output) { .dwarf => |dbg_out| { // TODO Look into using the DWARF special opcodes to compress this data. @@ -806,7 +808,6 @@ fn dbgAdvancePCAndLine(emit: *Emit, line: u32, column: u32) InnerError!void { leb128.writeILEB128(dbg_out.dbg_line.writer(), delta_line) catch unreachable; } dbg_out.dbg_line.appendAssumeCapacity(DW.LNS.copy); - emit.prev_di_pc = emit.code.items.len; emit.prev_di_line = line; emit.prev_di_column = column; emit.prev_di_pc = emit.code.items.len; @@ -856,6 +857,7 @@ fn mirDbgPrologueEnd(emit: *Emit, inst: Mir.Inst.Index) InnerError!void { switch (emit.debug_output) { .dwarf => |dbg_out| { try dbg_out.dbg_line.append(DW.LNS.set_prologue_end); + log.debug("mirDbgPrologueEnd (line={d}, col={d})", .{ emit.prev_di_line, emit.prev_di_column }); try emit.dbgAdvancePCAndLine(emit.prev_di_line, emit.prev_di_column); }, .plan9 => {}, @@ -869,6 +871,7 @@ fn mirDbgEpilogueBegin(emit: *Emit, inst: Mir.Inst.Index) InnerError!void { switch (emit.debug_output) { .dwarf => |dbg_out| { try dbg_out.dbg_line.append(DW.LNS.set_epilogue_begin); + log.debug("mirDbgEpilogueBegin (line={d}, col={d})", .{ emit.prev_di_line, emit.prev_di_column }); try emit.dbgAdvancePCAndLine(emit.prev_di_line, emit.prev_di_column); }, .plan9 => {}, diff --git a/src/link/Elf.zig b/src/link/Elf.zig index bae23a0336..71956cd5b3 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -2507,7 +2507,13 @@ pub fn updateFunc(self: *Elf, module: *Module, func: *Module.Fn, air: Air, liven defer deinitRelocs(self.base.allocator, &dbg_info_type_relocs); const decl = func.owner_decl; - const line_off = @intCast(u28, decl.src_line + func.lbrace_line); + log.debug("updateFunc {s}{*}", .{ decl.name, func.owner_decl }); + log.debug(" (decl.src_line={d}, func.lbrace_line={d}, func.rbrace_line={d})", .{ + decl.src_line, + func.lbrace_line, + func.rbrace_line, + }); + const line = @intCast(u28, decl.src_line + func.lbrace_line); const ptr_width_bytes = self.ptrWidthBytes(); dbg_line_buffer.appendSliceAssumeCapacity(&[_]u8{ @@ -2524,7 +2530,7 @@ pub fn updateFunc(self: *Elf, module: *Module, func: *Module.Fn, air: Air, liven // to this function's begin curly. assert(self.getRelocDbgLineOff() == dbg_line_buffer.items.len); // Here we use a ULEB128-fixed-4 to make sure this field can be overwritten later. - leb128.writeUnsignedFixed(4, dbg_line_buffer.addManyAsArrayAssumeCapacity(4), line_off); + leb128.writeUnsignedFixed(4, dbg_line_buffer.addManyAsArrayAssumeCapacity(4), line); dbg_line_buffer.appendAssumeCapacity(DW.LNS.set_file); assert(self.getRelocDbgFileIndex() == dbg_line_buffer.items.len); @@ -3070,15 +3076,22 @@ pub fn updateDeclLineNumber(self: *Elf, module: *Module, decl: *const Module.Dec const tracy = trace(@src()); defer tracy.end(); + log.debug("updateDeclLineNumber {s}{*}", .{ decl.name, decl }); + if (self.llvm_object) |_| return; const func = decl.val.castTag(.function).?.data; - const casted_line_off = @intCast(u28, decl.src_line + func.lbrace_line); + log.debug(" (decl.src_line={d}, func.lbrace_line={d}, func.rbrace_line={d})", .{ + decl.src_line, + func.lbrace_line, + func.rbrace_line, + }); + const line = @intCast(u28, decl.src_line + func.lbrace_line); const shdr = &self.sections.items[self.debug_line_section_index.?]; const file_pos = shdr.sh_offset + decl.fn_link.elf.off + self.getRelocDbgLineOff(); var data: [4]u8 = undefined; - leb128.writeUnsignedFixed(4, &data, casted_line_off); + leb128.writeUnsignedFixed(4, &data, line); try self.base.file.?.pwriteAll(&data, file_pos); }