From ee7df57c995415857e72e34f91b8c1949aa2b1c3 Mon Sep 17 00:00:00 2001 From: "Sung, Po Han" Date: Fri, 15 May 2026 10:50:24 +0800 Subject: [PATCH 1/3] hash: strip basedirs from cmdline args in HashKeyParams::compute Path-bearing flags that end up in `common_args` (e.g. `-external:I ` on MSVC, or any other flag classified as `PassThroughPath`) embed the absolute build dir into the cache key. On CI runners whose workdir changes between job invocations (act_runner, ephemeral workers), this defeats the cache. Mirror what ccache does via `make_relative_path` for any `TAKES_PATH` arg: run each arg through `strip_basedirs` before feeding it into the digest. An explicit 0-byte separator between args is added so adjacent args don't blur into each other (`["-Ia", "b"]` must hash differently from `["-I", "ab"]`). Boundary semantics in `strip_basedirs` mean this currently only catches Separated-disposition args where the path occupies its own OsString (positions 0 in the arg byte-slice). Concatenated forms like `-IC:\path` remain a future enhancement. Test: HashKeyParams::compute with two workspaces' `-external:I ` args hashes equal under basedirs, divergent without; an inter-arg adjacency check catches a missing separator regression. --- src/compiler/c.rs | 64 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/src/compiler/c.rs b/src/compiler/c.rs index 8183072eee..df99529817 100644 --- a/src/compiler/c.rs +++ b/src/compiler/c.rs @@ -1566,7 +1566,11 @@ impl<'a> HashKeyParams<'a> { m.update(CACHE_VERSION); m.update(self.language.as_str().as_bytes()); for arg in self.arguments { - arg.hash(&mut HashToDigest { digest: &mut m }); + let arg_bytes = arg.as_encoded_bytes(); + let stripped = strip_basedirs(arg_bytes, self.basedirs); + m.update(&stripped); + // Separator so adjacent args don't blur together. + m.update(&[0u8]); } for hash in self.extra_hashes { m.update(hash.as_bytes()); @@ -1761,6 +1765,64 @@ mod test { assert_eq!(h1, hash_with_basedirs(preprocessed1, &multi_basedirs)); } + #[test] + fn test_hash_key_args_basedirs() { + // Args carry absolute paths into the cache key for any flag whose + // disposition ends up in `common_args` with the path on its own + // (Separated disposition). On MSVC, `-external:I ` is the + // canonical case. Verify that basedirs strip neutralises the + // per-workspace difference. + let basedirs = [ + b"/home/user1/project".to_vec(), + b"/home/user2/project".to_vec(), + ]; + let preprocessed = b"int main() { return 0; }"; + + let args1: Vec = ovec![ + "-c", + "-external:I", + "/home/user1/project/include", + "-o", + "/home/user1/project/build/main.o" + ]; + let args2: Vec = ovec![ + "-c", + "-external:I", + "/home/user2/project/include", + "-o", + "/home/user2/project/build/main.o" + ]; + + let h1 = HashKeyParams::new("abcd", Language::C, &args1, preprocessed) + .with_basedirs(&basedirs) + .compute(); + let h2 = HashKeyParams::new("abcd", Language::C, &args2, preprocessed) + .with_basedirs(&basedirs) + .compute(); + assert_eq!( + h1, h2, + "Args with workspace-relative absolute paths should hash equal under basedirs" + ); + + // Without basedirs, same args produce divergent hashes — proves args + // (not just preprocessor output) feed into the key. + let h1_nb = HashKeyParams::new("abcd", Language::C, &args1, preprocessed).compute(); + let h2_nb = HashKeyParams::new("abcd", Language::C, &args2, preprocessed).compute(); + assert_neq!(h1_nb, h2_nb); + + // Sanity: adjacent args must not blur — the inter-arg separator + // matters. `["-Ia", "b"]` and `["-I", "ab"]` are distinct cmdlines. + let args_a: Vec = ovec!["-Ia", "b"]; + let args_b: Vec = ovec!["-I", "ab"]; + let h_a = HashKeyParams::new("abcd", Language::C, &args_a, preprocessed) + .with_basedirs(&basedirs) + .compute(); + let h_b = HashKeyParams::new("abcd", Language::C, &args_b, preprocessed) + .with_basedirs(&basedirs) + .compute(); + assert_neq!(h_a, h_b); + } + #[test] fn test_language_from_file_name() { fn t(extension: &str, expected: Language) { From 9049e7e667263ccc64abfdcd0e5471069bdfdeb2 Mon Sep 17 00:00:00 2001 From: "Sung, Po Han" Date: Fri, 15 May 2026 10:51:55 +0800 Subject: [PATCH 2/3] hash: collapse escaped backslashes in preprocessor output on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MSVC's preprocessor expands `__FILE__` (e.g. inside `_wassert(L"...", L"", line)`) into a *C source-form* string literal where every backslash is doubled. `normalize_win_path` inside `strip_basedirs` walks the output byte-by-byte and maps each `\` to `/`, so a doubled `\\path` becomes `//path` — which never matches a basedir stored as `/path`. The result on Windows MSVC + basedirs is: cache key includes the absolute workspace path verbatim, defeating sharing across CI jobs with ephemeral workdirs. Collapse `\\` byte pairs into a single `\` before handing the preprocessor output to `strip_basedirs`. Gate both the helper and its unit test on `target_os = "windows"` since non-Windows preprocessor output already uses forward slashes natively. Tests (Windows-only): - `test_collapse_escaped_backslashes`: trivial cases, the headline case, and odd-length runs. - `test_hash_key_basedirs_windows_escaped_backslash_paths`: full `HashKeyParams::compute` round-trip showing two MSVC-shaped preprocessor outputs from two distinct workspaces hash equal under basedirs. --- src/compiler/c.rs | 85 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 83 insertions(+), 2 deletions(-) diff --git a/src/compiler/c.rs b/src/compiler/c.rs index df99529817..c52677801d 100644 --- a/src/compiler/c.rs +++ b/src/compiler/c.rs @@ -1448,6 +1448,38 @@ impl pkg::ToolchainPackager for CToolchainPackager { /// The cache is versioned by the inputs to `HashKeyParams::compute`. pub const CACHE_VERSION: &[u8] = b"12"; +/// Collapse `\\` byte pairs into a single `\`. +#[cfg(target_os = "windows")] +fn collapse_escaped_backslashes(input: &[u8]) -> Vec { + let mut iter = input.iter().peekable(); + std::iter::from_fn(move || { + let &b = iter.next()?; + if b == b'\\' { + iter.next_if_eq(&&b'\\'); + } + Some(b) + }) + .collect() +} + +/// On Windows, collapse C/C++ source-form `\\` escapes in preprocessor output +/// before basedirs prefix-stripping. MSVC keeps `__FILE__`-style paths in +/// source form, so `normalize_win_path` would otherwise map each `\\` to +/// `//` and basedirs (stored as `/...`) would never match. +#[cfg(target_os = "windows")] +fn maybe_collapse_escaped_backslashes<'a>(input: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + if basedirs.iter().any(|b| !b.is_empty()) { + Cow::Owned(collapse_escaped_backslashes(input)) + } else { + Cow::Borrowed(input) + } +} + +#[cfg(not(target_os = "windows"))] +fn maybe_collapse_escaped_backslashes<'a>(input: &'a [u8], _basedirs: &[Vec]) -> Cow<'a, [u8]> { + Cow::Borrowed(input) +} + /// Environment variables that are factored into the cache key. static CACHED_ENV_VARS: LazyLock> = LazyLock::new(|| { [ @@ -1584,8 +1616,9 @@ impl<'a> HashKeyParams<'a> { } } - // Strip basedirs from preprocessor output if configured - let preprocessor_output_to_hash = strip_basedirs(self.preprocessor_output, self.basedirs); + let preprocessor_for_strip = + maybe_collapse_escaped_backslashes(self.preprocessor_output, self.basedirs); + let preprocessor_output_to_hash = strip_basedirs(&preprocessor_for_strip, self.basedirs); m.update(&preprocessor_output_to_hash); m.finish() @@ -1765,6 +1798,54 @@ mod test { assert_eq!(h1, hash_with_basedirs(preprocessed1, &multi_basedirs)); } + #[cfg(target_os = "windows")] + #[test] + fn test_hash_key_basedirs_windows_escaped_backslash_paths() { + // MSVC `__FILE__` stringification produces source-form paths with + // doubled backslashes; basedirs are stored already normalised to + // forward slashes by `Config::basedir`. + let args = ovec!["/c"]; + let preprocessed1 = + b"_wassert(L\"x\", L\"c:\\\\users\\\\user1\\\\work\\\\xyz\\\\file.c\", 42);\n"; + let preprocessed2 = + b"_wassert(L\"x\", L\"d:\\\\actions\\\\runner\\\\hex\\\\repo\\\\file.c\", 42);\n"; + let basedirs = [ + b"c:/users/user1/work/xyz".to_vec(), + b"d:/actions/runner/hex/repo".to_vec(), + ]; + + let h1 = HashKeyParams::new("abcd", Language::C, &args, preprocessed1) + .with_basedirs(&basedirs) + .compute(); + let h2 = HashKeyParams::new("abcd", Language::C, &args, preprocessed2) + .with_basedirs(&basedirs) + .compute(); + assert_eq!( + h1, h2, + "Hashes should match after collapsing escaped backslashes" + ); + } + + #[cfg(target_os = "windows")] + #[test] + fn test_collapse_escaped_backslashes() { + // Trivial cases + assert_eq!(collapse_escaped_backslashes(b""), b""); + assert_eq!(collapse_escaped_backslashes(b"abc"), b"abc"); + + // The headline case: source-form path emitted by MSVC for __FILE__ + // inside _wassert macros. + assert_eq!( + collapse_escaped_backslashes(b"\"c:\\\\users\\\\user1\\\\file.h\""), + b"\"c:\\users\\user1\\file.h\"" + ); + + // Odd-length runs collapse pair-by-pair, leftmost first. A lone + // trailing backslash (no pair partner) stays untouched. + assert_eq!(collapse_escaped_backslashes(b"a\\b"), b"a\\b"); + assert_eq!(collapse_escaped_backslashes(b"a\\\\\\b"), b"a\\\\b"); + } + #[test] fn test_hash_key_args_basedirs() { // Args carry absolute paths into the cache key for any flag whose From 456bef39c90c86a4d985481f18be38fcaec040d7 Mon Sep 17 00:00:00 2001 From: "Sung, Po Han" Date: Fri, 15 May 2026 10:52:56 +0800 Subject: [PATCH 3/3] hash: strip basedirs from cmdline args in direct-mode hash key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Direct mode (`preprocessor_cache_entry_hash_key`) hashed each arg through `OsStr::hash`, leaving absolute paths in the key. Symmetric to the preprocessor-mode fix in `HashKeyParams::compute`: run each arg through `strip_basedirs`, then a 0-byte separator. Until this lands, direct mode has to be disabled (`SCCACHE_DIRECT=false`) on any CI runner whose workdir changes between invocations, which forces every TU through the preprocessor. Notes: - Same boundary-check caveat as the preprocessor-mode fix: only Separated-disposition args (path at position 0 inside the arg byte slice) are caught. - Include paths stored *inside* a cached `PreprocessorCacheEntry` remain absolute. When workspace B looks up an entry written by workspace A, `result_matches` will fail on the project-local include paths and fall back to the preprocessor-mode key — which is fine, because the preprocessor-mode key is now also basedirs- stripped. So this change cannot regress correctness; the worst it does is fail to skip the preprocessor for a TU whose includes are all under basedirs but happen to be project-local. Test: `test_preprocessor_cache_entry_hash_key_args_basedirs` constructs two workspaces with distinct tempdir roots, verifies that args carrying each workspace's absolute path hash equal under basedirs and divergent without, and checks that the inter-arg separator catches an adjacency regression. --- src/compiler/preprocessor_cache.rs | 175 ++++++++++++++++++++++++++++- 1 file changed, 174 insertions(+), 1 deletion(-) diff --git a/src/compiler/preprocessor_cache.rs b/src/compiler/preprocessor_cache.rs index 61cc889f6d..01574539bb 100644 --- a/src/compiler/preprocessor_cache.rs +++ b/src/compiler/preprocessor_cache.rs @@ -392,7 +392,11 @@ pub fn preprocessor_cache_entry_hash_key( m.update(&[FORMAT_VERSION]); m.update(language.as_str().as_bytes()); for arg in arguments { - arg.hash(&mut HashToDigest { digest: &mut m }); + let arg_bytes = arg.as_encoded_bytes(); + let stripped = strip_basedirs(arg_bytes, basedirs); + m.update(&stripped); + // Separator so adjacent args don't blur together. + m.update(&[0u8]); } for hash in extra_hashes { m.update(hash.as_bytes()); @@ -772,4 +776,173 @@ mod test { "Hashes should be different without basedirs for files in different directories" ); } + + #[test] + fn test_preprocessor_cache_entry_hash_key_args_basedirs() { + #[cfg(target_os = "windows")] + use crate::util::normalize_win_path; + use std::fs; + use tempfile::TempDir; + + // Two distinct workspace roots simulating two CI workdirs / worktrees. + let dir1 = TempDir::new().unwrap(); + let dir2 = TempDir::new().unwrap(); + + let dir_bytes = |p: &Path| -> Vec { + let bytes = p.to_string_lossy().into_owned().into_bytes(); + #[cfg(target_os = "windows")] + { + normalize_win_path(&bytes) + } + #[cfg(not(target_os = "windows"))] + { + bytes + } + }; + + let basedirs = vec![dir_bytes(dir1.path()), dir_bytes(dir2.path())]; + + // Same source content placed in each workspace at the same relative path. + let file1_path = dir1.path().join("test.c"); + let file2_path = dir2.path().join("test.c"); + let content = b"int main() { return 0; }"; + fs::write(&file1_path, content).unwrap(); + fs::write(&file2_path, content).unwrap(); + + // Args that embed the absolute workspace path. We test the + // Separated-disposition shape: `["-external:I", "/include"]` + // — this is the form MSVC system-include flags take after sccache's + // arg parser normalizes them, and is the realistic case in CI today. + // + // (The Concatenated shape — `["-IC:\workspace\include"]` — is NOT + // caught by `strip_basedirs` since its boundary check rejects matches + // where the previous byte is alphanumeric. Real MSVC builds rarely + // hit this in `common_args` because `/I` goes to `preprocessor_args`, + // not `common_args`. Concatenated stripping is a future enhancement.) + let inc1_path: OsString = format!("{}/include", dir1.path().display()).into(); + let inc2_path: OsString = format!("{}/include", dir2.path().display()).into(); + let out1_path: OsString = format!("{}/build/test.o", dir1.path().display()).into(); + let out2_path: OsString = format!("{}/build/test.o", dir2.path().display()).into(); + let args1 = vec![ + OsString::from("-c"), + OsString::from("-external:I"), + inc1_path, + OsString::from("-o"), + out1_path, + ]; + let args2 = vec![ + OsString::from("-c"), + OsString::from("-external:I"), + inc2_path, + OsString::from("-o"), + out2_path, + ]; + + let config = PreprocessorCacheModeConfig::activated(); + + // With basedirs, args carrying different absolute paths hash equal. + let h1 = preprocessor_cache_entry_hash_key( + "test_digest", + Language::C, + &args1, + &[], + &[], + &file1_path, + false, + config, + &basedirs, + ) + .unwrap() + .unwrap(); + + let h2 = preprocessor_cache_entry_hash_key( + "test_digest", + Language::C, + &args2, + &[], + &[], + &file2_path, + false, + config, + &basedirs, + ) + .unwrap() + .unwrap(); + + assert_eq!( + h1, h2, + "Direct-mode hash should be equal across workspaces when args paths are covered by basedirs" + ); + + // Without basedirs, the same args produce divergent hashes — proves + // the args (not just the input_file path) feed into the key. + let h1_nb = preprocessor_cache_entry_hash_key( + "test_digest", + Language::C, + &args1, + &[], + &[], + &file1_path, + false, + config, + &[], + ) + .unwrap() + .unwrap(); + + let h2_nb = preprocessor_cache_entry_hash_key( + "test_digest", + Language::C, + &args2, + &[], + &[], + &file2_path, + false, + config, + &[], + ) + .unwrap() + .unwrap(); + + assert_ne!( + h1_nb, h2_nb, + "Direct-mode hash should differ across workspaces when basedirs are not provided" + ); + + // Sanity: adjacent args don't blur into each other. `["-Ia", "b"]` + // must hash differently from `["-I", "ab"]`. Catches a regression + // where someone removes the inter-arg separator. + let args_a = vec![OsString::from("-Ia"), OsString::from("b")]; + let args_b = vec![OsString::from("-I"), OsString::from("ab")]; + let h_a = preprocessor_cache_entry_hash_key( + "test_digest", + Language::C, + &args_a, + &[], + &[], + &file1_path, + false, + config, + &basedirs, + ) + .unwrap() + .unwrap(); + let h_b = preprocessor_cache_entry_hash_key( + "test_digest", + Language::C, + &args_b, + &[], + &[], + &file1_path, + false, + config, + &basedirs, + ) + .unwrap() + .unwrap(); + assert_ne!( + h_a, h_b, + "Adjacent args must not blur — inter-arg separator missing?" + ); + } }