Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
3 changes: 2 additions & 1 deletion compiler/rustc_incremental/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ pub(crate) struct DeleteFull<'a> {
}

#[derive(Diagnostic)]
#[diag("error finalizing incremental compilation session directory `{$path}`: {$err}")]
#[diag("did not finalize incremental compilation session directory `{$path}`: {$err}")]
#[help("the next build will not be able to reuse work from this compilation")]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like that this focuses the issue directly on the impact the user might care about 👍

pub(crate) struct Finalize<'a> {
pub path: &'a Path,
pub err: std::io::Error,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_incremental/src/persist/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ pub fn finalize_session_directory(sess: &Session, svh: Option<Svh>) {
}
Err(e) => {
// Warn about the error. However, no need to abort compilation now.
sess.dcx().emit_warn(errors::Finalize { path: &incr_comp_session_dir, err: e });
sess.dcx().emit_note(errors::Finalize { path: &incr_comp_session_dir, err: e });

debug!("finalize_session_directory() - error, marking as invalid");
// Drop the file lock, so we can garage collect
Expand Down
5 changes: 5 additions & 0 deletions tests/run-make/incremental-finalize-fail/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
poison::poison_finalize!();

pub fn hello() -> i32 {
42
}
87 changes: 87 additions & 0 deletions tests/run-make/incremental-finalize-fail/poison/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//! A proc macro that sabotages the incremental compilation finalize step.
//!
//! When invoked, it locates the `-working` session directory inside the
//! incremental compilation directory (passed via POISON_INCR_DIR) and
//! makes it impossible to rename:
//!
//! - On Unix: removes write permission from the parent (crate) directory.
//! - On Windows: creates a file inside the -working directory and leaks
//! the file handle, preventing the directory from being renamed.

extern crate proc_macro;

use std::fs;
use std::path::PathBuf;

use proc_macro::TokenStream;

#[proc_macro]
pub fn poison_finalize(_input: TokenStream) -> TokenStream {
let incr_dir = std::env::var("POISON_INCR_DIR").expect("POISON_INCR_DIR must be set");

let crate_dir = find_crate_dir(&incr_dir);
let working_dir = find_working_dir(&crate_dir);

#[cfg(unix)]
poison_unix(&crate_dir);

#[cfg(windows)]
poison_windows(&working_dir);

TokenStream::new()
}

/// Remove write permission from the crate directory.
/// This causes rename() to fail with EACCES
#[cfg(unix)]
fn poison_unix(crate_dir: &PathBuf) {
use std::os::unix::fs::PermissionsExt;
let mut perms = fs::metadata(crate_dir).unwrap().permissions();
perms.set_mode(0o555); // r-xr-xr-x
fs::set_permissions(crate_dir, perms).unwrap();
}

/// Create a file inside the -working directory and leak the
/// handle. Windows prevents renaming a directory when any file inside it
/// has an open handle. The handle stays open until the rustc process exits.
#[cfg(windows)]
fn poison_windows(working_dir: &PathBuf) {
let poison_file = working_dir.join("_poison_handle");
let f = fs::File::create(&poison_file).unwrap();
// Leak the handle so it stays open for the lifetime of the rustc process.
std::mem::forget(f);
}

/// Find the crate directory for `foo` inside the incremental compilation dir.
///
/// The incremental directory layout is:
/// {incr_dir}/{crate_name}-{stable_crate_id}/
fn find_crate_dir(incr_dir: &str) -> PathBuf {
let mut dirs = fs::read_dir(incr_dir).unwrap().filter_map(|e| {
let e = e.ok()?;
let name = e.file_name();
let name = name.to_str()?;
if e.file_type().ok()?.is_dir() && name.starts_with("foo-") { Some(e.path()) } else { None }
});

let first =
dirs.next().unwrap_or_else(|| panic!("no foo-* crate directory found in {incr_dir}"));
assert!(
dirs.next().is_none(),
"expected exactly one foo-* crate directory in {incr_dir}, found multiple"
);
first
}

/// Find the session directory ending in "-working" inside the crate directory
fn find_working_dir(crate_dir: &PathBuf) -> PathBuf {
for entry in fs::read_dir(crate_dir).unwrap() {
let entry = entry.unwrap();
let name = entry.file_name();
let name = name.to_str().unwrap().to_string();
if name.starts_with("s-") && name.ends_with("-working") {
return entry.path();
}
}
panic!("no -working session directory found in {}", crate_dir.display());
}
103 changes: 103 additions & 0 deletions tests/run-make/incremental-finalize-fail/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
//! Test that a failure to finalize the incremental compilation session directory
//! (i.e., the rename from "-working" to the SVH-based name) results in a
//! note, not an ICE, and that the compilation output is still produced.
//!
//! Strategy:
//! 1. Build the `poison` proc-macro crate
//! 2. Compile foo.rs with incremental compilation
//! The proc macro runs mid-compilation (after prepare_session_directory
//! but before finalize_session_directory) and sabotages the rename:
//! - On Unix: removes write permission from the crate directory,
//! so rename() fails with EACCES.
//! - On Windows: creates and leaks an open file handle inside the
//! -working directory, so rename() fails with ERROR_ACCESS_DENIED.
//! 3. Assert that stderr contains the finalize failure messages

use std::fs;
use std::path::{Path, PathBuf};

use run_make_support::rustc;

/// Guard that restores permissions on the incremental directory on drop,
/// to ensure cleanup is possible
struct IncrDirCleanup;

fn main() {
let _cleanup = IncrDirCleanup;

// Build the poison proc-macro crate
rustc().input("poison/lib.rs").crate_name("poison").crate_type("proc-macro").run();

let poison_dylib = find_proc_macro_dylib("poison");

// Incremental compile with the poison macro active
let out = rustc()
.input("foo.rs")
.crate_type("rlib")
.incremental("incr")
.extern_("poison", &poison_dylib)
.env("POISON_INCR_DIR", "incr")
.run();

out.assert_stderr_contains("note: did not finalize incremental compilation session directory");
out.assert_stderr_contains(
"help: the next build will not be able to reuse work from this compilation",
);
out.assert_stderr_not_contains("internal compiler error");
}

impl Drop for IncrDirCleanup {
fn drop(&mut self) {
let incr = Path::new("incr");
if !incr.exists() {
return;
}

#[cfg(unix)]
restore_permissions(incr);
}
}

/// Recursively restore write permissions so rm -rf works after the chmod trick
#[cfg(unix)]
fn restore_permissions(path: &Path) {
use std::os::unix::fs::PermissionsExt;
if let Ok(entries) = fs::read_dir(path) {
for entry in entries.filter_map(|e| e.ok()) {
if entry.file_type().map_or(false, |ft| ft.is_dir()) {
let mut perms = match fs::metadata(entry.path()) {
Ok(m) => m.permissions(),
Err(_) => continue,
};
perms.set_mode(0o755);
let _ = fs::set_permissions(entry.path(), perms);
}
}
}
}

/// Locate the compiled proc-macro dylib by scanning the current directory.
fn find_proc_macro_dylib(name: &str) -> PathBuf {
let prefix = if cfg!(target_os = "windows") { "" } else { "lib" };

let ext: &str = if cfg!(target_os = "macos") {
"dylib"
} else if cfg!(target_os = "windows") {
"dll"
} else {
"so"
};

let lib_name = format!("{prefix}{name}.{ext}");

for entry in fs::read_dir(".").unwrap() {
let entry = entry.unwrap();
let name = entry.file_name();
let name = name.to_str().unwrap();
if name == lib_name {
return entry.path();
}
}

panic!("could not find proc-macro dylib for `{name}`");
}
Loading