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

Implement debuginfo path remapping #41419

Closed
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/grammar/verify.rs
Original file line number Diff line number Diff line change
@@ -296,7 +296,7 @@ fn main() {
syntax::errors::registry::Registry::new(&[]),
Rc::new(DummyCrateStore));
let filemap = session.parse_sess.codemap()
.new_filemap("<n/a>".to_string(), None, code);
.new_filemap("<n/a>".to_string(), code);
let mut lexer = lexer::StringReader::new(session.diagnostic(), filemap);
let cm = session.codemap();

15 changes: 15 additions & 0 deletions src/librustc/middle/cstore.rs
Original file line number Diff line number Diff line change
@@ -237,6 +237,15 @@ pub trait CrateStore {
fn exported_symbols(&self, cnum: CrateNum) -> Vec<DefId>;
fn is_no_builtins(&self, cnum: CrateNum) -> bool;

/// Gives the directory the compiler has been executed in when compiling
/// crate `cnum`. This is used by debuginfo generation.
fn compiler_working_dir(&self, cnum: CrateNum) -> String;

/// Gives the prefix map as specified via `-Zdebug-prefix-map-*`. We keep
/// using the original prefix-mapping for a crate, not the mapping specified
/// for the current one.
fn debug_prefix_map(&self, cnum: CrateNum) -> Vec<(String, String)>;

// resolve
fn retrace_path(&self,
cnum: CrateNum,
@@ -380,6 +389,12 @@ impl CrateStore for DummyCrateStore {
{ bug!("native_libraries") }
fn exported_symbols(&self, cnum: CrateNum) -> Vec<DefId> { bug!("exported_symbols") }
fn is_no_builtins(&self, cnum: CrateNum) -> bool { bug!("is_no_builtins") }
fn compiler_working_dir(&self, cnum: CrateNum) -> String {
bug!("compiler_working_dir")
}
fn debug_prefix_map(&self, cnum: CrateNum) -> Vec<(String, String)> {
bug!("debug_prefix_map")
}

// resolve
fn retrace_path(&self,
21 changes: 21 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
@@ -1012,6 +1012,10 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"Set the optimization fuel quota for a crate."),
print_fuel: Option<String> = (None, parse_opt_string, [TRACKED],
"Make Rustc print the total optimization fuel used by a crate."),
debug_prefix_map_from: Vec<String> = (vec![], parse_string_push, [TRACKED],
"push a debuginfo path remapping source"),
debug_prefix_map_to: Vec<String> = (vec![], parse_string_push, [TRACKED],
"push a debuginfo path remapping target"),
}

pub fn default_lib_output() -> CrateType {
@@ -1430,6 +1434,23 @@ pub fn build_session_options_and_crate_config(matches: &getopts::Matches)
output_types.insert(OutputType::Exe, None);
}

let debug_prefix_map_sources = debugging_opts.debug_prefix_map_from.len();
let debug_prefix_map_targets = debugging_opts.debug_prefix_map_from.len();

if debug_prefix_map_targets < debug_prefix_map_sources {
for source in &debugging_opts.debug_prefix_map_from[debug_prefix_map_targets..] {
early_error(error_format,
&format!("option `-Zdebug_prefix_map_from='{}'` does not have \
a corresponding `-Zdebug_prefix_map_to`", source))
}
} else if debug_prefix_map_targets > debug_prefix_map_sources {
for target in &debugging_opts.debug_prefix_map_to[debug_prefix_map_sources..] {
early_error(error_format,
&format!("option `-Zdebug_prefix_map_to='{}'` does not have \
a corresponding `-Zdebug_prefix_map_from`", target))
}
}

let mut cg = build_codegen_options(matches, error_format);

// Issue #30063: if user requests llvm-related output to one
9 changes: 0 additions & 9 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
@@ -631,15 +631,6 @@ pub fn build_session_(sopts: config::Options,
None => Some(filesearch::get_or_default_sysroot())
};

// Make the path absolute, if necessary
let local_crate_source_file = local_crate_source_file.map(|path|
if path.is_absolute() {
path.clone()
} else {
env::current_dir().unwrap().join(&path)
}
);

let optimization_fuel_crate = sopts.debugging_opts.fuel.as_ref().map(|i| i.0.clone());
let optimization_fuel_limit = Cell::new(sopts.debugging_opts.fuel.as_ref()
.map(|i| i.1).unwrap_or(0));
10 changes: 9 additions & 1 deletion src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
@@ -340,6 +340,14 @@ impl CrateStore for cstore::CStore {
self.get_crate_data(cnum).is_no_builtins()
}

fn compiler_working_dir(&self, cnum: CrateNum) -> String {
self.get_crate_data(cnum).compiler_working_dir()
}

fn debug_prefix_map(&self, cnum: CrateNum) -> Vec<(String, String)> {
self.get_crate_data(cnum).debug_prefix_map()
}

fn retrace_path(&self,
cnum: CrateNum,
path: &[DisambiguatedDefPathData])
@@ -399,7 +407,7 @@ impl CrateStore for cstore::CStore {
let (name, def) = data.get_macro(id.index);
let source_name = format!("<{} macros>", name);

let filemap = sess.parse_sess.codemap().new_filemap(source_name, None, def.body);
let filemap = sess.parse_sess.codemap().new_filemap(source_name, def.body);
let local_span = Span { lo: filemap.start_pos, hi: filemap.end_pos, ctxt: NO_EXPANSION };
let body = filemap_to_stream(&sess.parse_sess, filemap);

10 changes: 8 additions & 2 deletions src/librustc_metadata/decoder.rs
Original file line number Diff line number Diff line change
@@ -646,6 +646,14 @@ impl<'a, 'tcx> CrateMetadata {
self.root.lang_items.decode(self).collect()
}

pub fn compiler_working_dir(&self) -> String {
self.root.compiler_working_dir.clone()
}

pub fn debug_prefix_map(&self) -> Vec<(String, String)> {
self.root.debug_prefix_map.decode(self).collect()
}

/// Iterates over each child of the given item.
pub fn each_child_of_item<F>(&self, id: DefIndex, mut callback: F)
where F: FnMut(def::Export)
@@ -1120,7 +1128,6 @@ impl<'a, 'tcx> CrateMetadata {
// We can't reuse an existing FileMap, so allocate a new one
// containing the information we need.
let syntax_pos::FileMap { name,
abs_path,
start_pos,
end_pos,
lines,
@@ -1144,7 +1151,6 @@ impl<'a, 'tcx> CrateMetadata {
}

let local_version = local_codemap.new_imported_filemap(name,
abs_path,
source_length,
lines,
multibyte_chars);
26 changes: 26 additions & 0 deletions src/librustc_metadata/encoder.rs
Original file line number Diff line number Diff line change
@@ -1342,6 +1342,25 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
self.lazy_seq(exported_symbols.iter().map(|&id| tcx.hir.local_def_id(id).index))
}

fn encode_debug_prefix_map(&mut self) -> LazySeq<(String, String)> {
let tcx = self.tcx;

let debug_prefix_map = tcx.sess
.opts
.debugging_opts
.debug_prefix_map_from
.iter()
.cloned()
.zip(tcx.sess
.opts
.debugging_opts
.debug_prefix_map_from
.iter()
.cloned())
.collect::<Vec<(String, String)>>();
self.lazy_seq(debug_prefix_map.into_iter())
}

fn encode_dylib_dependency_formats(&mut self) -> LazySeq<Option<LinkagePreference>> {
match self.tcx.sess.dependency_formats.borrow().get(&config::CrateTypeDylib) {
Some(arr) => {
@@ -1397,6 +1416,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
let exported_symbols = self.encode_exported_symbols();
let exported_symbols_bytes = self.position() - i;

i = self.position();
let debug_prefix_map = self.encode_debug_prefix_map();
let debug_prefix_map_bytes = self.position() - i;

// Encode and index the items.
i = self.position();
let items = self.encode_info_for_items();
@@ -1435,6 +1458,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
def_path_table: def_path_table,
impls: impls,
exported_symbols: exported_symbols,
compiler_working_dir: tcx.sess.working_dir.to_string_lossy().into_owned(),
debug_prefix_map: debug_prefix_map,
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I'm not familiar with the surrounding code. Does this mean the mapping is going to be recorded in the output? In that case, the output would contain some data that is dependent on the build-path. But ideally we'd like the output to be reproducible regardless of the build-path, which is one of the motivations for remapping debuginfo in the first place.

We ran into similar issues with GCC's -fdebug-prefix-map - if we set this on a distribution-wide level (dpkg-buildflags in Debian unstable does this at the moment) we find that some build tools will save CFLAGS etc into other parts of the build output (such as pkg-config files etc) which makes the overall output reproducible, even though GCC's own output was reproducible. Previously, GCC was also saving the value of this flag into the DW_AT_producer field, then they accepted a patch from us to specifically filter this flag out.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's correct, the mapping and compilation directory will end up in any rlib and Rust-dylib (but not in executables, cdylibs, or staticlibs). This is indeed a problem for reproducible builds :(

Copy link
Contributor

Choose a reason for hiding this comment

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

From #38322 :

For this to work, we need to do what @jmesmon says: remap paths as they are stored or -- equivalently -- store the mapping with the crate and apply it to anything that comes from that crate.

I should've mentioned the above point earlier, when you wrote this comment. So I take it you have implemented the second option. Is it possible instead to implement the first option? That might avoid the problem I mentioned. (I still need to reason through the example cases you gave.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this problem could be solved by adapting the rules slightly. We could say that

  • if you specify any debug-prefix-map then debuginfo for inlined functions just does not work anymore out-of-the-box (you can still fix that manually of course).
  • Conversely, if you do not specify a debug-prefix-map then your builds will not be reproducible.

If we do it like this we should be able to transform the codemap before storing it in crate metadata and thus would not need to store the working dir and prefix mapping.

I think this would work well in practice.

index: index,
});

@@ -1455,6 +1480,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
println!(" codemap bytes: {}", codemap_bytes);
println!(" impl bytes: {}", impl_bytes);
println!(" exp. symbols bytes: {}", exported_symbols_bytes);
println!("debug prefix map bytes: {}", debug_prefix_map_bytes);
println!(" def-path table bytes: {}", def_path_table_bytes);
println!(" item bytes: {}", item_bytes);
println!(" index bytes: {}", index_bytes);
2 changes: 2 additions & 0 deletions src/librustc_metadata/schema.rs
Original file line number Diff line number Diff line change
@@ -205,6 +205,8 @@ pub struct CrateRoot {
pub def_path_table: Lazy<hir::map::definitions::DefPathTable>,
pub impls: LazySeq<TraitImpls>,
pub exported_symbols: LazySeq<DefIndex>,
pub compiler_working_dir: String,
pub debug_prefix_map: LazySeq<(String, String)>,
pub index: LazySeq<index::Index>,
}

2 changes: 1 addition & 1 deletion src/librustc_trans/context.rs
Original file line number Diff line number Diff line change
@@ -452,7 +452,7 @@ impl<'tcx> LocalCrateContext<'tcx> {
&llmod_id[..]);

let dbg_cx = if shared.tcx.sess.opts.debuginfo != NoDebugInfo {
let dctx = debuginfo::CrateDebugContext::new(llmod);
let dctx = debuginfo::CrateDebugContext::new(shared.sess(), llmod);
debuginfo::metadata::compile_unit_metadata(shared, &dctx, shared.tcx.sess);
Some(dctx)
} else {
25 changes: 15 additions & 10 deletions src/librustc_trans/debuginfo/create_scope_map.rs
Original file line number Diff line number Diff line change
@@ -8,12 +8,12 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use super::FunctionDebugContext;
use super::{FunctionDebugContext, FunctionDebugContextData};
use super::metadata::file_metadata;
use super::utils::{DIB, span_start};

use llvm;
use llvm::debuginfo::{DIScope, DISubprogram};
use llvm::debuginfo::DIScope;
use common::CrateContext;
use rustc::mir::{Mir, VisibilityScope};

@@ -53,8 +53,8 @@ pub fn create_mir_scopes(ccx: &CrateContext, mir: &Mir, debug_context: &Function
};
let mut scopes = IndexVec::from_elem(null_scope, &mir.visibility_scopes);

let fn_metadata = match *debug_context {
FunctionDebugContext::RegularContext(ref data) => data.fn_metadata,
let debug_context = match *debug_context {
FunctionDebugContext::RegularContext(ref data) => data,
FunctionDebugContext::DebugInfoDisabled |
FunctionDebugContext::FunctionWithoutDebugInfo => {
return scopes;
@@ -71,7 +71,12 @@ pub fn create_mir_scopes(ccx: &CrateContext, mir: &Mir, debug_context: &Function
// Instantiate all scopes.
for idx in 0..mir.visibility_scopes.len() {
let scope = VisibilityScope::new(idx);
make_mir_scope(ccx, &mir, &has_variables, fn_metadata, scope, &mut scopes);
make_mir_scope(ccx,
&mir,
&has_variables,
debug_context,
scope,
&mut scopes);
}

scopes
@@ -80,7 +85,7 @@ pub fn create_mir_scopes(ccx: &CrateContext, mir: &Mir, debug_context: &Function
fn make_mir_scope(ccx: &CrateContext,
mir: &Mir,
has_variables: &BitVector,
fn_metadata: DISubprogram,
debug_context: &FunctionDebugContextData,
scope: VisibilityScope,
scopes: &mut IndexVec<VisibilityScope, MirDebugScope>) {
if scopes[scope].is_valid() {
@@ -89,13 +94,13 @@ fn make_mir_scope(ccx: &CrateContext,

let scope_data = &mir.visibility_scopes[scope];
let parent_scope = if let Some(parent) = scope_data.parent_scope {
make_mir_scope(ccx, mir, has_variables, fn_metadata, parent, scopes);
make_mir_scope(ccx, mir, has_variables, debug_context, parent, scopes);
scopes[parent]
} else {
// The root is the function itself.
let loc = span_start(ccx, mir.span);
scopes[scope] = MirDebugScope {
scope_metadata: fn_metadata,
scope_metadata: debug_context.fn_metadata,
file_start_pos: loc.file.start_pos,
file_end_pos: loc.file.end_pos,
};
@@ -109,14 +114,14 @@ 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.scope_metadata != fn_metadata {
if parent_scope.scope_metadata != debug_context.fn_metadata {
scopes[scope] = parent_scope;
return;
}
}

let loc = span_start(ccx, scope_data.span);
let file_metadata = file_metadata(ccx, &loc.file.name, &loc.file.abs_path);
let file_metadata = file_metadata(ccx, &loc.file.name, debug_context.defining_crate);
let scope_metadata = unsafe {
llvm::LLVMRustDIBuilderCreateLexicalBlock(
DIB(ccx),
Loading