Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

llvm: Fix line number information to prevent spurious breakpoint hits #20543

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+69 −20
Open

llvm: Fix line number information to prevent spurious breakpoint hits #20543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions src/codegen/llvm.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5209,13 +5209,25 @@ pub const FuncGen = struct {
const old_inlined = self.inlined;
const old_base_line = self.base_line;
const old_scope = self.scope;
defer if (maybe_inline_func) |_| {
self.wip.debug_location = self.inlined;
self.file = old_file;
self.inlined = old_inlined;
self.base_line = old_base_line;
};
defer self.scope = old_scope;
const old_line = self.prev_dbg_line;
const old_column = self.prev_dbg_column;

defer {
if (maybe_inline_func) |_| {
self.wip.debug_location = self.inlined;
self.file = old_file;
self.inlined = old_inlined;
self.base_line = old_base_line;
}
self.scope = old_scope;
// In some conditions, such as on the exit branch of a loop
// header, the body is not immediately followed by a
// dbg_stmt. In this case, it makes more sense to associate
// the following instructions with the statement before the
// scope than with last statement inside the scope.
self.prev_dbg_line = old_line;
self.prev_dbg_column = old_column;
}

if (maybe_inline_func) |inline_func| {
const o = self.ng.object;
Expand Down Expand Up @@ -5281,7 +5293,11 @@ pub const FuncGen = struct {
.no_location => {},
}
defer switch (self.wip.debug_location) {
.location => |*l| l.scope = old_scope,
.location => |*l| {
l.line = old_line;
l.column = old_column;
l.scope = old_scope;
},
.no_location => {},
};

Expand Down Expand Up @@ -6004,7 +6020,13 @@ pub const FuncGen = struct {
var breaks: BreakList = if (have_block_result) .{ .list = .{} } else .{ .len = 0 };
defer if (have_block_result) breaks.list.deinit(self.gpa);

const parent_bb = try self.wip.block(0, "Block");
// GDB is sensitive to the ordering of blocks. In a loop, for
// example, the update step will go into a parent_bb; if that
// block were placed here, its instructions would immediately
// follow the loop header and be covered by the same line
// number info. GDB would then never stop on the loop header
// while trying to step by line.
const parent_bb = try self.wip.unplacedBlock(0, "Block");
try self.blocks.putNoClobber(self.gpa, inst, .{
.parent_bb = parent_bb,
.breaks = &breaks,
Expand All @@ -6013,6 +6035,7 @@ pub const FuncGen = struct {

try self.genBodyDebugScope(maybe_inline_func, body, .none);

try self.wip.placeBlock(parent_bb);
self.wip.cursor = .{ .block = parent_bb };

// Create a phi node only if the block returns a value.
Expand Down
48 changes: 37 additions & 11 deletions src/codegen/llvm/Builder.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4065,6 +4065,8 @@ pub const Function = struct {

pub const Block = struct {
instruction: Instruction.Index,
// Reverses the block_order mapping.
remapped_index: u32,

pub const Index = WipFunction.Block.Index;
};
Expand Down Expand Up @@ -5010,6 +5012,10 @@ pub const Function = struct {
return argument_index.toValue();
}

fn remapBlock(self: @This(), blk: Block.Index) u32 {
return self.blocks[@intFromEnum(blk)].remapped_index;
}

const ExtraDataTrail = struct {
index: Instruction.ExtraIndex,

Expand Down Expand Up @@ -5097,6 +5103,7 @@ pub const WipFunction = struct {
debug_location: DebugLocation,
cursor: Cursor,
blocks: std.ArrayListUnmanaged(Block),
block_order: std.ArrayListUnmanaged(u32),
instructions: std.MultiArrayList(Instruction),
names: std.ArrayListUnmanaged(String),
strip: bool,
Expand Down Expand Up @@ -5143,6 +5150,7 @@ pub const WipFunction = struct {
.debug_location = .no_location,
.cursor = undefined,
.blocks = .{},
.block_order = .{},
.instructions = .{},
.names = .{},
.strip = options.strip,
Expand Down Expand Up @@ -5178,6 +5186,13 @@ pub const WipFunction = struct {
}

pub fn block(self: *WipFunction, incoming: u32, name: []const u8) Allocator.Error!Block.Index {
try self.block_order.ensureUnusedCapacity(self.builder.gpa, 1);
const res = try self.unplacedBlock(incoming, name);
self.block_order.appendAssumeCapacity(@intFromEnum(res));
return res;
}

pub fn unplacedBlock(self: *WipFunction, incoming: u32, name: []const u8) Allocator.Error!Block.Index {
try self.blocks.ensureUnusedCapacity(self.builder.gpa, 1);

const index: Block.Index = @enumFromInt(self.blocks.items.len);
Expand All @@ -5190,6 +5205,10 @@ pub const WipFunction = struct {
return index;
}

pub fn placeBlock(self: *WipFunction, blk: Block.Index) Allocator.Error!void {
try self.block_order.append(self.builder.gpa, @intFromEnum(blk));
}

pub fn ret(self: *WipFunction, val: Value) Allocator.Error!Instruction.Index {
assert(val.typeOfWip(self) == self.function.typeOf(self.builder).functionReturn(self.builder));
try self.ensureUnusedExtraCapacity(1, NoExtra, 0);
Expand Down Expand Up @@ -6112,6 +6131,7 @@ pub const WipFunction = struct {
const function = self.function.ptr(self.builder);
const params_len = self.function.typeOf(self.builder).functionParameters(self.builder).len;
const final_instructions_len = self.blocks.items.len + self.instructions.len;
assert(self.blocks.items.len == self.block_order.items.len); // All blocks should be placed once.

const blocks = try gpa.alloc(Function.Block, self.blocks.items.len);
errdefer gpa.free(blocks);
Expand Down Expand Up @@ -6215,15 +6235,19 @@ pub const WipFunction = struct {
instructions.items[param_index] = final_instruction_index;
final_instruction_index = @enumFromInt(@intFromEnum(final_instruction_index) + 1);
}
for (blocks, self.blocks.items) |*final_block, current_block| {
for (self.block_order.items) |i| {
const current_block = self.blocks.items[i];
assert(current_block.incoming == current_block.branches);
final_block.instruction = final_instruction_index;
blocks[i].instruction = final_instruction_index;
final_instruction_index = @enumFromInt(@intFromEnum(final_instruction_index) + 1);
for (current_block.instructions.items) |instruction| {
instructions.items[@intFromEnum(instruction)] = final_instruction_index;
final_instruction_index = @enumFromInt(@intFromEnum(final_instruction_index) + 1);
}
}
for (self.block_order.items, 0..) |blk_idx, llvm_idx| {
blocks[blk_idx].remapped_index = @intCast(llvm_idx);
}
}

var wip_name: struct {
Expand Down Expand Up @@ -6290,7 +6314,8 @@ pub const WipFunction = struct {
debug_values[index] = new_argument_index;
}
}
for (self.blocks.items) |current_block| {
for (self.block_order.items) |i| {
const current_block = self.blocks.items[i];
const new_block_index: Instruction.Index = @enumFromInt(function.instructions.len);
value_indices[function.instructions.len] = value_index;
function.instructions.appendAssumeCapacity(.{
Expand Down Expand Up @@ -6636,6 +6661,7 @@ pub const WipFunction = struct {
self.instructions.deinit(self.builder.gpa);
for (self.blocks.items) |*b| b.instructions.deinit(self.builder.gpa);
self.blocks.deinit(self.builder.gpa);
self.block_order.deinit(self.builder.gpa);
self.* = undefined;
}

Expand Down Expand Up @@ -13894,7 +13920,7 @@ pub fn toBitcode(self: *Builder, allocator: Allocator) bitcode_writer.Error![]co
try constants_block.writeAbbrev(Constants.BlockAddress{
.type_id = extra.function.typeOf(self),
.function = constant_adapter.getConstantIndex(extra.function.toConst(self)),
.block = @intFromEnum(extra.block),
.block = extra.function.ptrConst(self).remapBlock(extra.block),
});
},
.dso_local_equivalent,
Expand Down Expand Up @@ -14883,14 +14909,14 @@ pub fn toBitcode(self: *Builder, allocator: Allocator) bitcode_writer.Error![]co
},
.br => {
try function_block.writeAbbrev(FunctionBlock.BrUnconditional{
.block = data,
.block = func.remapBlock(@enumFromInt(datas[instr_index])),
});
},
.br_cond => {
const extra = func.extraData(Function.Instruction.BrCond, data);
try function_block.writeAbbrev(FunctionBlock.BrConditional{
.then_block = @intFromEnum(extra.then),
.else_block = @intFromEnum(extra.@"else"),
.then_block = func.remapBlock(extra.then),
.else_block = func.remapBlock(extra.@"else"),
.condition = adapter.getOffsetValueIndex(extra.cond),
});
},
Expand All @@ -14906,13 +14932,13 @@ pub fn toBitcode(self: *Builder, allocator: Allocator) bitcode_writer.Error![]co
record.appendAssumeCapacity(adapter.getOffsetValueIndex(extra.data.val));

// Default block
record.appendAssumeCapacity(@intFromEnum(extra.data.default));
record.appendAssumeCapacity(func.remapBlock(extra.data.default));

const vals = extra.trail.next(extra.data.cases_len, Constant, &func);
const blocks = extra.trail.next(extra.data.cases_len, Function.Block.Index, &func);
for (vals, blocks) |val, block| {
record.appendAssumeCapacity(adapter.constant_adapter.getConstantIndex(val));
record.appendAssumeCapacity(@intFromEnum(block));
record.appendAssumeCapacity(func.remapBlock(block));
}

try function_block.writeUnabbrev(12, record.items);
Expand Down Expand Up @@ -14944,7 +14970,7 @@ pub fn toBitcode(self: *Builder, allocator: Allocator) bitcode_writer.Error![]co
const abs_value: u32 = @intCast(@abs(offset_value));
const signed_vbr = if (offset_value > 0) abs_value << 1 else ((abs_value << 1) | 1);
record.appendAssumeCapacity(signed_vbr);
record.appendAssumeCapacity(@intFromEnum(block));
record.appendAssumeCapacity(func.remapBlock(block));
}

if (kind == .@"phi fast") record.appendAssumeCapacity(@as(u8, @bitCast(FastMath{})));
Expand Down Expand Up @@ -15025,7 +15051,7 @@ pub fn toBitcode(self: *Builder, allocator: Allocator) bitcode_writer.Error![]co
if (name == .none or name == .empty) continue;

try value_symtab_block.writeAbbrev(ValueSymbolTable.BlockEntry{
.value_id = @intCast(block_index),
.value_id = func.remapBlock(@enumFromInt(block_index)),
.string = name.slice(self).?,
});
}
Expand Down