Skip to content

debuginfo: Make sure that all calls to drop glue are associated with debug locations. #17533

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.

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

Merged
merged 1 commit into from
Sep 29, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 18 additions & 7 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1791,7 +1791,7 @@ pub fn trans_closure(ccx: &CrateContext,
body: &ast::Block,
llfndecl: ValueRef,
param_substs: &param_substs,
id: ast::NodeId,
fn_ast_id: ast::NodeId,
_attributes: &[ast::Attribute],
arg_types: Vec<ty::t>,
output_type: ty::t,
Expand All @@ -1811,7 +1811,7 @@ pub fn trans_closure(ccx: &CrateContext,
let arena = TypedArena::new();
let fcx = new_fn_ctxt(ccx,
llfndecl,
id,
fn_ast_id,
has_env,
output_type,
param_substs,
Expand All @@ -1820,7 +1820,9 @@ pub fn trans_closure(ccx: &CrateContext,
let mut bcx = init_function(&fcx, false, output_type);

// cleanup scope for the incoming arguments
let arg_scope = fcx.push_custom_cleanup_scope();
let fn_cleanup_debug_loc =
debuginfo::get_cleanup_debug_loc_for_ast_node(fn_ast_id, body.span, true);
let arg_scope = fcx.push_custom_cleanup_scope_with_debug_loc(fn_cleanup_debug_loc);

let block_ty = node_id_type(bcx, body.id);

Expand Down Expand Up @@ -1969,7 +1971,9 @@ pub fn trans_named_tuple_constructor<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
ctor_ty: ty::t,
disr: ty::Disr,
args: callee::CallArgs,
dest: expr::Dest) -> Result<'blk, 'tcx> {
dest: expr::Dest,
call_info: Option<NodeInfo>)
-> Result<'blk, 'tcx> {

let ccx = bcx.fcx.ccx;
let tcx = ccx.tcx();
Expand Down Expand Up @@ -1999,8 +2003,13 @@ pub fn trans_named_tuple_constructor<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
match args {
callee::ArgExprs(exprs) => {
let fields = exprs.iter().map(|x| &**x).enumerate().collect::<Vec<_>>();
bcx = expr::trans_adt(bcx, result_ty, disr, fields.as_slice(),
None, expr::SaveIn(llresult));
bcx = expr::trans_adt(bcx,
result_ty,
disr,
fields.as_slice(),
None,
expr::SaveIn(llresult),
call_info);
}
_ => ccx.sess().bug("expected expr as arguments for variant/struct tuple constructor")
}
Expand All @@ -2010,7 +2019,9 @@ pub fn trans_named_tuple_constructor<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>,
// drop the temporary we made
let bcx = match dest {
expr::SaveIn(_) => bcx,
expr::Ignore => glue::drop_ty(bcx, llresult, result_ty)
expr::Ignore => {
glue::drop_ty(bcx, llresult, result_ty, call_info)
}
};

Result::new(bcx, llresult)
Expand Down
10 changes: 7 additions & 3 deletions src/librustc/middle/trans/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -714,8 +714,12 @@ pub fn trans_call_inner<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
fcx.pop_custom_cleanup_scope(arg_cleanup_scope);

let ctor_ty = callee_ty.subst(bcx.tcx(), &substs);
return base::trans_named_tuple_constructor(bcx, ctor_ty, disr,
args, dest.unwrap());
return base::trans_named_tuple_constructor(bcx,
ctor_ty,
disr,
args,
dest.unwrap(),
call_info);
}
};

Expand Down Expand Up @@ -835,7 +839,7 @@ pub fn trans_call_inner<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
match (dest, opt_llretslot) {
(Some(expr::Ignore), Some(llretslot)) => {
// drop the value if it is not being saved.
bcx = glue::drop_ty(bcx, llretslot, ret_ty);
bcx = glue::drop_ty(bcx, llretslot, ret_ty, call_info);
call_lifetime_end(bcx, llretslot);
}
_ => {}
Expand Down
120 changes: 96 additions & 24 deletions src/librustc/middle/trans/cleanup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ use middle::trans::base;
use middle::trans::build;
use middle::trans::callee;
use middle::trans::common;
use middle::trans::common::{Block, FunctionContext, ExprId};
use middle::trans::common::{Block, FunctionContext, ExprId, NodeInfo};
use middle::trans::debuginfo;
use middle::trans::glue;
use middle::trans::type_::Type;
use middle::ty;
Expand All @@ -36,6 +37,10 @@ pub struct CleanupScope<'blk, 'tcx: 'blk> {
// Cleanups to run upon scope exit.
cleanups: Vec<CleanupObj>,

// The debug location any drop calls generated for this scope will be
// associated with.
debug_loc: Option<NodeInfo>,

cached_early_exits: Vec<CachedEarlyExit>,
cached_landing_pad: Option<BasicBlockRef>,
}
Expand Down Expand Up @@ -69,7 +74,10 @@ pub struct CachedEarlyExit {
pub trait Cleanup {
fn must_unwind(&self) -> bool;
fn clean_on_unwind(&self) -> bool;
fn trans<'blk, 'tcx>(&self, bcx: Block<'blk, 'tcx>) -> Block<'blk, 'tcx>;
fn trans<'blk, 'tcx>(&self,
bcx: Block<'blk, 'tcx>,
debug_loc: Option<NodeInfo>)
-> Block<'blk, 'tcx>;
}

pub type CleanupObj = Box<Cleanup+'static>;
Expand All @@ -80,14 +88,14 @@ pub enum ScopeId {
}

impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
fn push_ast_cleanup_scope(&self, id: ast::NodeId) {
fn push_ast_cleanup_scope(&self, debug_loc: NodeInfo) {
/*!
* Invoked when we start to trans the code contained
* within a new cleanup scope.
*/

debug!("push_ast_cleanup_scope({})",
self.ccx.tcx().map.node_to_string(id));
self.ccx.tcx().map.node_to_string(debug_loc.id));

// FIXME(#2202) -- currently closure bodies have a parent
// region, which messes up the assertion below, since there
Expand All @@ -101,10 +109,15 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
// this new AST scope had better be its immediate child.
let top_scope = self.top_ast_scope();
if top_scope.is_some() {
assert_eq!(self.ccx.tcx().region_maps.opt_encl_scope(id), top_scope);
assert_eq!(self.ccx
.tcx()
.region_maps
.opt_encl_scope(debug_loc.id),
top_scope);
}

self.push_scope(CleanupScope::new(AstScopeKind(id)));
self.push_scope(CleanupScope::new(AstScopeKind(debug_loc.id),
Some(debug_loc)));
}

fn push_loop_cleanup_scope(&self,
Expand All @@ -114,13 +127,38 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
self.ccx.tcx().map.node_to_string(id));
assert_eq!(Some(id), self.top_ast_scope());

self.push_scope(CleanupScope::new(LoopScopeKind(id, exits)));
// Just copy the debuginfo source location from the enclosing scope
let debug_loc = self.scopes
.borrow()
.last()
.unwrap()
.debug_loc;

self.push_scope(CleanupScope::new(LoopScopeKind(id, exits), debug_loc));
}

fn push_custom_cleanup_scope(&self) -> CustomScopeIndex {
let index = self.scopes_len();
debug!("push_custom_cleanup_scope(): {}", index);
self.push_scope(CleanupScope::new(CustomScopeKind));

// Just copy the debuginfo source location from the enclosing scope
let debug_loc = self.scopes
.borrow()
.last()
.map(|opt_scope| opt_scope.debug_loc)
.unwrap_or(None);

self.push_scope(CleanupScope::new(CustomScopeKind, debug_loc));
CustomScopeIndex { index: index }
}

fn push_custom_cleanup_scope_with_debug_loc(&self,
debug_loc: NodeInfo)
-> CustomScopeIndex {
let index = self.scopes_len();
debug!("push_custom_cleanup_scope(): {}", index);

self.push_scope(CleanupScope::new(CustomScopeKind, Some(debug_loc)));
CustomScopeIndex { index: index }
}

Expand All @@ -141,7 +179,6 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {

let scope = self.pop_scope();
self.trans_scope_cleanups(bcx, &scope)

}

fn pop_loop_cleanup_scope(&self,
Expand Down Expand Up @@ -175,9 +212,9 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> {
}

fn pop_and_trans_custom_cleanup_scope(&self,
bcx: Block<'blk, 'tcx>,
custom_scope: CustomScopeIndex)
-> Block<'blk, 'tcx> {
bcx: Block<'blk, 'tcx>,
custom_scope: CustomScopeIndex)
-> Block<'blk, 'tcx> {
/*!
* Removes the top cleanup scope from the stack, which must be
* a temporary scope, and generates the code to do its
Expand Down Expand Up @@ -503,7 +540,7 @@ impl<'blk, 'tcx> CleanupHelperMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx
let mut bcx = bcx;
if !bcx.unreachable.get() {
for cleanup in scope.cleanups.iter().rev() {
bcx = cleanup.trans(bcx);
bcx = cleanup.trans(bcx, scope.debug_loc);
}
}
bcx
Expand Down Expand Up @@ -671,7 +708,8 @@ impl<'blk, 'tcx> CleanupHelperMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx
let mut bcx_out = bcx_in;
for cleanup in scope.cleanups.iter().rev() {
if cleanup_is_suitable_for(&**cleanup, label) {
bcx_out = cleanup.trans(bcx_out);
bcx_out = cleanup.trans(bcx_out,
scope.debug_loc);
}
}
build::Br(bcx_out, prev_llbb);
Expand Down Expand Up @@ -785,9 +823,12 @@ impl<'blk, 'tcx> CleanupHelperMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx
}

impl<'blk, 'tcx> CleanupScope<'blk, 'tcx> {
fn new(kind: CleanupScopeKind<'blk, 'tcx>) -> CleanupScope<'blk, 'tcx> {
fn new(kind: CleanupScopeKind<'blk, 'tcx>,
debug_loc: Option<NodeInfo>)
-> CleanupScope<'blk, 'tcx> {
CleanupScope {
kind: kind,
debug_loc: debug_loc,
cleanups: vec!(),
cached_early_exits: vec!(),
cached_landing_pad: None,
Expand Down Expand Up @@ -902,11 +943,14 @@ impl Cleanup for DropValue {
self.must_unwind
}

fn trans<'blk, 'tcx>(&self, bcx: Block<'blk, 'tcx>) -> Block<'blk, 'tcx> {
fn trans<'blk, 'tcx>(&self,
bcx: Block<'blk, 'tcx>,
debug_loc: Option<NodeInfo>)
-> Block<'blk, 'tcx> {
let bcx = if self.is_immediate {
glue::drop_ty_immediate(bcx, self.val, self.ty)
glue::drop_ty_immediate(bcx, self.val, self.ty, debug_loc)
} else {
glue::drop_ty(bcx, self.val, self.ty)
glue::drop_ty(bcx, self.val, self.ty, debug_loc)
};
if self.zero {
base::zero_mem(bcx, self.val, self.ty);
Expand Down Expand Up @@ -935,7 +979,12 @@ impl Cleanup for FreeValue {
true
}

fn trans<'blk, 'tcx>(&self, bcx: Block<'blk, 'tcx>) -> Block<'blk, 'tcx> {
fn trans<'blk, 'tcx>(&self,
bcx: Block<'blk, 'tcx>,
debug_loc: Option<NodeInfo>)
-> Block<'blk, 'tcx> {
apply_debug_loc(bcx.fcx, debug_loc);

match self.heap {
HeapManaged => {
glue::trans_free(bcx, self.ptr)
Expand Down Expand Up @@ -963,7 +1012,12 @@ impl Cleanup for FreeSlice {
true
}

fn trans<'blk, 'tcx>(&self, bcx: Block<'blk, 'tcx>) -> Block<'blk, 'tcx> {
fn trans<'blk, 'tcx>(&self,
bcx: Block<'blk, 'tcx>,
debug_loc: Option<NodeInfo>)
-> Block<'blk, 'tcx> {
apply_debug_loc(bcx.fcx, debug_loc);

match self.heap {
HeapManaged => {
glue::trans_free(bcx, self.ptr)
Expand All @@ -988,7 +1042,11 @@ impl Cleanup for LifetimeEnd {
true
}

fn trans<'blk, 'tcx>(&self, bcx: Block<'blk, 'tcx>) -> Block<'blk, 'tcx> {
fn trans<'blk, 'tcx>(&self,
bcx: Block<'blk, 'tcx>,
debug_loc: Option<NodeInfo>)
-> Block<'blk, 'tcx> {
apply_debug_loc(bcx.fcx, debug_loc);
base::call_lifetime_end(bcx, self.ptr);
bcx
}
Expand Down Expand Up @@ -1023,15 +1081,29 @@ fn cleanup_is_suitable_for(c: &Cleanup,
!label.is_unwind() || c.clean_on_unwind()
}

fn apply_debug_loc(fcx: &FunctionContext, debug_loc: Option<NodeInfo>) {
match debug_loc {
Some(ref src_loc) => {
debuginfo::set_source_location(fcx, src_loc.id, src_loc.span);
}
None => {
debuginfo::clear_source_location(fcx);
}
}
}

///////////////////////////////////////////////////////////////////////////
// These traits just exist to put the methods into this file.

pub trait CleanupMethods<'blk, 'tcx> {
fn push_ast_cleanup_scope(&self, id: ast::NodeId);
fn push_ast_cleanup_scope(&self, id: NodeInfo);
fn push_loop_cleanup_scope(&self,
id: ast::NodeId,
exits: [Block<'blk, 'tcx>, ..EXIT_MAX]);
id: ast::NodeId,
exits: [Block<'blk, 'tcx>, ..EXIT_MAX]);
fn push_custom_cleanup_scope(&self) -> CustomScopeIndex;
fn push_custom_cleanup_scope_with_debug_loc(&self,
debug_loc: NodeInfo)
-> CustomScopeIndex;
fn pop_and_trans_ast_cleanup_scope(&self,
bcx: Block<'blk, 'tcx>,
cleanup_scope: ast::NodeId)
Expand Down
12 changes: 8 additions & 4 deletions src/librustc/middle/trans/controlflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use middle::trans::cleanup;
use middle::trans::common::*;
use middle::trans::consts;
use middle::trans::datum;
use middle::trans::debuginfo;
use middle::trans::expr;
use middle::trans::meth;
use middle::trans::type_::Type;
Expand Down Expand Up @@ -53,7 +54,9 @@ pub fn trans_stmt<'blk, 'tcx>(cx: Block<'blk, 'tcx>,
let mut bcx = cx;

let id = ast_util::stmt_id(s);
fcx.push_ast_cleanup_scope(id);
let cleanup_debug_loc =
debuginfo::get_cleanup_debug_loc_for_ast_node(id, s.span, false);
fcx.push_ast_cleanup_scope(cleanup_debug_loc);

match s.node {
ast::StmtExpr(ref e, _) | ast::StmtSemi(ref e, _) => {
Expand All @@ -75,8 +78,7 @@ pub fn trans_stmt<'blk, 'tcx>(cx: Block<'blk, 'tcx>,
ast::StmtMac(..) => cx.tcx().sess.bug("unexpanded macro")
}

bcx = fcx.pop_and_trans_ast_cleanup_scope(
bcx, ast_util::stmt_id(s));
bcx = fcx.pop_and_trans_ast_cleanup_scope(bcx, ast_util::stmt_id(s));

return bcx;
}
Expand All @@ -100,7 +102,9 @@ pub fn trans_block<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
let fcx = bcx.fcx;
let mut bcx = bcx;

fcx.push_ast_cleanup_scope(b.id);
let cleanup_debug_loc =
debuginfo::get_cleanup_debug_loc_for_ast_node(b.id, b.span, true);
fcx.push_ast_cleanup_scope(cleanup_debug_loc);

for s in b.stmts.iter() {
bcx = trans_stmt(bcx, &**s);
Expand Down
Loading