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

Do not reuse post LTO products when exports change #71131

Merged
Show file tree
Hide file tree
Changes from 2 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
37 changes: 33 additions & 4 deletions src/librustc_codegen_llvm/back/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,11 +517,20 @@ fn thin_lto(

let prev_imports = prev_import_map.modules_imported_by(module_name);
let curr_imports = curr_import_map.modules_imported_by(module_name);
let prev_exports = prev_import_map.modules_exported_by(module_name);
let curr_exports = curr_import_map.modules_exported_by(module_name);
let imports_all_green = curr_imports
.iter()
.all(|imported_module| green_modules.contains_key(imported_module));
let exports_all_green = curr_exports
pnkfelix marked this conversation as resolved.
Show resolved Hide resolved
.iter()
.all(|exported_module| green_modules.contains_key(exported_module));

if imports_all_green && equivalent_as_sets(prev_imports, curr_imports) {
if imports_all_green
&& equivalent_as_sets(prev_imports, curr_imports)
&& exports_all_green
&& equivalent_as_sets(prev_exports, curr_exports)
{
let work_product = green_modules[module_name].clone();
copy_jobs.push(work_product);
info!(" - {}: re-used", module_name);
Expand Down Expand Up @@ -885,13 +894,19 @@ pub unsafe fn optimize_thin_module(
pub struct ThinLTOImports {
pnkfelix marked this conversation as resolved.
Show resolved Hide resolved
// key = llvm name of importing module, value = list of modules it imports from
imports: FxHashMap<String, Vec<String>>,
// key = llvm name of exporting module, value = list of modules it exports to
exports: FxHashMap<String, Vec<String>>,
}

impl ThinLTOImports {
fn modules_imported_by(&self, llvm_module_name: &str) -> &[String] {
self.imports.get(llvm_module_name).map(|v| &v[..]).unwrap_or(&[])
}

fn modules_exported_by(&self, llvm_module_name: &str) -> &[String] {
self.exports.get(llvm_module_name).map(|v| &v[..]).unwrap_or(&[])
}

fn save_to_file(&self, path: &Path) -> io::Result<()> {
use std::io::Write;
let file = File::create(path)?;
Expand All @@ -909,13 +924,17 @@ impl ThinLTOImports {
fn load_from_file(path: &Path) -> io::Result<ThinLTOImports> {
use std::io::BufRead;
let mut imports = FxHashMap::default();
let mut current_module = None;
let mut current_imports = vec![];
let mut exports: FxHashMap<_, Vec<_>> = FxHashMap::default();
let mut current_module: Option<String> = None;
let mut current_imports: Vec<String> = vec![];
let file = File::open(path)?;
for line in io::BufReader::new(file).lines() {
let line = line?;
if line.is_empty() {
let importing_module = current_module.take().expect("Importing module not set");
for imported in &current_imports {
exports.entry(imported.clone()).or_default().push(importing_module.clone());
}
imports.insert(importing_module, mem::replace(&mut current_imports, vec![]));
} else if line.starts_with(' ') {
// Space marks an imported module
Expand All @@ -927,7 +946,7 @@ impl ThinLTOImports {
current_module = Some(line.trim().to_string());
}
}
Ok(ThinLTOImports { imports })
Ok(ThinLTOImports { imports, exports })
}

/// Loads the ThinLTO import map from ThinLTOData.
Expand All @@ -951,7 +970,17 @@ impl ThinLTOImports {
.get_mut(importing_module_name)
.unwrap()
.push(imported_module_name.to_owned());

if !map.exports.contains_key(imported_module_name) {
map.exports.insert(imported_module_name.to_owned(), vec![]);
}

map.exports
.get_mut(imported_module_name)
.unwrap()
.push(importing_module_name.to_owned());
}

let mut map = ThinLTOImports::default();
llvm::LLVMRustGetThinLTOModuleImports(
data,
Expand Down
26 changes: 26 additions & 0 deletions src/test/incremental/thinlto/cgu_invalidated_when_export_added.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// revisions: cfail1 cfail2
// build-pass

// rust-lang/rust#69798:
//
// This is analgous to cgu_invalidated_when_import_added, but it covers a
// problem uncovered where a change to the *export* set caused a link failure
// when reusing post-LTO optimized object code.

pub struct Foo {}
impl Drop for Foo {
fn drop(&mut self) {
println!("Dropping Foo");
}
}
#[no_mangle]
pub extern "C" fn run() {
thread_local! { pub static FOO : Foo = Foo { } ; }

#[cfg(cfail2)]
{
FOO.with(|_f| ())
}
}

pub fn main() { run() }
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// revisions: cfail1 cfail2
// build-pass

// rust-lang/rust#69798:
//
// This is analgous to cgu_invalidated_when_export_added, but it covers the
// other direction. This is analogous to cgu_invalidated_when_import_added: we
// include it, because it may uncover bugs in variant implementation strategies.

pub struct Foo {}
impl Drop for Foo {
fn drop(&mut self) {
println!("Dropping Foo");
}
}
#[no_mangle]
pub extern "C" fn run() {
thread_local! { pub static FOO : Foo = Foo { } ; }

#[cfg(cfail1)]
{
FOO.with(|_f| ())
}
}

pub fn main() { run() }