Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
debuginfo: Make debuginfo source location assignment more stable (Pt. 1)
So far, the source location an LLVM instruction was linked to was controlled by
`debuginfo::set_source_location()` and `debuginfo::clear_source_location()`.
This interface mimicked how LLVM's `IRBuilder` handles debug location
assignment. While this interface has some theoretical performance benefits, it
also makes things terribly unstable: One sets some quasi-global state and then
hopes that it is still correct when a given instruction is emitted---an
assumption that has been proven to not hold a bit too often.

This patch requires the debug source location to be passed to the actual
instruction emitting function. This makes source location assignment explicit
and will prevent future changes to `trans` from accidentally breaking things in
the majority of cases.

This patch does not yet implement the new principle for all instruction kinds
but the stepping experience should have improved significantly nonetheless
already.
  • Loading branch information
michaelwoerister committed Jan 21, 2015
1 parent 51e28dd commit a55ef3a
Show file tree
Hide file tree
Showing 14 changed files with 889 additions and 405 deletions.
22 changes: 11 additions & 11 deletions src/librustc_trans/trans/_match.rs
Expand Up @@ -208,10 +208,10 @@ use trans::cleanup::{self, CleanupMethods};
use trans::common::*;
use trans::consts;
use trans::datum::*;
use trans::debuginfo::{self, DebugLoc, ToDebugLoc};
use trans::expr::{self, Dest};
use trans::tvec;
use trans::type_of;
use trans::debuginfo;
use middle::ty::{self, Ty};
use session::config::FullDebugInfo;
use util::common::indenter;
Expand Down Expand Up @@ -632,7 +632,7 @@ fn bind_subslice_pat(bcx: Block,

let slice_begin = InBoundsGEP(bcx, base, &[C_uint(bcx.ccx(), offset_left)]);
let slice_len_offset = C_uint(bcx.ccx(), offset_left + offset_right);
let slice_len = Sub(bcx, len, slice_len_offset);
let slice_len = Sub(bcx, len, slice_len_offset, DebugLoc::None);
let slice_ty = ty::mk_slice(bcx.tcx(),
bcx.tcx().mk_region(ty::ReStatic),
ty::mt {ty: vt.unit_ty, mutbl: ast::MutImmutable});
Expand All @@ -656,7 +656,7 @@ fn extract_vec_elems<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
elems.extend(range(0, before).map(|i| GEPi(bcx, base, &[i])));
elems.extend(range(0, after).rev().map(|i| {
InBoundsGEP(bcx, base, &[
Sub(bcx, len, C_uint(bcx.ccx(), i + 1))
Sub(bcx, len, C_uint(bcx.ccx(), i + 1), DebugLoc::None)
])
}));
ExtractedBlock { vals: elems, bcx: bcx }
Expand Down Expand Up @@ -731,7 +731,7 @@ impl FailureHandler {
Infallible =>
panic!("attempted to panic in a non-panicking panic handler!"),
JumpToBasicBlock(basic_block) =>
Br(bcx, basic_block),
Br(bcx, basic_block, DebugLoc::None),
Unreachable =>
build::Unreachable(bcx)
}
Expand Down Expand Up @@ -889,7 +889,7 @@ fn compile_guard<'a, 'p, 'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
}
}

