Skip to content

Commit

Permalink
Don't leave debug locations for constants sitting on the builder inde…
Browse files Browse the repository at this point in the history
…finitely.

Because constants are currently emitted *before* the prologue, leaving the
debug location on the IRBuilder spills onto other instructions in the prologue
and messes up both line numbers as well as the point LLVM chooses to be the
prologue end.

Example LLVM IR (irrelevant IR elided):
Before:

define internal { i64, i64 } @_ZN3tmp3Foo18var_return_opt_try17he02116165b0fc08cE(ptr align 8 %self) !dbg !347 {
start:
  %self.dbg.spill = alloca [8 x i8], align 8
  %_0 = alloca [16 x i8], align 8
  %residual.dbg.spill = alloca [0 x i8], align 1
    #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357)
  store ptr %self, ptr %self.dbg.spill, align 8, !dbg !357
    #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358)

After:

define internal { i64, i64 } @_ZN3tmp3Foo18var_return_opt_try17h00b17d08874ddd90E(ptr align 8 %self) !dbg !347 {
start:
  %self.dbg.spill = alloca [8 x i8], align 8
  %_0 = alloca [16 x i8], align 8
  %residual.dbg.spill = alloca [0 x i8], align 1
    #dbg_declare(ptr %residual.dbg.spill, !353, !DIExpression(), !357)
  store ptr %self, ptr %self.dbg.spill, align 8
    #dbg_declare(ptr %self.dbg.spill, !350, !DIExpression(), !358)

Note in particular how !357 from %residual.dbg.spill's dbg_declare no longer
falls through onto the store to %self.dbg.spill. This fixes argument values
at entry when the constant is a ZST (e.g. <Option as Try>::Residual). This
fixes #130003 (but note that it does *not* fix issues with argument values and
non-ZST constants, which emit their own stores that have debug info on them,
like #128945).
  • Loading branch information
khuey committed Sep 6, 2024
1 parent 59d4114 commit 7ed9f94
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 2 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_gcc/src/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ impl<'a, 'gcc, 'tcx> DebugInfoBuilderMethods for Builder<'a, 'gcc, 'tcx> {
fn set_dbg_loc(&mut self, dbg_loc: Self::DILocation) {
self.location = Some(dbg_loc);
}

fn clear_dbg_loc(&mut self) {
self.location = None;
}
}

/// Generate the `debug_context` in an MIR Body.
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#![doc = include_str!("doc.md")]

use std::cell::{OnceCell, RefCell};
use std::iter;
use std::ops::Range;
use std::{iter, ptr};

use libc::c_uint;
use rustc_codegen_ssa::debuginfo::type_names;
Expand Down Expand Up @@ -209,6 +209,12 @@ impl<'ll> DebugInfoBuilderMethods for Builder<'_, 'll, '_> {
}
}

fn clear_dbg_loc(&mut self) {
unsafe {
llvm::LLVMSetCurrentDebugLocation2(self.llbuilder, ptr::null());
}
}

fn insert_reference_to_gdb_debug_scripts_section_global(&mut self) {
gdb::insert_reference_to_gdb_debug_scripts_section_global(self)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ unsafe extern "C" {
pub fn LLVMDisposeBuilder<'a>(Builder: &'a mut Builder<'a>);

// Metadata
pub fn LLVMSetCurrentDebugLocation2<'a>(Builder: &Builder<'a>, Loc: &'a Metadata);
pub fn LLVMSetCurrentDebugLocation2<'a>(Builder: &Builder<'a>, Loc: *const Metadata);

// Terminators
pub fn LLVMBuildRetVoid<'a>(B: &Builder<'a>) -> &'a Value;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/mir/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
self.set_debug_loc(bx, var.source_info);
let base =
Self::spill_operand_to_stack(operand, Some(var.name.to_string()), bx);
bx.clear_dbg_loc();

bx.dbg_var_addr(
dbg_var,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/traits/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ pub trait DebugInfoBuilderMethods: BackendTypes {
fragment: Option<Range<Size>>,
);
fn set_dbg_loc(&mut self, dbg_loc: Self::DILocation);
fn clear_dbg_loc(&mut self);
fn insert_reference_to_gdb_debug_scripts_section_global(&mut self);
fn set_var_name(&mut self, value: Self::Value, name: &str);
}
72 changes: 72 additions & 0 deletions tests/debuginfo/zst-interferes-with-prologue.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
//@ min-lldb-version: 310

//@ compile-flags:-g

// === GDB TESTS ===================================================================================

// gdb-command:break zst_interferes_with_prologue::Foo::var_return_opt_try
// gdb-command:run

// gdb-command:print self
// gdb-command:next
// gdb-command:print self
// gdb-command:print $1 == $2
// gdb-check:true

// === LLDB TESTS ==================================================================================

// lldb-command:b "zst_interferes_with_prologue::Foo::var_return_opt_try"
// lldb-command:run

// lldb-command:expr self
// lldb-command:next
// lldb-command:expr self
// lldb-command:print $0 == $1
// lldb-check:true

struct Foo {
a: usize,
}

impl Foo {
#[inline(never)]
fn get_a(&self) -> Option<usize> {
Some(self.a)
}

#[inline(never)]
fn var_return(&self) -> usize {
let r = self.get_a().unwrap();
r
}

#[inline(never)]
fn var_return_opt_unwrap(&self) -> Option<usize> {
let r = self.get_a().unwrap();
Some(r)
}

#[inline(never)]
fn var_return_opt_match(&self) -> Option<usize> {
let r = match self.get_a() {
None => return None,
Some(a) => a,
};
Some(r)
}

#[inline(never)]
fn var_return_opt_try(&self) -> Option<usize> {
let r = self.get_a()?;
Some(r)
}
}

fn main() {
let f1 = Foo{ a: 1 };
let f2 = Foo{ a: 1 };
f1.var_return();
f1.var_return_opt_unwrap();
f1.var_return_opt_match();
f2.var_return_opt_try();
}

0 comments on commit 7ed9f94

Please sign in to comment.