From c49eb075dbb8b1921b3056e0fb1cb87fc0397e15 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 3 Sep 2013 18:23:59 +0200 Subject: [PATCH] debuginfo: Much improved handling of captured variables and by-value arguments. --- src/librustc/middle/trans/_match.rs | 3 + src/librustc/middle/trans/closure.rs | 23 ++- src/librustc/middle/trans/debuginfo.rs | 137 +++++++++++------- .../var-captured-in-nested-closure.rs | 23 ++- 4 files changed, 124 insertions(+), 62 deletions(-) diff --git a/src/librustc/middle/trans/_match.rs b/src/librustc/middle/trans/_match.rs index bc4cc2ce0e1..39e3e97b489 100644 --- a/src/librustc/middle/trans/_match.rs +++ b/src/librustc/middle/trans/_match.rs @@ -2000,6 +2000,9 @@ pub fn store_arg(mut bcx: @mut Block, let arg_ty = node_id_type(bcx, pat.id); add_clean(bcx, llval, arg_ty); + // Debug information (the llvm.dbg.declare intrinsic to be precise) always expects to get an + // alloca, which only is the case on the general path, so lets disable the optimized path when + // debug info is enabled. let fast_path = !bcx.ccx().sess.opts.extra_debuginfo && simple_identifier(pat).is_some(); if fast_path { diff --git a/src/librustc/middle/trans/closure.rs b/src/librustc/middle/trans/closure.rs index d25fedf2d8e..690d7343489 100644 --- a/src/librustc/middle/trans/closure.rs +++ b/src/librustc/middle/trans/closure.rs @@ -308,7 +308,17 @@ pub fn load_environment(fcx: @mut FunctionContext, // Load a pointer to the closure data, skipping over the box header: let llcdata = opaque_box_body(bcx, cdata_ty, fcx.llenv); - // Populate the upvars from the environment. + // Store the pointer to closure data in an alloca for debug info because that's what the + // llvm.dbg.declare intrinsic expects + let env_pointer_alloca = if fcx.ccx.sess.opts.extra_debuginfo { + let alloc = alloc_ty(bcx, ty::mk_mut_ptr(bcx.tcx(), cdata_ty), "__debuginfo_env_ptr"); + Store(bcx, llcdata, alloc); + Some(alloc) + } else { + None + }; + + // Populate the upvars from the environment let mut i = 0u; for cap_var in cap_vars.iter() { let mut upvarptr = GEPi(bcx, llcdata, [0u, i]); @@ -319,8 +329,15 @@ pub fn load_environment(fcx: @mut FunctionContext, let def_id = ast_util::def_id_of_def(cap_var.def); fcx.llupvars.insert(def_id.node, upvarptr); - if fcx.ccx.sess.opts.extra_debuginfo { - debuginfo::create_captured_var_metadata(bcx, def_id.node, upvarptr, cap_var.span); + for &env_pointer_alloca in env_pointer_alloca.iter() { + debuginfo::create_captured_var_metadata( + bcx, + def_id.node, + cdata_ty, + env_pointer_alloca, + i, + sigil, + cap_var.span); } i += 1u; diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs index 09956438bb4..112595d5576 100644 --- a/src/librustc/middle/trans/debuginfo.rs +++ b/src/librustc/middle/trans/debuginfo.rs @@ -163,11 +163,12 @@ struct FunctionDebugContextData { source_locations_enabled: bool, } -enum VariableAccess { - // The value given is a pointer to the data (T*) - DirectVariable, - // The value given has to be dereferenced once to get the pointer to data (T**) - IndirectVariable +enum VariableAccess<'self> { + // The llptr given is an alloca containing the variable's value + DirectVariable { alloca: ValueRef }, + // The llptr given is an alloca containing the start of some pointer chain leading to the + // variable's content. + IndirectVariable { alloca: ValueRef, address_operations: &'self [ValueRef] } } enum VariableKind { @@ -213,11 +214,10 @@ pub fn create_local_var_metadata(bcx: @mut Block, let scope_metadata = scope_metadata(bcx.fcx, node_id, span); declare_local(bcx, - llptr, var_ident, var_type, scope_metadata, - DirectVariable, + DirectVariable { alloca: llptr }, LocalVariable, span); } @@ -228,7 +228,10 @@ pub fn create_local_var_metadata(bcx: @mut Block, /// Adds the created metadata nodes directly to the crate's IR. pub fn create_captured_var_metadata(bcx: @mut Block, node_id: ast::NodeId, - llptr: ValueRef, + env_data_type: ty::t, + env_pointer: ValueRef, + env_index: uint, + closure_sigil: ast::Sigil, span: Span) { if fn_should_be_ignored(bcx.fcx) { return; @@ -250,15 +253,39 @@ pub fn create_captured_var_metadata(bcx: @mut Block, Captured var-id refers to unexpected ast_map variant: %?", ast_item)); } }; + let variable_type = node_id_type(bcx, node_id); let scope_metadata = bcx.fcx.debug_context.get_ref(cx, span).fn_metadata; + let llvm_env_data_type = type_of::type_of(cx, env_data_type); + let byte_offset_of_var_in_env = machine::llelement_offset(cx, llvm_env_data_type, env_index); + + let address_operations = unsafe { + [llvm::LLVMDIBuilderCreateOpDeref(Type::i64().to_ref()), + llvm::LLVMDIBuilderCreateOpPlus(Type::i64().to_ref()), + C_i64(byte_offset_of_var_in_env as i64), + llvm::LLVMDIBuilderCreateOpDeref(Type::i64().to_ref())] + }; + + let address_op_count = match closure_sigil { + ast::BorrowedSigil => { + address_operations.len() + } + ast::ManagedSigil | ast::OwnedSigil => { + address_operations.len() - 1 + } + }; + + let variable_access = IndirectVariable { + alloca: env_pointer, + address_operations: address_operations.slice_to(address_op_count) + }; + declare_local(bcx, - llptr, variable_ident, variable_type, scope_metadata, - IndirectVariable, + variable_access, CapturedVariable, span); } @@ -285,11 +312,10 @@ pub fn create_match_binding_metadata(bcx: @mut Block, let scope_metadata = scope_metadata(bcx.fcx, node_id, span); declare_local(bcx, - llptr, variable_ident, variable_type, scope_metadata, - DirectVariable, + DirectVariable { alloca: llptr }, LocalVariable, span); } @@ -333,14 +359,17 @@ pub fn create_self_argument_metadata(bcx: @mut Block, argument_index }; + let address_operations = &[unsafe { llvm::LLVMDIBuilderCreateOpDeref(Type::i64().to_ref()) }]; + let variable_access = if unsafe { llvm::LLVMIsAAllocaInst(llptr) } != ptr::null() { - DirectVariable + DirectVariable { alloca: llptr } } else { - IndirectVariable + // This is not stable and may break with future LLVM versions. llptr should really always + // be an alloca. Anything else is not supported and just works by chance. + IndirectVariable { alloca: llptr, address_operations: address_operations } }; declare_local(bcx, - llptr, special_idents::self_, type_of_self, scope_metadata, @@ -373,11 +402,10 @@ pub fn create_argument_metadata(bcx: @mut Block, } }; - let variable_access = if unsafe { llvm::LLVMIsAAllocaInst(llptr) } != ptr::null() { - DirectVariable - } else { - IndirectVariable - }; + if unsafe { llvm::LLVMIsAAllocaInst(llptr) } == ptr::null() { + cx.sess.span_bug(span, "debuginfo::create_argument_metadata() - \ + Referenced variable location is not an alloca!"); + } let argument_type = node_id_type(bcx, node_id); let argument_ident = ast_util::path_to_ident(path_ref); @@ -390,11 +418,10 @@ pub fn create_argument_metadata(bcx: @mut Block, }; declare_local(bcx, - llptr, argument_ident, argument_type, scope_metadata, - variable_access, + DirectVariable { alloca: llptr }, ArgumentVariable(argument_index), span); } @@ -783,7 +810,6 @@ fn compile_unit_metadata(cx: @mut CrateContext) { } fn declare_local(bcx: @mut Block, - llptr: ValueRef, variable_ident: ast::Ident, variable_type: ty::t, scope_metadata: DIScope, @@ -805,37 +831,40 @@ fn declare_local(bcx: @mut Block, CapturedVariable => 0 } as c_uint; - let var_metadata = do name.with_c_str |name| { + let (var_alloca, var_metadata) = do name.with_c_str |name| { match variable_access { - DirectVariable => unsafe { - llvm::LLVMDIBuilderCreateLocalVariable( - DIB(cx), - DW_TAG_auto_variable, - scope_metadata, - name, - file_metadata, - loc.line as c_uint, - type_metadata, - cx.sess.opts.optimize != session::No, - 0, - argument_index) - }, - IndirectVariable => unsafe { - let address_op = llvm::LLVMDIBuilderCreateOpDeref(Type::i64().to_ref()); - let address_op_count = 1; - - llvm::LLVMDIBuilderCreateComplexVariable( - DIB(cx), - DW_TAG_auto_variable, - scope_metadata, - name, - file_metadata, - loc.line as c_uint, - type_metadata, - ptr::to_unsafe_ptr(&address_op), - address_op_count, - argument_index) - } + DirectVariable { alloca } => ( + alloca, + unsafe { + llvm::LLVMDIBuilderCreateLocalVariable( + DIB(cx), + DW_TAG_auto_variable, + scope_metadata, + name, + file_metadata, + loc.line as c_uint, + type_metadata, + cx.sess.opts.optimize != session::No, + 0, + argument_index) + } + ), + IndirectVariable { alloca, address_operations } => ( + alloca, + unsafe { + llvm::LLVMDIBuilderCreateComplexVariable( + DIB(cx), + DW_TAG_auto_variable, + scope_metadata, + name, + file_metadata, + loc.line as c_uint, + type_metadata, + vec::raw::to_ptr(address_operations), + address_operations.len() as c_uint, + argument_index) + } + ) } }; @@ -843,7 +872,7 @@ fn declare_local(bcx: @mut Block, unsafe { let instr = llvm::LLVMDIBuilderInsertDeclareAtEnd( DIB(cx), - llptr, + var_alloca, var_metadata, bcx.llbb); diff --git a/src/test/debug-info/var-captured-in-nested-closure.rs b/src/test/debug-info/var-captured-in-nested-closure.rs index 60ad2a3544a..cd20209ddfd 100644 --- a/src/test/debug-info/var-captured-in-nested-closure.rs +++ b/src/test/debug-info/var-captured-in-nested-closure.rs @@ -29,6 +29,23 @@ // check:$7 = 8 // debugger:continue +// debugger:finish +// debugger:print variable +// check:$8 = 1 +// debugger:print constant +// check:$9 = 2 +// debugger:print a_struct +// check:$10 = {a = -3, b = 4.5, c = 5} +// debugger:print *struct_ref +// check:$11 = {a = -3, b = 4.5, c = 5} +// debugger:print *owned +// check:$12 = 6 +// debugger:print managed->val +// check:$13 = 7 +// debugger:print closure_local +// check:$14 = 8 +// debugger:continue + #[allow(unused_variable)]; struct Struct { @@ -59,11 +76,7 @@ fn main() { variable = constant + a_struct.a + struct_ref.a + *owned + *managed + closure_local; }; - // breaking here will yield a wrong value for 'constant'. In particular, GDB will - // read the value of the register that supposedly contains the pointer to 'constant' - // and try derefence it. The register, however, already contains the actual value, and - // not a pointer to it. -mw - // zzz(); + zzz(); nested_closure(); };