with_cond(bcx, Not(bcx, val), |bcx| {
with_cond(bcx, Not(bcx, val, guard_expr.debug_loc()), |bcx| {
// Guard does not match: remove all bindings from the lllocals table
for (_, &binding_info) in data.bindings_map.iter() {
call_lifetime_end(bcx, binding_info.llmatch);
Expand Down Expand Up @@ -966,7 +966,7 @@ fn compile_submatch<'a, 'p, 'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
}
_ => ()
}
Br(bcx, data.bodycx.llbb);
Br(bcx, data.bodycx.llbb, DebugLoc::None);
}
}
}
Expand Down Expand Up @@ -1096,7 +1096,7 @@ fn compile_submatch_continue<'a, 'p, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
if !exhaustive || i + 1 < len {
opt_cx = bcx.fcx.new_temp_block("match_case");
match kind {
Single => Br(bcx, opt_cx.llbb),
Single => Br(bcx, opt_cx.llbb, DebugLoc::None),
Switch => {
match opt.trans(bcx) {
SingleResult(r) => {
Expand Down Expand Up @@ -1131,7 +1131,7 @@ fn compile_submatch_continue<'a, 'p, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
compare_scalar_types(
bcx, test_val, vend,
t, ast::BiLe);
Result::new(bcx, And(bcx, llge, llle))
Result::new(bcx, And(bcx, llge, llle, DebugLoc::None))
}
LowerBound(Result { bcx, val }) => {
compare_scalar_types(bcx, test_val, val, t, ast::BiGe)
Expand All @@ -1149,12 +1149,12 @@ fn compile_submatch_continue<'a, 'p, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
if i + 1 < len && (guarded || multi_pats || kind == CompareSliceLength) {
branch_chk = Some(JumpToBasicBlock(bcx.llbb));
}
CondBr(after_cx, matches, opt_cx.llbb, bcx.llbb);
CondBr(after_cx, matches, opt_cx.llbb, bcx.llbb, DebugLoc::None);
}
_ => ()
}
} else if kind == Compare || kind == CompareSliceLength {
Br(bcx, else_cx.llbb);
Br(bcx, else_cx.llbb, DebugLoc::None);
}

let mut size = 0u;
Expand Down Expand Up @@ -1194,7 +1194,7 @@ fn compile_submatch_continue<'a, 'p, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
// Compile the fall-through case, if any
if !exhaustive && kind != Single {
if kind == Compare || kind == CompareSliceLength {
Br(bcx, else_cx.llbb);
Br(bcx, else_cx.llbb, DebugLoc::None);
}
match chk {
// If there is only one default arm left, move on to the next
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_trans/trans/adt.rs
Expand Up @@ -62,6 +62,7 @@ use trans::cleanup;
use trans::cleanup::CleanupMethods;
use trans::common::*;
use trans::datum;
use trans::debuginfo::DebugLoc;
use trans::machine;
use trans::monomorphize;
use trans::type_::Type;
Expand Down Expand Up @@ -979,7 +980,7 @@ pub fn fold_variants<'blk, 'tcx, F>(bcx: Block<'blk, 'tcx>,
let variant_value = PointerCast(variant_cx, value, real_ty.ptr_to());

variant_cx = f(variant_cx, case, variant_value);
Br(variant_cx, bcx_next.llbb);
Br(variant_cx, bcx_next.llbb, DebugLoc::None);
}

bcx_next
Expand Down
76 changes: 38 additions & 38 deletions src/librustc_trans/trans/base.rs
Expand Up @@ -57,7 +57,7 @@ use trans::closure;
use trans::common::{Block, C_bool, C_bytes_in_context, C_i32, C_integral};
use trans::common::{C_null, C_struct_in_context, C_u64, C_u8, C_undef};
use trans::common::{CrateContext, ExternMap, FunctionContext};
use trans::common::{NodeInfo, Result};
use trans::common::{Result};
use trans::common::{node_id_type, return_type_is_void};
use trans::common::{tydesc_info, type_is_immediate};
use trans::common::{type_is_zero_size, val_ty};
Expand All @@ -66,7 +66,7 @@ use trans::consts;
use trans::context::SharedCrateContext;
use trans::controlflow;
use trans::datum;
use trans::debuginfo;
use trans::debuginfo::{self, DebugLoc};
use trans::expr;
use trans::foreign;
use trans::glue;
Expand Down Expand Up @@ -792,7 +792,7 @@ pub fn iter_structural_ty<'blk, 'tcx, F>(cx: Block<'blk, 'tcx>,
&**variant,
substs,
&mut f);
Br(variant_cx, next_cx.llbb);
Br(variant_cx, next_cx.llbb, DebugLoc::None);
}
cx = next_cx;
}
Expand Down Expand Up @@ -957,7 +957,7 @@ pub fn invoke<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
llfn: ValueRef,
llargs: &[ValueRef],
fn_ty: Ty<'tcx>,
call_info: Option<NodeInfo>)
debug_loc: DebugLoc)
-> (ValueRef, Block<'blk, 'tcx>) {
let _icx = push_ctxt("invoke_");
if bcx.unreachable.get() {
Expand All @@ -983,30 +983,25 @@ pub fn invoke<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
let normal_bcx = bcx.fcx.new_temp_block("normal-return");
let landing_pad = bcx.fcx.get_landing_pad();

match call_info {
Some(info) => debuginfo::set_source_location(bcx.fcx, info.id, info.span),
None => debuginfo::clear_source_location(bcx.fcx)
};

let llresult = Invoke(bcx,
llfn,
&llargs[],
normal_bcx.llbb,
landing_pad,
Some(attributes));
Some(attributes),
debug_loc);
return (llresult, normal_bcx);
} else {
debug!("calling {} at {:?}", bcx.val_to_string(llfn), bcx.llbb);
for &llarg in llargs.iter() {
debug!("arg: {}", bcx.val_to_string(llarg));
}

match call_info {
Some(info) => debuginfo::set_source_location(bcx.fcx, info.id, info.span),
None => debuginfo::clear_source_location(bcx.fcx)
};

let llresult = Call(bcx, llfn, &llargs[], Some(attributes));
let llresult = Call(bcx,
llfn,
&llargs[],
Some(attributes),
debug_loc);
return (llresult, bcx);
}
}
Expand Down Expand Up @@ -1094,10 +1089,10 @@ pub fn with_cond<'blk, 'tcx, F>(bcx: Block<'blk, 'tcx>,
let fcx = bcx.fcx;
let next_cx = fcx.new_temp_block("next");
let cond_cx = fcx.new_temp_block("cond");
CondBr(bcx, val, cond_cx.llbb, next_cx.llbb);
CondBr(bcx, val, cond_cx.llbb, next_cx.llbb, DebugLoc::None);
let after_cx = f(cond_cx);
if !after_cx.terminated.get() {
Br(after_cx, next_cx.llbb);
Br(after_cx, next_cx.llbb, DebugLoc::None);
}
next_cx
}
Expand All @@ -1113,7 +1108,7 @@ pub fn call_lifetime_start(cx: Block, ptr: ValueRef) {
let llsize = C_u64(ccx, machine::llsize_of_alloc(ccx, val_ty(ptr).element_type()));
let ptr = PointerCast(cx, ptr, Type::i8p(ccx));
let lifetime_start = ccx.get_intrinsic(&"llvm.lifetime.start");
Call(cx, lifetime_start, &[llsize, ptr], None);
Call(cx, lifetime_start, &[llsize, ptr], None, DebugLoc::None);
}

