From a4fb1404904e42b9b8897d90a506fd128afbe4d0 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 27 Mar 2014 13:43:01 +0100 Subject: [PATCH 1/3] debuginfo: Improve source code position assignment for inlined functions. This commit makes sure that code inlined from other functions isn't assigned the source position of the call site, since this leads to undesired behavior when setting line breakpoints (issue #12886) --- src/librustc/middle/trans/debuginfo.rs | 36 +++++++++++++++----------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs index 8a7f30ee2c42d..6f6484ae1a596 100644 --- a/src/librustc/middle/trans/debuginfo.rs +++ b/src/librustc/middle/trans/debuginfo.rs @@ -533,21 +533,26 @@ pub fn create_argument_metadata(bcx: &Block, arg: &ast::Arg) { pub fn set_source_location(fcx: &FunctionContext, node_id: ast::NodeId, span: Span) { - if fn_should_be_ignored(fcx) { - return; - } - - let cx = fcx.ccx; + match fcx.debug_context { + DebugInfoDisabled => return, + FunctionWithoutDebugInfo => { + set_debug_location(fcx.ccx, UnknownLocation); + return; + } + FunctionDebugContext(~ref function_debug_context) => { + let cx = fcx.ccx; - debug!("set_source_location: {}", cx.sess().codemap().span_to_str(span)); + debug!("set_source_location: {}", cx.sess().codemap().span_to_str(span)); - if fcx.debug_context.get_ref(cx, span).source_locations_enabled.get() { - let loc = span_start(cx, span); - let scope = scope_metadata(fcx, node_id, span); + if function_debug_context.source_locations_enabled.get() { + let loc = span_start(cx, span); + let scope = scope_metadata(fcx, node_id, span); - set_debug_location(cx, DebugLocation::new(scope, loc.line, loc.col.to_uint())); - } else { - set_debug_location(cx, UnknownLocation); + set_debug_location(cx, DebugLocation::new(scope, loc.line, loc.col.to_uint())); + } else { + set_debug_location(cx, UnknownLocation); + } + } } } @@ -590,6 +595,10 @@ pub fn create_function_debug_context(cx: &CrateContext, return DebugInfoDisabled; } + // Clear the debug location so we don't assign them in the function prelude. Do this here + // already, in case we do an early exit from this function. + set_debug_location(cx, UnknownLocation); + if fn_ast_id == -1 { return FunctionWithoutDebugInfo; } @@ -740,9 +749,6 @@ pub fn create_function_debug_context(cx: &CrateContext, fn_metadata, &mut *fn_debug_context.scope_map.borrow_mut()); - // Clear the debug location so we don't assign them in the function prelude - set_debug_location(cx, UnknownLocation); - return FunctionDebugContext(fn_debug_context); fn get_function_signature(cx: &CrateContext, From e6800d2279aea52419cfeb19f402c6fba3791a76 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 27 Mar 2014 15:47:13 +0100 Subject: [PATCH 2/3] debuginfo: Implement discriminator type metadata re-use. An optimization for sharing the type metadata of generic enum discriminators between monomorphized instances (fixes issue #12840) --- src/librustc/middle/trans/debuginfo.rs | 68 +++++++++++++++++++------- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs index 6f6484ae1a596..c2828b333edd1 100644 --- a/src/librustc/middle/trans/debuginfo.rs +++ b/src/librustc/middle/trans/debuginfo.rs @@ -130,6 +130,7 @@ use driver::session::{FullDebugInfo, LimitedDebugInfo, NoDebugInfo}; use lib::llvm::llvm; use lib::llvm::{ModuleRef, ContextRef, ValueRef}; use lib::llvm::debuginfo::*; +use metadata::csearch; use middle::trans::adt; use middle::trans::common::*; use middle::trans::datum::{Datum, Lvalue}; @@ -178,6 +179,7 @@ pub struct CrateDebugContext { current_debug_location: Cell, created_files: RefCell>, created_types: RefCell>, + created_enum_disr_types: RefCell>, namespace_map: RefCell , @NamespaceTreeNode>>, // This collection is used to assert that composite types (structs, enums, ...) have their // members only set once: @@ -196,6 +198,7 @@ impl CrateDebugContext { current_debug_location: Cell::new(UnknownLocation), created_files: RefCell::new(HashMap::new()), created_types: RefCell::new(HashMap::new()), + created_enum_disr_types: RefCell::new(HashMap::new()), namespace_map: RefCell::new(HashMap::new()), composite_types_completed: RefCell::new(HashSet::new()), }; @@ -1542,24 +1545,45 @@ fn prepare_enum_metadata(cx: &CrateContext, .collect(); let discriminant_type_metadata = |inttype| { - let discriminant_llvm_type = adt::ll_inttype(cx, inttype); - let (discriminant_size, discriminant_align) = size_and_align_of(cx, discriminant_llvm_type); - let discriminant_base_type_metadata = type_metadata(cx, adt::ty_of_inttype(inttype), - codemap::DUMMY_SP); - enum_name.with_c_str(|enum_name| { - unsafe { - llvm::LLVMDIBuilderCreateEnumerationType( - DIB(cx), - containing_scope, - enum_name, - file_metadata, - loc.line as c_uint, - bytes_to_bits(discriminant_size), - bytes_to_bits(discriminant_align), - create_DIArray(DIB(cx), enumerators_metadata.as_slice()), - discriminant_base_type_metadata) + // We can reuse the type of the discriminant for all monomorphized instances of an enum + // because it doesn't depend on any type parameters. The def_id, uniquely identifying the + // enum's polytype acts as key in this cache. + let cached_discriminant_type_metadata = debug_context(cx).created_enum_disr_types + .borrow() + .find_copy(&enum_def_id); + match cached_discriminant_type_metadata { + Some(discriminant_type_metadata) => discriminant_type_metadata, + None => { + let discriminant_llvm_type = adt::ll_inttype(cx, inttype); + let (discriminant_size, discriminant_align) = + size_and_align_of(cx, discriminant_llvm_type); + let discriminant_base_type_metadata = type_metadata(cx, + adt::ty_of_inttype(inttype), + codemap::DUMMY_SP); + let discriminant_name = get_enum_discriminant_name(cx, enum_def_id); + + let discriminant_type_metadata = discriminant_name.get().with_c_str(|name| { + unsafe { + llvm::LLVMDIBuilderCreateEnumerationType( + DIB(cx), + containing_scope, + name, + file_metadata, + loc.line as c_uint, + bytes_to_bits(discriminant_size), + bytes_to_bits(discriminant_align), + create_DIArray(DIB(cx), enumerators_metadata.as_slice()), + discriminant_base_type_metadata) + } + }); + + debug_context(cx).created_enum_disr_types + .borrow_mut() + .insert(enum_def_id, discriminant_type_metadata); + + discriminant_type_metadata } - }) + } }; let type_rep = adt::represent_type(cx, enum_type); @@ -1648,6 +1672,16 @@ fn prepare_enum_metadata(cx: &CrateContext, } } }; + + fn get_enum_discriminant_name(cx: &CrateContext, def_id: ast::DefId) -> token::InternedString { + let name = if def_id.krate == ast::LOCAL_CRATE { + cx.tcx.map.get_path_elem(def_id.node).name() + } else { + csearch::get_item_path(&cx.tcx, def_id).last().unwrap().name() + }; + + token::get_name(name) + } } enum MemberOffset { From c5139a05f9dc653a51c1861fed66638771329467 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 10 Apr 2014 10:25:13 +0200 Subject: [PATCH 3/3] debuginfo: Don't create debuginfo for statics inlined from other crates. Fixes issue #13213, that is linker errors when the inlined static has been optimized out of the exporting crate. --- src/librustc/middle/trans/debuginfo.rs | 7 +++++++ src/test/auxiliary/issue13213aux.rs | 27 ++++++++++++++++++++++++++ src/test/debug-info/issue13213.rs | 26 +++++++++++++++++++++++++ 3 files changed, 60 insertions(+) create mode 100644 src/test/auxiliary/issue13213aux.rs create mode 100644 src/test/debug-info/issue13213.rs diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs index c2828b333edd1..8ea2065509bb8 100644 --- a/src/librustc/middle/trans/debuginfo.rs +++ b/src/librustc/middle/trans/debuginfo.rs @@ -293,6 +293,13 @@ pub fn create_global_var_metadata(cx: &CrateContext, return; } + // Don't create debuginfo for globals inlined from other crates. The other crate should already + // contain debuginfo for it. More importantly, the global might not even exist in un-inlined + // form anywhere which would lead to a linker errors. + if cx.external_srcs.borrow().contains_key(&node_id) { + return; + } + let var_item = cx.tcx.map.get(node_id); let (ident, span) = match var_item { diff --git a/src/test/auxiliary/issue13213aux.rs b/src/test/auxiliary/issue13213aux.rs new file mode 100644 index 0000000000000..23cf49fb1d8da --- /dev/null +++ b/src/test/auxiliary/issue13213aux.rs @@ -0,0 +1,27 @@ +// Copyright 2013-2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![crate_type = "lib"] +// compile-flags:-g + +pub use private::P; + +pub struct S { + p: P, +} + +mod private { + pub struct P { + p: i32, + } + pub static THREE: P = P { p: 3 }; +} + +pub static A: S = S { p: private::THREE }; \ No newline at end of file diff --git a/src/test/debug-info/issue13213.rs b/src/test/debug-info/issue13213.rs new file mode 100644 index 0000000000000..e60643eb3f402 --- /dev/null +++ b/src/test/debug-info/issue13213.rs @@ -0,0 +1,26 @@ +// Copyright 2013-2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// ignore-android: FIXME(#10381) + +// aux-build:issue13213aux.rs +extern crate issue13213aux; + +// compile-flags:-g + +// This tests make sure that we get no linker error when using a completely inlined static. Some +// statics that are marked with AvailableExternallyLinkage in the importing crate, may actually not +// be available because they have been optimized out from the exporting crate. +fn main() { + let b: issue13213aux::S = issue13213aux::A; + zzz(); +} + +fn zzz() {()} \ No newline at end of file