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 no_std tests that load libc from the sysroot when download-rustc is enabled #110229

Merged
merged 1 commit into from
Apr 19, 2023
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
14 changes: 3 additions & 11 deletions src/bootstrap/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,17 +271,9 @@ impl Step for Rustc {
false,
);

// HACK: This avoids putting the newly built artifacts in the sysroot if we're using
// `download-rustc`, to avoid "multiple candidates for `rmeta`" errors. Technically, that's
// not quite right: people can set `download-rustc = true` to download even if there are
// changes to the compiler, and in that case ideally we would put the *new* artifacts in the
// sysroot, in case there are API changes that should be used by tools. In practice,
// though, that should be very uncommon, and people can still disable download-rustc.
if !builder.download_rustc() {
let libdir = builder.sysroot_libdir(compiler, target);
let hostdir = builder.sysroot_libdir(compiler, compiler.host);
add_to_sysroot(&builder, &libdir, &hostdir, &librustc_stamp(builder, compiler, target));
}
let libdir = builder.sysroot_libdir(compiler, target);
let hostdir = builder.sysroot_libdir(compiler, compiler.host);
add_to_sysroot(&builder, &libdir, &hostdir, &librustc_stamp(builder, compiler, target));
}
}

Expand Down
51 changes: 48 additions & 3 deletions src/bootstrap/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use std::borrow::Cow;
use std::collections::HashSet;
use std::env;
use std::ffi::OsStr;
use std::fs;
use std::io::prelude::*;
use std::io::BufReader;
Expand Down Expand Up @@ -652,8 +653,19 @@ impl Step for Rustc {
// so its artifacts can't be reused.
if builder.download_rustc() && compiler.stage != 0 {
// Copy the existing artifacts instead of rebuilding them.
// NOTE: this path is only taken for tools linking to rustc-dev.
builder.ensure(Sysroot { compiler });
// NOTE: this path is only taken for tools linking to rustc-dev (including ui-fulldeps tests).
let sysroot = builder.ensure(Sysroot { compiler });

let ci_rustc_dir = builder.out.join(&*builder.build.build.triple).join("ci-rustc");
for file in builder.config.rustc_dev_contents() {
let src = ci_rustc_dir.join(&file);
let dst = sysroot.join(file);
if src.is_dir() {
t!(fs::create_dir_all(dst));
} else {
builder.copy(&src, &dst);
}
}
return;
}

Expand Down Expand Up @@ -1281,7 +1293,40 @@ impl Step for Sysroot {
}

// Copy the compiler into the correct sysroot.
builder.cp_r(&builder.ci_rustc_dir(builder.build.build), &sysroot);
// NOTE(#108767): We intentionally don't copy `rustc-dev` artifacts until they're requested with `builder.ensure(Rustc)`.
// This fixes an issue where we'd have multiple copies of libc in the sysroot with no way to tell which to load.
// There are a few quirks of bootstrap that interact to make this reliable:
// 1. The order `Step`s are run is hard-coded in `builder.rs` and not configurable. This
// avoids e.g. reordering `test::UiFulldeps` before `test::Ui` and causing the latter to
// fail because of duplicate metadata.
// 2. The sysroot is deleted and recreated between each invocation, so running `x test
// ui-fulldeps && x test ui` can't cause failures.
let mut filtered_files = Vec::new();
// Don't trim directories or files that aren't loaded per-target; they can't cause conflicts.
let suffix = format!("lib/rustlib/{}/lib", compiler.host);
for path in builder.config.rustc_dev_contents() {
let path = Path::new(&path);
if path.parent().map_or(false, |parent| parent.ends_with(&suffix)) {
filtered_files.push(path.file_name().unwrap().to_owned());
}
}

let filtered_extensions = [OsStr::new("rmeta"), OsStr::new("rlib"), OsStr::new("so")];
let ci_rustc_dir = builder.ci_rustc_dir(builder.config.build);
builder.cp_filtered(&ci_rustc_dir, &sysroot, &|path| {
if path.extension().map_or(true, |ext| !filtered_extensions.contains(&ext)) {
return true;
}
if !path.parent().map_or(true, |p| p.ends_with(&suffix)) {
return true;
}
if !filtered_files.iter().all(|f| f != path.file_name().unwrap()) {
builder.verbose_than(1, &format!("ignoring {}", path.display()));
false
} else {
true
}
});
return INTERNER.intern_path(sysroot);
}

Expand Down
29 changes: 26 additions & 3 deletions src/bootstrap/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::{
env,
ffi::{OsStr, OsString},
fs::{self, File},
io::{BufRead, BufReader, ErrorKind},
io::{BufRead, BufReader, BufWriter, ErrorKind, Write},
path::{Path, PathBuf},
process::{Command, Stdio},
};
Expand Down Expand Up @@ -262,10 +262,20 @@ impl Config {
let directory_prefix = Path::new(Path::new(uncompressed_filename).file_stem().unwrap());

// decompress the file
let data = t!(File::open(tarball));
let data = t!(File::open(tarball), format!("file {} not found", tarball.display()));
let decompressor = XzDecoder::new(BufReader::new(data));

let mut tar = tar::Archive::new(decompressor);

// `compile::Sysroot` needs to know the contents of the `rustc-dev` tarball to avoid adding
// it to the sysroot unless it was explicitly requested. But parsing the 100 MB tarball is slow.
// Cache the entries when we extract it so we only have to read it once.
let mut recorded_entries = if dst.ends_with("ci-rustc") && pattern == "rustc-dev" {
Some(BufWriter::new(t!(File::create(dst.join(".rustc-dev-contents")))))
} else {
None
};

for member in t!(tar.entries()) {
let mut member = t!(member);
let original_path = t!(member.path()).into_owned();
Expand All @@ -283,13 +293,19 @@ impl Config {
if !t!(member.unpack_in(dst)) {
panic!("path traversal attack ??");
}
if let Some(record) = &mut recorded_entries {
t!(writeln!(record, "{}", short_path.to_str().unwrap()));
}
let src_path = dst.join(original_path);
if src_path.is_dir() && dst_path.exists() {
continue;
}
t!(fs::rename(src_path, dst_path));
}
t!(fs::remove_dir_all(dst.join(directory_prefix)));
let dst_dir = dst.join(directory_prefix);
if dst_dir.exists() {
t!(fs::remove_dir_all(&dst_dir), format!("failed to remove {}", dst_dir.display()));
}
}

/// Returns whether the SHA256 checksum of `path` matches `expected`.
Expand Down Expand Up @@ -365,6 +381,13 @@ impl Config {
Some(rustfmt_path)
}

pub(crate) fn rustc_dev_contents(&self) -> Vec<String> {
assert!(self.download_rustc());
let ci_rustc_dir = self.out.join(&*self.build.triple).join("ci-rustc");
let rustc_dev_contents_file = t!(File::open(ci_rustc_dir.join(".rustc-dev-contents")));
t!(BufReader::new(rustc_dev_contents_file).lines().collect())
}

pub(crate) fn download_ci_rustc(&self, commit: &str) {
self.verbose(&format!("using downloaded stage2 artifacts from CI (commit {commit})"));

Expand Down
7 changes: 7 additions & 0 deletions tests/ui/meta/no_std-extern-libc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Test that `download-rustc` doesn't put duplicate copies of libc in the sysroot.
// check-pass
#![crate_type = "lib"]
#![no_std]
#![feature(rustc_private)]

extern crate libc;