Skip to content
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

Fix debug line number info for macro expansions. #35238

Merged
merged 1 commit into from
Aug 25, 2016
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
2 changes: 1 addition & 1 deletion src/librustc/middle/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ impl CodeExtent {
// (This is the special case aluded to in the
// doc-comment for this method)
let stmt_span = blk.stmts[r.first_statement_index as usize].span;
Some(Span { lo: stmt_span.hi, ..blk.span })
Some(Span { lo: stmt_span.hi, hi: blk.span.hi, expn_id: stmt_span.expn_id })
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"force overflow checks on or off"),
trace_macros: bool = (false, parse_bool, [UNTRACKED],
"for every macro invocation, print its name and arguments"),
debug_macros: bool = (false, parse_bool, [TRACKED],
"emit line numbers debug info inside macros"),
enable_nonzeroing_move_hints: bool = (false, parse_bool, [TRACKED],
"force nonzeroing move optimization on"),
keep_hygiene_data: bool = (false, parse_bool, [UNTRACKED],
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1796,6 +1796,11 @@ extern {
Col: c_uint)
-> DILexicalBlock;

pub fn LLVMRustDIBuilderCreateLexicalBlockFile(Builder: DIBuilderRef,
Scope: DIScope,
File: DIFile)
-> DILexicalBlock;

pub fn LLVMRustDIBuilderCreateStaticVariable(Builder: DIBuilderRef,
Context: DIScope,
Name: *const c_char,
Expand Down
48 changes: 40 additions & 8 deletions src/librustc_trans/debuginfo/create_scope_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,33 @@ use syntax_pos::Pos;
use rustc_data_structures::bitvec::BitVector;
use rustc_data_structures::indexed_vec::{Idx, IndexVec};

use syntax_pos::BytePos;

#[derive(Clone, Copy, Debug)]
pub struct MirDebugScope {
pub scope_metadata: DIScope,
// Start and end offsets of the file to which this DIScope belongs.
// These are used to quickly determine whether some span refers to the same file.
pub file_start_pos: BytePos,
pub file_end_pos: BytePos,
}

impl MirDebugScope {
pub fn is_valid(&self) -> bool {
!self.scope_metadata.is_null()
}
}

/// Produce DIScope DIEs for each MIR Scope which has variables defined in it.
/// If debuginfo is disabled, the returned vector is empty.
pub fn create_mir_scopes(fcx: &FunctionContext) -> IndexVec<VisibilityScope, DIScope> {
pub fn create_mir_scopes(fcx: &FunctionContext) -> IndexVec<VisibilityScope, MirDebugScope> {
let mir = fcx.mir.clone().expect("create_mir_scopes: missing MIR for fn");
let mut scopes = IndexVec::from_elem(ptr::null_mut(), &mir.visibility_scopes);
let null_scope = MirDebugScope {
scope_metadata: ptr::null_mut(),
file_start_pos: BytePos(0),
file_end_pos: BytePos(0)
};
let mut scopes = IndexVec::from_elem(null_scope, &mir.visibility_scopes);

let fn_metadata = match fcx.debug_context {
FunctionDebugContext::RegularContext(box ref data) => data.fn_metadata,
Expand Down Expand Up @@ -59,8 +81,8 @@ fn make_mir_scope(ccx: &CrateContext,
has_variables: &BitVector,
fn_metadata: DISubprogram,
scope: VisibilityScope,
scopes: &mut IndexVec<VisibilityScope, DIScope>) {
if !scopes[scope].is_null() {
scopes: &mut IndexVec<VisibilityScope, MirDebugScope>) {
if scopes[scope].is_valid() {
return;
}

Expand All @@ -70,7 +92,12 @@ fn make_mir_scope(ccx: &CrateContext,
scopes[parent]
} else {
// The root is the function itself.
scopes[scope] = fn_metadata;
let loc = span_start(ccx, mir.span);
scopes[scope] = MirDebugScope {
scope_metadata: fn_metadata,
file_start_pos: loc.file.start_pos,
file_end_pos: loc.file.end_pos,
};
return;
};

Expand All @@ -81,20 +108,25 @@ fn make_mir_scope(ccx: &CrateContext,
// However, we don't skip creating a nested scope if
// our parent is the root, because we might want to
// put arguments in the root and not have shadowing.
if parent_scope != fn_metadata {
if parent_scope.scope_metadata != fn_metadata {
scopes[scope] = parent_scope;
return;
}
}

let loc = span_start(ccx, scope_data.span);
scopes[scope] = unsafe {
let file_metadata = file_metadata(ccx, &loc.file.name, &loc.file.abs_path);
let scope_metadata = unsafe {
llvm::LLVMRustDIBuilderCreateLexicalBlock(
DIB(ccx),
parent_scope,
parent_scope.scope_metadata,
file_metadata,
loc.line as c_uint,
loc.col.to_usize() as c_uint)
};
scopes[scope] = MirDebugScope {
scope_metadata: scope_metadata,
file_start_pos: loc.file.start_pos,
file_end_pos: loc.file.end_pos,
};
}
16 changes: 15 additions & 1 deletion src/librustc_trans/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use context::SharedCrateContext;
use session::Session;

use llvm::{self, ValueRef};
use llvm::debuginfo::{DIType, DIFile, DIScope, DIDescriptor, DICompositeType};
use llvm::debuginfo::{DIType, DIFile, DIScope, DIDescriptor, DICompositeType, DILexicalBlock};

use rustc::hir::def_id::DefId;
use rustc::ty::subst::Substs;
Expand Down Expand Up @@ -1839,3 +1839,17 @@ pub fn create_global_var_metadata(cx: &CrateContext,
ptr::null_mut());
}
}

// Creates an "extension" of an existing DIScope into another file.
pub fn extend_scope_to_file(ccx: &CrateContext,
scope_metadata: DIScope,
file: &syntax_pos::FileMap)
-> DILexicalBlock {
let file_metadata = file_metadata(ccx, &file.name, &file.abs_path);
unsafe {
llvm::LLVMRustDIBuilderCreateLexicalBlockFile(
DIB(ccx),
scope_metadata,
file_metadata)
}
}
3 changes: 2 additions & 1 deletion src/librustc_trans/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ pub mod metadata;
mod create_scope_map;
mod source_loc;

pub use self::create_scope_map::create_mir_scopes;
pub use self::create_scope_map::{create_mir_scopes, MirDebugScope};
pub use self::source_loc::start_emitting_source_locations;
pub use self::metadata::create_global_var_metadata;
pub use self::metadata::extend_scope_to_file;

#[allow(non_upper_case_globals)]
const DW_TAG_auto_variable: c_uint = 0x100;
Expand Down
141 changes: 100 additions & 41 deletions src/librustc_trans/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,17 @@

use libc::c_uint;
use llvm::{self, ValueRef};
use llvm::debuginfo::DIScope;
use rustc::ty;
use rustc::mir::repr as mir;
use rustc::mir::tcx::LvalueTy;
use session::config::FullDebugInfo;
use base;
use common::{self, Block, BlockAndBuilder, CrateContext, FunctionContext, C_null};
use debuginfo::{self, declare_local, DebugLoc, VariableAccess, VariableKind};
use debuginfo::{self, declare_local, DebugLoc, VariableAccess, VariableKind, FunctionDebugContext};
use machine;
use type_of;

use syntax_pos::DUMMY_SP;
use syntax_pos::{DUMMY_SP, NO_EXPANSION, COMMAND_LINE_EXPN, BytePos};
use syntax::parse::token::keywords;

use std::ops::Deref;
Expand Down Expand Up @@ -103,12 +102,67 @@ pub struct MirContext<'bcx, 'tcx:'bcx> {
locals: IndexVec<mir::Local, LocalRef<'tcx>>,

/// Debug information for MIR scopes.
scopes: IndexVec<mir::VisibilityScope, DIScope>
scopes: IndexVec<mir::VisibilityScope, debuginfo::MirDebugScope>,
}

impl<'blk, 'tcx> MirContext<'blk, 'tcx> {
pub fn debug_loc(&self, source_info: mir::SourceInfo) -> DebugLoc {
DebugLoc::ScopeAt(self.scopes[source_info.scope], source_info.span)
pub fn debug_loc(&mut self, source_info: mir::SourceInfo) -> DebugLoc {
// Bail out if debug info emission is not enabled.
match self.fcx.debug_context {
FunctionDebugContext::DebugInfoDisabled |
FunctionDebugContext::FunctionWithoutDebugInfo => {
// Can't return DebugLoc::None here because intrinsic::trans_intrinsic_call()
// relies on debug location to obtain span of the call site.
return DebugLoc::ScopeAt(self.scopes[source_info.scope].scope_metadata,
source_info.span);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean we are building LLVM scope metadata even if debuginfo is disabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No: create_mir_scopes() bails out early when debug info is disabled, so scope_metadata would be null; trans_intrinsic_call() only uses the span part. I hope this stuff can go away when we get rid of oldtrans.

}
FunctionDebugContext::RegularContext(_) =>{}
}

// In order to have a good line stepping behavior in debugger, we overwrite debug
// locations of macro expansions with that of the outermost expansion site
// (unless the crate is being compiled with `-Z debug-macros`).
if source_info.span.expn_id == NO_EXPANSION ||
source_info.span.expn_id == COMMAND_LINE_EXPN ||
self.fcx.ccx.sess().opts.debugging_opts.debug_macros {

let scope_metadata = self.scope_metadata_for_loc(source_info.scope,
source_info.span.lo);
DebugLoc::ScopeAt(scope_metadata, source_info.span)
} else {
let cm = self.fcx.ccx.sess().codemap();
// Walk up the macro expansion chain until we reach a non-expanded span.
let mut span = source_info.span;
while span.expn_id != NO_EXPANSION && span.expn_id != COMMAND_LINE_EXPN {
if let Some(callsite_span) = cm.with_expn_info(span.expn_id,
|ei| ei.map(|ei| ei.call_site.clone())) {
span = callsite_span;
} else {
break;
}
}
let scope_metadata = self.scope_metadata_for_loc(source_info.scope, span.lo);
// Use span of the outermost call site, while keeping the original lexical scope
DebugLoc::ScopeAt(scope_metadata, span)
}
}

// DILocations inherit source file name from the parent DIScope. Due to macro expansions
// it may so happen that the current span belongs to a different file than the DIScope
// corresponding to span's containing visibility scope. If so, we need to create a DIScope
// "extension" into that file.
fn scope_metadata_for_loc(&self, scope_id: mir::VisibilityScope, pos: BytePos)
-> llvm::debuginfo::DIScope {
let scope_metadata = self.scopes[scope_id].scope_metadata;
if pos < self.scopes[scope_id].file_start_pos ||
pos >= self.scopes[scope_id].file_end_pos {
let cm = self.fcx.ccx.sess().codemap();
debuginfo::extend_scope_to_file(self.fcx.ccx,
scope_metadata,
&cm.lookup_char_pos(pos).file)
} else {
scope_metadata
}
}
}

Expand Down Expand Up @@ -155,16 +209,38 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) {
analyze::cleanup_kinds(bcx, &mir))
});

// Allocate a `Block` for every basic block
let block_bcxs: IndexVec<mir::BasicBlock, Block<'blk,'tcx>> =
mir.basic_blocks().indices().map(|bb| {
if bb == mir::START_BLOCK {
fcx.new_block("start")
} else {
fcx.new_block(&format!("{:?}", bb))
}
}).collect();

// Compute debuginfo scopes from MIR scopes.
let scopes = debuginfo::create_mir_scopes(fcx);

let mut mircx = MirContext {
mir: mir.clone(),
fcx: fcx,
llpersonalityslot: None,
blocks: block_bcxs,
unreachable_block: None,
cleanup_kinds: cleanup_kinds,
landing_pads: IndexVec::from_elem(None, mir.basic_blocks()),
scopes: scopes,
locals: IndexVec::new(),
};

// Allocate variable and temp allocas
let locals = {
let args = arg_local_refs(&bcx, &mir, &scopes, &lvalue_locals);
mircx.locals = {
let args = arg_local_refs(&bcx, &mir, &mircx.scopes, &lvalue_locals);
let vars = mir.var_decls.iter().enumerate().map(|(i, decl)| {
let ty = bcx.monomorphize(&decl.ty);
let scope = scopes[decl.source_info.scope];
let dbg = !scope.is_null() && bcx.sess().opts.debuginfo == FullDebugInfo;
let debug_scope = mircx.scopes[decl.source_info.scope];
let dbg = debug_scope.is_valid() && bcx.sess().opts.debuginfo == FullDebugInfo;

let local = mir.local_index(&mir::Lvalue::Var(mir::Var::new(i))).unwrap();
if !lvalue_locals.contains(local.index()) && !dbg {
Expand All @@ -173,11 +249,16 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) {

let lvalue = LvalueRef::alloca(&bcx, ty, &decl.name.as_str());
if dbg {
bcx.with_block(|bcx| {
declare_local(bcx, decl.name, ty, scope,
VariableAccess::DirectVariable { alloca: lvalue.llval },
VariableKind::LocalVariable, decl.source_info.span);
});
let dbg_loc = mircx.debug_loc(decl.source_info);
if let DebugLoc::ScopeAt(scope, span) = dbg_loc {
bcx.with_block(|bcx| {
declare_local(bcx, decl.name, ty, scope,
VariableAccess::DirectVariable { alloca: lvalue.llval },
VariableKind::LocalVariable, span);
});
} else {
panic!("Unexpected");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would avoid creating debuginfo for variables declared inside of macros altogether. That can only lead to consistency problems for LLVM to trip over. And it seems to me that the current algorithm would make all variables declared within a macro expansion be visible in the expansion site scope, even nested ones, since we are just walking up parent scopes until we find one that's not from a macro.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, good catch! I dunno about not creating debug locs altogether, but I definitely should have used the original lexical scope here. declare_local() probably needs to take two scopes: one for the lexical scope nesting and one to associate a DILocation with.

}
LocalRef::Lvalue(lvalue)
});
Expand All @@ -203,37 +284,15 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) {
})).collect()
};

// Allocate a `Block` for every basic block
let block_bcxs: IndexVec<mir::BasicBlock, Block<'blk,'tcx>> =
mir.basic_blocks().indices().map(|bb| {
if bb == mir::START_BLOCK {
fcx.new_block("start")
} else {
fcx.new_block(&format!("{:?}", bb))
}
}).collect();

// Branch to the START block
let start_bcx = block_bcxs[mir::START_BLOCK];
let start_bcx = mircx.blocks[mir::START_BLOCK];
bcx.br(start_bcx.llbb);

// Up until here, IR instructions for this function have explicitly not been annotated with
// source code location, so we don't step into call setup code. From here on, source location
// emitting should be enabled.
debuginfo::start_emitting_source_locations(fcx);

let mut mircx = MirContext {
mir: mir.clone(),
fcx: fcx,
llpersonalityslot: None,
blocks: block_bcxs,
unreachable_block: None,
cleanup_kinds: cleanup_kinds,
landing_pads: IndexVec::from_elem(None, mir.basic_blocks()),
locals: locals,
scopes: scopes
};

let mut visited = BitVector::new(mir.basic_blocks().len());

let mut rpo = traversal::reverse_postorder(&mir);
Expand Down Expand Up @@ -271,7 +330,7 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) {
/// indirect.
fn arg_local_refs<'bcx, 'tcx>(bcx: &BlockAndBuilder<'bcx, 'tcx>,
mir: &mir::Mir<'tcx>,
scopes: &IndexVec<mir::VisibilityScope, DIScope>,
scopes: &IndexVec<mir::VisibilityScope, debuginfo::MirDebugScope>,
lvalue_locals: &BitVector)
-> Vec<LocalRef<'tcx>> {
let fcx = bcx.fcx();
Expand All @@ -281,8 +340,8 @@ fn arg_local_refs<'bcx, 'tcx>(bcx: &BlockAndBuilder<'bcx, 'tcx>,

// Get the argument scope, if it exists and if we need it.
let arg_scope = scopes[mir::ARGUMENT_VISIBILITY_SCOPE];
let arg_scope = if !arg_scope.is_null() && bcx.sess().opts.debuginfo == FullDebugInfo {
Some(arg_scope)
let arg_scope = if arg_scope.is_valid() && bcx.sess().opts.debuginfo == FullDebugInfo {
Some(arg_scope.scope_metadata)
} else {
None
};
Expand Down
9 changes: 9 additions & 0 deletions src/rustllvm/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,15 @@ extern "C" LLVMRustMetadataRef LLVMRustDIBuilderCreateLexicalBlock(
));
}

extern "C" LLVMRustMetadataRef LLVMRustDIBuilderCreateLexicalBlockFile(
LLVMRustDIBuilderRef Builder,
LLVMRustMetadataRef Scope,
LLVMRustMetadataRef File) {
return wrap(Builder->createLexicalBlockFile(
unwrapDI<DIDescriptor>(Scope),
unwrapDI<DIFile>(File)));
}

extern "C" LLVMRustMetadataRef LLVMRustDIBuilderCreateStaticVariable(
LLVMRustDIBuilderRef Builder,
LLVMRustMetadataRef Context,
Expand Down
Loading