pub fn call_lifetime_end(cx: Block, ptr: ValueRef) {
Expand All @@ -1127,7 +1122,7 @@ pub fn call_lifetime_end(cx: Block, ptr: ValueRef) {
let llsize = C_u64(ccx, machine::llsize_of_alloc(ccx, val_ty(ptr).element_type()));
let ptr = PointerCast(cx, ptr, Type::i8p(ccx));
let lifetime_end = ccx.get_intrinsic(&"llvm.lifetime.end");
Call(cx, lifetime_end, &[llsize, ptr], None);
Call(cx, lifetime_end, &[llsize, ptr], None, DebugLoc::None);
}

pub fn call_memcpy(cx: Block, dst: ValueRef, src: ValueRef, n_bytes: ValueRef, align: u32) {
Expand All @@ -1144,7 +1139,7 @@ pub fn call_memcpy(cx: Block, dst: ValueRef, src: ValueRef, n_bytes: ValueRef, a
let size = IntCast(cx, n_bytes, ccx.int_type());
let align = C_i32(ccx, align as i32);
let volatile = C_bool(ccx, false);
Call(cx, memcpy, &[dst_ptr, src_ptr, size, align, volatile], None);
Call(cx, memcpy, &[dst_ptr, src_ptr, size, align, volatile], None, DebugLoc::None);
}

pub fn memcpy_ty<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
Expand Down Expand Up @@ -1697,13 +1692,14 @@ fn copy_unboxed_closure_args_to_allocas<'blk, 'tcx>(
// and builds the return block.
pub fn finish_fn<'blk, 'tcx>(fcx: &'blk FunctionContext<'blk, 'tcx>,
last_bcx: Block<'blk, 'tcx>,
retty: ty::FnOutput<'tcx>) {
retty: ty::FnOutput<'tcx>,
ret_debug_loc: DebugLoc) {
let _icx = push_ctxt("finish_fn");

let ret_cx = match fcx.llreturn.get() {
Some(llreturn) => {
if !last_bcx.terminated.get() {
Br(last_bcx, llreturn);
Br(last_bcx, llreturn, DebugLoc::None);
}
raw_block(fcx, false, llreturn)
}
Expand All @@ -1713,7 +1709,7 @@ pub fn finish_fn<'blk, 'tcx>(fcx: &'blk FunctionContext<'blk, 'tcx>,
// This shouldn't need to recompute the return type,
// as new_fn_ctxt did it already.
let substd_retty = fcx.monomorphize(&retty);
build_return_block(fcx, ret_cx, substd_retty);
build_return_block(fcx, ret_cx, substd_retty, ret_debug_loc);

debuginfo::clear_source_location(fcx);
fcx.cleanup();
Expand All @@ -1722,10 +1718,11 @@ pub fn finish_fn<'blk, 'tcx>(fcx: &'blk FunctionContext<'blk, 'tcx>,
// Builds the return block for a function.
pub fn build_return_block<'blk, 'tcx>(fcx: &FunctionContext<'blk, 'tcx>,
ret_cx: Block<'blk, 'tcx>,
retty: ty::FnOutput<'tcx>) {
retty: ty::FnOutput<'tcx>,
ret_debug_location: DebugLoc) {
if fcx.llretslotptr.get().is_none() ||
(!fcx.needs_ret_allocas && fcx.caller_expects_out_pointer) {
return RetVoid(ret_cx);
return RetVoid(ret_cx, ret_debug_location);
}

let retslot = if fcx.needs_ret_allocas {
Expand Down Expand Up @@ -1755,26 +1752,26 @@ pub fn build_return_block<'blk, 'tcx>(fcx: &FunctionContext<'blk, 'tcx>,
if let ty::FnConverging(retty) = retty {
store_ty(ret_cx, retval, get_param(fcx.llfn, 0), retty);
}
RetVoid(ret_cx)
RetVoid(ret_cx, ret_debug_location)
} else {
Ret(ret_cx, retval)
Ret(ret_cx, retval, ret_debug_location)
}
}
// Otherwise, copy the return value to the ret slot
None => match retty {
ty::FnConverging(retty) => {
if fcx.caller_expects_out_pointer {
memcpy_ty(ret_cx, get_param(fcx.llfn, 0), retslot, retty);
RetVoid(ret_cx)
RetVoid(ret_cx, ret_debug_location)
} else {
Ret(ret_cx, load_ty(ret_cx, retslot, retty))
Ret(ret_cx, load_ty(ret_cx, retslot, retty), ret_debug_location)
}
}
ty::FnDiverging => {
if fcx.caller_expects_out_pointer {
RetVoid(ret_cx)
RetVoid(ret_cx, ret_debug_location)
} else {
Ret(ret_cx, C_undef(Type::nil(fcx.ccx)))
Ret(ret_cx, C_undef(Type::nil(fcx.ccx)), ret_debug_location)
}
}
}
Expand Down Expand Up @@ -1905,7 +1902,7 @@ pub fn trans_closure<'a, 'b, 'tcx>(ccx: &CrateContext<'a, 'tcx>,

match fcx.llreturn.get() {
Some(_) => {
Br(bcx, fcx.return_exit_block());
Br(bcx, fcx.return_exit_block(), DebugLoc::None);
fcx.pop_custom_cleanup_scope(arg_scope);
}
None => {
Expand All @@ -1924,8 +1921,11 @@ pub fn trans_closure<'a, 'b, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
}
}

let ret_debug_loc = DebugLoc::At(fn_cleanup_debug_loc.id,
fn_cleanup_debug_loc.span);

// Insert the mandatory first few basic blocks before lltop.
finish_fn(&fcx, bcx, output_type);
finish_fn(&fcx, bcx, output_type, ret_debug_loc);
}

// trans_fn: creates an LLVM function corresponding to a source language
Expand Down Expand Up @@ -1977,7 +1977,7 @@ pub fn trans_named_tuple_constructor<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
disr: ty::Disr,
args: callee::CallArgs,
dest: expr::Dest,
call_info: Option<NodeInfo>)
debug_loc: DebugLoc)
-> Result<'blk, 'tcx> {

let ccx = bcx.fcx.ccx;
Expand Down Expand Up @@ -2016,7 +2016,7 @@ pub fn trans_named_tuple_constructor<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
&fields[],
None,
expr::SaveIn(llresult),
call_info);
debug_loc);
}
_ => ccx.sess().bug("expected expr as arguments for variant/struct tuple constructor")
}
Expand All @@ -2027,7 +2027,7 @@ pub fn trans_named_tuple_constructor<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
let bcx = match dest {
expr::SaveIn(_) => bcx,
expr::Ignore => {
glue::drop_ty(bcx, llresult, result_ty, call_info)
glue::drop_ty(bcx, llresult, result_ty, debug_loc)
}
};

Expand Down Expand Up @@ -2094,7 +2094,7 @@ fn trans_enum_variant_or_tuple_like_struct<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx
adt::trans_set_discr(bcx, &*repr, dest, disr);
}

finish_fn(&fcx, bcx, result_ty);
finish_fn(&fcx, bcx, result_ty, DebugLoc::None);
}

fn enum_variant_size_lint(ccx: &CrateContext, enum_def: &ast::EnumDef, sp: Span, id: ast::NodeId) {
Expand Down

0 comments on commit a55ef3a

Please sign in to comment.