Skip to content

Commit

Permalink
Auto merge of #68298 - Mark-Simulacrum:binary-depdep-fix, r=petrochenkov
Browse files Browse the repository at this point in the history
Avoid declaring a fake dependency edge

When we're producing an rlib, we do not need anything more than an rmeta file
for each of our dependencies (this is indeed utilized by Cargo for pipelining).
Previously, we were still storing the paths of possible rlib/dylib crates, which
meant that they could still plausibly be accessed. With -Zbinary-dep-depinfo,
that meant that Cargo thought that rustc was using both the rlib and an (earlier
emitted) rmeta, and so needed a recompile, as the rlib may have finished writing
*after* compilation started (for more detail, see issue 68149).

This commit changes metadata loading to not store the filepaths of dylib/rlib if
we're going to end up creating an rlib only.

Fixes #68149.
  • Loading branch information
bors committed Jan 23, 2020
2 parents e23dd66 + be663bf commit 462fc37
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 8 deletions.
44 changes: 38 additions & 6 deletions src/librustc_metadata/locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -654,14 +654,36 @@ impl<'a> CrateLocator<'a> {
dylibs: FxHashMap<PathBuf, PathKind>,
) -> Option<(Svh, Library)> {
let mut slot = None;
// Order here matters, rmeta should come first. See comment in
// `extract_one` below.
let source = CrateSource {
rlib: self.extract_one(rlibs, CrateFlavor::Rlib, &mut slot),
rmeta: self.extract_one(rmetas, CrateFlavor::Rmeta, &mut slot),
rlib: self.extract_one(rlibs, CrateFlavor::Rlib, &mut slot),
dylib: self.extract_one(dylibs, CrateFlavor::Dylib, &mut slot),
};
slot.map(|(svh, metadata)| (svh, Library { source, metadata }))
}

fn needs_crate_flavor(&self, flavor: CrateFlavor) -> bool {
if flavor == CrateFlavor::Dylib && self.is_proc_macro == Some(true) {
return true;
}

// The all loop is because `--crate-type=rlib --crate-type=rlib` is
// legal and produces both inside this type.
let is_rlib = self.sess.crate_types.borrow().iter().all(|c| *c == config::CrateType::Rlib);
let needs_object_code = self.sess.opts.output_types.should_codegen();
// If we're producing an rlib, then we don't need object code.
// Or, if we're not producing object code, then we don't need it either
// (e.g., if we're a cdylib but emitting just metadata).
if is_rlib || !needs_object_code {
flavor == CrateFlavor::Rmeta
} else {
// we need all flavors (perhaps not true, but what we do for now)
true
}
}

// Attempts to extract *one* library from the set `m`. If the set has no
// elements, `None` is returned. If the set has more than one element, then
// the errors and notes are emitted about the set of libraries.
Expand All @@ -679,12 +701,22 @@ impl<'a> CrateLocator<'a> {
let mut ret: Option<(PathBuf, PathKind)> = None;
let mut error = 0;

// If we are producing an rlib, and we've already loaded metadata, then
// we should not attempt to discover further crate sources (unless we're
// locating a proc macro; exact logic is in needs_crate_flavor). This means
// that under -Zbinary-dep-depinfo we will not emit a dependency edge on
// the *unused* rlib, and by returning `None` here immediately we
// guarantee that we do indeed not use it.
//
// See also #68149 which provides more detail on why emitting the
// dependency on the rlib is a bad thing.
//
// We currenty do not verify that these other sources are even in sync,
// and this is arguably a bug (see #10786), but because reading metadata
// is quite slow (especially from dylibs) we currently do not read it
// from the other crate sources.
if slot.is_some() {
// FIXME(#10786): for an optimization, we only read one of the
// libraries' metadata sections. In theory we should
// read both, but reading dylib metadata is quite
// slow.
if m.is_empty() {
if m.is_empty() || !self.needs_crate_flavor(flavor) {
return None;
} else if m.len() == 1 {
return Some(m.into_iter().next().unwrap());
Expand Down
9 changes: 7 additions & 2 deletions src/test/ui/rmeta-rpass.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
// run-pass
// Test that using rlibs and rmeta dep crates work together. Specifically, that
// there can be both an rmeta and an rlib file and rustc will prefer the rlib.
// there can be both an rmeta and an rlib file and rustc will prefer the rmeta
// file.
//
// This behavior is simply making sure this doesn't accidentally change; in this
// case we want to make sure that the rlib isn't being used as that would cause
// bugs in -Zbinary-dep-depinfo (see #68298).

// aux-build:rmeta-rmeta.rs
// aux-build:rmeta-rlib-rpass.rs
Expand All @@ -9,5 +14,5 @@ extern crate rmeta_aux;
use rmeta_aux::Foo;

pub fn main() {
let _ = Foo { field: 42 };
let _ = Foo { field2: 42 };
}

0 comments on commit 462fc37

Please sign in to comment.