fix(cc): cache ordinary existing outputs

src/compiler/cc.rs

@@ -999,10 +999,10 @@ impl CcArgs {
     /// Whether an existing output needs the selected compiler's own path
     /// handling instead of cache materialization.
     ///
-    /// GCC and clang can differ in how they handle an existing output, and
-    /// filesystem permissions/ACLs can make truncate and replace semantics
-    /// observably different. Kache cannot safely infer those semantics from a
-    /// pathname, so every existing output uses the selected compiler directly.
+    /// Ordinary compiler-owned outputs are private, owner-writable regular
+    /// files. Kache may replace those on a hit and let the compiler overwrite
+    /// them on a miss. Symlinks, hardlinks, read-only files and non-regular
+    /// paths still need the selected compiler's exact pathname semantics.
     pub(crate) fn requires_compiler_output_semantics(&self) -> bool {
         self.compiler_output_paths()
             .into_iter()
@@ -1073,13 +1073,37 @@ impl CcArgs {
     }
 }
 
-fn output_path_requires_compiler_semantics(path: &Path) -> bool {
+pub(crate) fn output_path_requires_compiler_semantics(path: &Path) -> bool {
     match std::fs::symlink_metadata(path) {
-        Ok(_) => true,
+        Ok(meta) => !meta.file_type().is_file() || !regular_output_is_replaceable(path, &meta),
         Err(err) => err.kind() != std::io::ErrorKind::NotFound,
     }
 }
 
+fn regular_output_is_replaceable(path: &Path, meta: &std::fs::Metadata) -> bool {
+    regular_output_is_independent(path, meta) && regular_output_is_owner_writable(meta)
+}
+
+fn regular_output_is_owner_writable(meta: &std::fs::Metadata) -> bool {
+    if meta.permissions().readonly() {
+        return false;
+    }
+
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::{MetadataExt, PermissionsExt};
+
+        // SAFETY: geteuid has no arguments, pointers, or preconditions.
+        let current_uid = unsafe { libc::geteuid() };
+        meta.uid() == current_uid && meta.permissions().mode() & 0o200 != 0
+    }
+
+    #[cfg(not(unix))]
+    {
+        true
+    }
+}
+
 #[cfg(unix)]
 fn regular_output_is_independent(_path: &Path, meta: &std::fs::Metadata) -> bool {
     use std::os::unix::fs::MetadataExt;
@@ -8499,22 +8523,48 @@ mod tests {
     }
 
     #[test]
-    fn cc_output_safety_refuses_existing_writable_regular_file() {
+    fn cc_output_safety_allows_existing_writable_private_regular_file() {
         let dir = tempfile::tempdir().unwrap();
         let output = dir.path().join("plain.o");
         std::fs::write(&output, b"ordinary compiler output").unwrap();
 
         let output_str = output.to_string_lossy().into_owned();
         let parsed = CcArgs::parse(&s(&["cc", "-c", "foo.c", "-o", &output_str])).unwrap();
 
-        assert!(parsed.requires_compiler_output_semantics());
+        assert!(!parsed.requires_compiler_output_semantics());
+        assert!(!parsed.refuse_reasons(&[]).iter().any(|reason| {
+            reason
+                .description()
+                .contains("requires compiler write semantics")
+        }));
         assert_eq!(
             discover_cc_output_artifacts(&parsed).outputs().len(),
             1,
             "post-compile discovery may ingest an independent regular file"
         );
     }
 
+    #[test]
+    fn regular_output_writability_distinguishes_readonly_metadata() {
+        let dir = tempfile::tempdir().unwrap();
+        let output = dir.path().join("permissions.o");
+        std::fs::write(&output, b"ordinary compiler output").unwrap();
+
+        let writable = std::fs::metadata(&output).unwrap();
+        let original_permissions = writable.permissions();
+        assert!(regular_output_is_owner_writable(&writable));
+
+        let mut readonly_permissions = original_permissions.clone();
+        readonly_permissions.set_readonly(true);
+        std::fs::set_permissions(&output, readonly_permissions).unwrap();
+        let readonly = std::fs::metadata(&output).unwrap();
+        assert!(!regular_output_is_owner_writable(&readonly));
+
+        // Windows refuses to remove a read-only file, so restore the exact
+        // original permissions before the temporary directory is dropped.
+        std::fs::set_permissions(&output, original_permissions).unwrap();
+    }
+
     /// A user-owned read-only output must reach the selected compiler intact.
     #[cfg(unix)]
     #[test]

src/link.rs

@@ -779,7 +779,7 @@ fn copy_file(src: &Path, dst: &Path, executable: bool) -> Result<()> {
     Ok(())
 }
 
-/// A fully-written C/C++ cache artifact awaiting no-clobber publication.
+/// A fully-written C/C++ cache artifact awaiting publication.
 ///
 /// The staging file is created in the target directory with the same requested
 /// mode as a compiler output (`0666`). The kernel therefore applies the current
@@ -810,6 +810,21 @@ impl PreparedWritableTarget {
         crate::opcounts::record_copied(self.bytes);
         Ok(())
     }
+
+    /// Atomically replace an existing ordinary compiler output.
+    ///
+    /// The caller must first establish that the target is a private, writable
+    /// regular file. Special paths retain the no-clobber path above and are
+    /// handled by the selected compiler instead.
+    pub(crate) fn publish_replacing(self) -> Result<()> {
+        let target = self.target;
+        self.staged
+            .persist(&target)
+            .map_err(|error| error.error)
+            .with_context(|| format!("publishing cc output over {}", target.display()))?;
+        crate::opcounts::record_copied(self.bytes);
+        Ok(())
+    }
 }
 
 fn new_writable_staging_file(target: &Path) -> Result<tempfile::NamedTempFile> {

src/wrapper.rs

@@ -509,10 +509,7 @@ fn should_store_cc_result(exit_code: i32, has_artifacts: bool) -> bool {
 }
 
 fn cc_output_path_requires_passthrough(path: &Path) -> bool {
-    match std::fs::symlink_metadata(path) {
-        Ok(_) => true,
-        Err(error) => error.kind() != std::io::ErrorKind::NotFound,
-    }
+    crate::compiler::cc::output_path_requires_compiler_semantics(path)
 }
 
 /// Forward a `cc`-crate compiler-family probe (`kache -E <file>`) to
@@ -1210,23 +1207,38 @@ fn publish_prepared_cc_artifacts_with(
     mut before_publish: impl FnMut(usize, &Path) -> Result<()>,
 ) -> Result<()> {
     // Validate the whole set before making any final pathname visible.
+    let mut replace_existing = Vec::with_capacity(prepared.len());
     for artifact in &prepared {
         if cc_output_path_requires_passthrough(artifact.target()) {
             anyhow::bail!(
                 "cc restore: output path changed and now requires compiler passthrough semantics"
             );
         }
+        replace_existing.push(std::fs::symlink_metadata(artifact.target()).is_ok());
     }
 
-    for (index, artifact) in prepared.into_iter().enumerate() {
+    for (index, (artifact, replace_existing)) in
+        prepared.into_iter().zip(replace_existing).enumerate()
+    {
         if let Err(error) = before_publish(index, artifact.target()) {
             return if index == 0 {
                 Err(error)
             } else {
                 Err(error.context(PartialCcRestore))
             };
         }
-        if let Err(error) = artifact.publish() {
+        let publish = if replace_existing {
+            if cc_output_path_requires_passthrough(artifact.target()) {
+                Err(anyhow::anyhow!(
+                    "cc restore: output path changed and now requires compiler passthrough semantics"
+                ))
+            } else {
+                artifact.publish_replacing()
+            }
+        } else {
+            artifact.publish()
+        };
+        if let Err(error) = publish {
             return if index == 0 {
                 Err(error)
             } else {
@@ -1239,9 +1251,10 @@ fn publish_prepared_cc_artifacts_with(
 
 /// Restore cached cc artifacts to this invocation's output paths.
 ///
-/// Every artifact is staged first, then the set is published absent-only. If a
-/// race wins after publication starts, the caller receives `PartialCcRestore`
-/// and must not run the compiler over the partially restored output set.
+/// Every artifact is staged first. Absent paths use no-clobber publication;
+/// validated ordinary existing outputs are atomically replaced. If a race wins
+/// after publication starts, the caller receives `PartialCcRestore` and must
+/// not run the compiler over the partially restored output set.
 fn restore_cc_from_cache(
     store: &Store,
     parsed: &crate::compiler::cc::CcArgs,
@@ -5853,6 +5866,60 @@ mod tests {
         assert!(blob_meta.permissions().readonly());
     }
 
+    #[test]
+    fn restore_cc_from_cache_replaces_existing_plain_object() {
+        let dir = tempfile::tempdir().unwrap();
+        let config = test_config(dir.path().join("cache"));
+        let store = Store::open(&config).unwrap();
+        let hash = "abababababababababababababababababababababababababababababababab";
+        create_blob(&store, hash, b"cached object");
+
+        let output = dir.path().join("output.o");
+        std::fs::write(&output, b"stale object").unwrap();
+        let output_str = output.to_string_lossy().into_owned();
+        let parsed = CcCompiler::new()
+            .parse(&s(&["cc", "-c", "foo.c", "-o", &output_str]))
+            .unwrap();
+        let meta = entry_meta("cc-replace-key", vec![cached_file("foo.o", hash)], &[]);
+
+        restore_cc_from_cache(&store, &parsed, &meta).unwrap();
+
+        assert_eq!(std::fs::read(&output).unwrap(), b"cached object");
+        assert_eq!(
+            std::fs::read(store.blob_path(hash)).unwrap(),
+            b"cached object"
+        );
+    }
+
+    #[test]
+    fn cc_restore_revalidates_existing_target_before_replacing_it() {
+        let dir = tempfile::tempdir().unwrap();
+        let output = dir.path().join("output.o");
+        std::fs::write(&output, b"race winner").unwrap();
+        let prepared =
+            vec![link::prepare_writable_target_from_bytes(&output, b"cached object").unwrap()];
+
+        let error = publish_prepared_cc_artifacts_with(prepared, |_, target| {
+            let mut permissions = std::fs::metadata(target)?.permissions();
+            permissions.set_readonly(true);
+            std::fs::set_permissions(target, permissions)?;
+            Ok(())
+        })
+        .unwrap_err();
+
+        assert!(error.to_string().contains("requires compiler passthrough"));
+        assert_eq!(std::fs::read(&output).unwrap(), b"race winner");
+
+        // TempDir cleanup cannot remove a read-only Windows file.
+        #[cfg(windows)]
+        {
+            let mut permissions = std::fs::metadata(&output).unwrap().permissions();
+            #[allow(clippy::permissions_set_readonly_false)]
+            permissions.set_readonly(false);
+            std::fs::set_permissions(&output, permissions).unwrap();
+        }
+    }
+
     #[test]
     fn cc_restore_marks_partial_publication_and_preserves_race_winner() {
         let dir = tempfile::tempdir().unwrap();
@@ -6800,13 +6867,13 @@ exit 0
     }
 
     #[test]
-    fn cc_output_path_passthrough_distinguishes_absent_and_existing_paths() {
+    fn cc_output_path_passthrough_allows_plain_files_but_refuses_symlinks() {
         let dir = tempfile::tempdir().unwrap();
         let output = dir.path().join("output.o");
 
         assert!(!cc_output_path_requires_passthrough(&output));
         std::fs::write(&output, b"existing").unwrap();
-        assert!(cc_output_path_requires_passthrough(&output));
+        assert!(!cc_output_path_requires_passthrough(&output));
 
         #[cfg(unix)]
         {
@@ -6846,6 +6913,7 @@ exit 0
         .unwrap();
         std::fs::set_permissions(&fallback, std::fs::Permissions::from_mode(0o755)).unwrap();
         std::fs::write(&output, b"existing output").unwrap();
+        std::fs::set_permissions(&output, std::fs::Permissions::from_mode(0o444)).unwrap();
 
         let capture_arg = capture.to_string_lossy().into_owned();
         let output_arg = output.to_string_lossy().into_owned();

tests/integration_test.rs

@@ -1406,8 +1406,8 @@ fn test_cc_dev_null_output_is_passthrough_and_preserved() {
 }
 
 /// C/C++ cache materialization must never recreate the read-only/shared shape
-/// that forced unsafe pre-cleaning. An existing warm output is passed to the
-/// selected compiler unchanged rather than replaced by Kache.
+/// that forced unsafe pre-cleaning. A private writable output remains cacheable,
+/// while every restored output remains independently compiler-writable.
 #[cfg(unix)]
 #[test]
 fn test_cc_cache_output_stays_writable_across_miss_hit_and_recompile() {
@@ -1439,12 +1439,56 @@ fn test_cc_cache_output_stays_writable_across_miss_hit_and_recompile() {
     assert_ne!(
         std::fs::metadata(&output).unwrap().permissions().mode() & 0o200,
         0,
-        "passthrough compiler must overwrite the warm output without pre-clean"
+        "compiler must overwrite the warm output without unsafe pre-clean"
     );
 
     let report = kache_report(cache_dir.path());
-    assert_cc_report_counts(&report, 1, 1);
-    assert_eq!(report["summary"]["passthroughs"].as_u64(), Some(1));
+    assert_cc_report_counts(&report, 2, 1);
+    assert_eq!(report["summary"]["passthroughs"].as_u64(), Some(0));
+}
+
+/// Regression for #744: an ordinary compiler-owned object remains cacheable
+/// when the build system leaves it in place. This covers both halves of the
+/// regression: a matching key may replace the stale object from cache, and a
+/// configuration change must compile and store rather than pass through.
+#[test]
+fn test_cc_existing_plain_output_hits_and_reconfigured_miss_is_stored() {
+    build_kache();
+    let project = TempDir::new().unwrap();
+    let cache_dir = TempDir::new().unwrap();
+    let source = project.path().join("foo.c");
+    let output = project.path().join("foo.o");
+    std::fs::write(&source, "int f(void) { return 42; }\n").unwrap();
+
+    let source_str = source.to_string_lossy().into_owned();
+    let output_str = output.to_string_lossy().into_owned();
+    let o0 = ["cc", "-c", &source_str, "-o", &output_str, "-O0", "-g0"];
+    let o2 = ["cc", "-c", &source_str, "-o", &output_str, "-O2", "-g0"];
+
+    run_kache_cc(project.path(), cache_dir.path(), &o0);
+    let cached_o0 = std::fs::read(&output).unwrap();
+
+    std::fs::write(&output, b"stale ordinary object").unwrap();
+    run_kache_cc(project.path(), cache_dir.path(), &o0);
+    assert_eq!(
+        std::fs::read(&output).unwrap(),
+        cached_o0,
+        "an existing private regular object must be replaceable on a cache hit"
+    );
+
+    run_kache_cc(project.path(), cache_dir.path(), &o2);
+    std::fs::remove_file(&output).unwrap();
+    run_kache_cc(project.path(), cache_dir.path(), &o2);
+
+    let report = kache_report(cache_dir.path());
+    let summary = &report["summary"];
+    assert_eq!(summary["local_hits"].as_u64(), Some(2));
+    assert_eq!(summary["passthroughs"].as_u64(), Some(0));
+    assert_eq!(
+        summary["misses"].as_u64().unwrap_or(0) + summary["dups"].as_u64().unwrap_or(0),
+        2,
+        "both cold configurations must be admitted to the cache"
+    );
 }
 
 /// A cache hit must not create an output directory that the selected compiler

tests/unknown_compiler_probe_test.rs

@@ -3,7 +3,7 @@
 use std::fs;
 use std::os::unix::fs::PermissionsExt;
 use std::process::Command;
-use std::time::Instant;
+use std::time::{Duration, Instant};
 
 fn kache_binary() -> &'static str {
     env!("CARGO_BIN_EXE_kache")
@@ -101,6 +101,14 @@ fn probe_recovers_when_wrapper_leaves_descendant_on_stdout() {
     if let Ok(pid_str) = fs::read_to_string(&pid_file)
         && let Ok(pid) = pid_str.trim().parse::<i32>()
     {
+        // The process-group SIGKILL has been sent before kache returns, but an
+        // orphaned descendant can remain visible as a zombie until init reaps
+        // it. Give that asynchronous reap a small bounded window before
+        // treating the process as still alive.
+        let deadline = Instant::now() + Duration::from_secs(2);
+        while unsafe { libc::kill(pid, 0) == 0 } && Instant::now() < deadline {
+            std::thread::sleep(Duration::from_millis(10));
+        }
         let still_alive = unsafe { libc::kill(pid, 0) == 0 };
         assert!(
             !still_alive,