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

This commit makes rustc emit debug locations for all call
and invoke statements in LLVM IR, if they are contained
within a function that debuginfo is enabled for. This is
important because LLVM does not handle the case where a
function body containing debuginfo is inlined into another
function with debuginfo, but the inlined call statement
does not have a debug location. In this case, LLVM will
not know where (in terms of source code coordinates) the
function was inlined to and we end up with some statements
still linked to the source locations in there original,
non-inlined function without any indication that they are
indeed an inline-copy. Later, when generating DWARF from
the IR, LLVM will interpret this as corrupt IR and abort.

Unfortunately, the undesirable case described above can
still occur when using LTO. If there is a crate compiled
without debuginfo calling into a crate compiled with
debuginfo, we again end up with the conditions triggering
the error. This is why some LTO tests still fail with the
dreaded assertion, if the standard library was built with
debuginfo enabled.
That is, `RUSTFLAGS_STAGE2=-g make rustc-stage2` will
succeed but `RUSTFLAGS_STAGE2=-g make check` will still
fail after this commit has been merged. This is a problem
that has to be dealt with separately.

Fixes #17201
Fixes #15816
Fixes #15156

Author: michaelwoerister

src/librustc/middle/trans/base.rs

@@ -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,
@@ -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,
@@ -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);
 
@@ -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();
@@ -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")
         }
@@ -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)

src/librustc/middle/trans/callee.rs

@@ -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);
         }
     };
 
@@ -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);
         }
         _ => {}

src/librustc/middle/trans/cleanup.rs

@@ -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;
@@ -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>,
 }
@@ -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>;
@@ -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
@@ -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,
@@ -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 }
     }
 
@@ -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,
@@ -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
@@ -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
@@ -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);
@@ -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,
@@ -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);
@@ -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)
@@ -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)
@@ -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
     }
@@ -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)

src/librustc/middle/trans/controlflow.rs

@@ -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;
@@ -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, _) => {
@@ -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;
 }
@@ -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);