Strip basedirs from the compiler arguments too

README.md

@@ -344,6 +344,10 @@ export SCCACHE_BASEDIRS="/home/user/project:/home/user/workspace"
 
 Path matching is **case-insensitive** on Windows and **case-sensitive** on other operating systems.
 
+Base directories are stripped from the preprocessed source and from the compiler arguments alike. The arguments matter because a flag can name the tree it is building: `-ffile-prefix-map=/home/user/project=.` makes the object file independent of where the tree is checked out, but the flag itself is not, and it is hashed verbatim.
+
+In an argument only the places that are expected to spell a pathname are considered: the whole argument, the value of an option written with an `=` (either half of a prefix map), and the value glued to a short option such as `-I`. A base directory appearing anywhere else is left alone, so a definition the compiler bakes into the output verbatim, `-DROOT="/home/user/project"`, still counts. A match also has to end where a path component ends, so a sibling `/home/user/project-docs` is not one.
+
 This is similar to ccache's `CCACHE_BASEDIR` and helps when:
 * Building the same project from different directories
 * Sharing cache between CI jobs with different checkout paths

docs/Configuration.md

@@ -20,6 +20,13 @@ server_startup_timeout_ms = 10000
 #
 # Path matching is case-insensitive on Windows and case-sensitive on other OSes.
 #
+# Applies to the preprocessed source and to the compiler arguments
+# alike, so that a flag naming the tree, such as
+# -ffile-prefix-map=/home/user/project=., does not tie the cache
+# entry to one checkout. In an argument only the places expected to
+# spell a pathname are considered: the whole argument, the value of
+# an =-separated option, and the value glued to a short option.
+#
 # Example:
 #   basedir = ["/home/user/project"] results in the path prefix rewrite:
 #   "/home/user/project/src/main.c" -> "src/main.c"

src/compiler/c.rs

@@ -27,7 +27,7 @@ use crate::dist::pkg;
 use crate::mock_command::CommandCreatorSync;
 use crate::util::{
     Digest, HashToDigest, MetadataCtimeExt, TimeMacroFinder, Timestamp, decode_path, encode_path,
-    hash_all, strip_basedirs,
+    hash_all, strip_basedirs, strip_basedirs_from_arg,
 };
 use async_trait::async_trait;
 use fs_err as fs;
@@ -1441,7 +1441,7 @@ impl pkg::ToolchainPackager for CToolchainPackager {
 }
 
 /// The cache is versioned by the inputs to `HashKeyParams::compute`.
-pub const CACHE_VERSION: &[u8] = b"12";
+pub const CACHE_VERSION: &[u8] = b"13";
 
 /// Environment variables that are factored into the cache key.
 static CACHED_ENV_VARS: LazyLock<HashSet<&'static OsStr>> = LazyLock::new(|| {
@@ -1463,6 +1463,20 @@ static CACHED_ENV_VARS: LazyLock<HashSet<&'static OsStr>> = LazyLock::new(|| {
     .collect()
 });
 
+/// Feed the compiler arguments into `m`, with the base directories stripped.
+///
+/// The arguments are hashed verbatim, so any that spell out an absolute path -
+/// `-ffile-prefix-map=/home/user/project=.` above all - would tie the cache
+/// entry to one checkout of the tree. See [`strip_basedirs_from_arg`].
+pub fn hash_arguments(m: &mut Digest, arguments: &[OsString], basedirs: &[Vec<u8>]) {
+    for arg in arguments {
+        // Same shape as OsString's own Hash impl: the bytes, then a separator
+        // that cannot occur in them.
+        m.update(&strip_basedirs_from_arg(arg.as_encoded_bytes(), basedirs));
+        m.update(&[0xff]);
+    }
+}
+
 /// Parameters for computing a hash key for C/C++ compilation caching.
 ///
 /// Construct with required fields via [`HashKeyParams::new`], then add optional
@@ -1560,9 +1574,7 @@ impl<'a> HashKeyParams<'a> {
         m.update(&[self.plusplus as u8]);
         m.update(CACHE_VERSION);
         m.update(self.language.as_str().as_bytes());
-        for arg in self.arguments {
-            arg.hash(&mut HashToDigest { digest: &mut m });
-        }
+        hash_arguments(&mut m, self.arguments, self.basedirs);
         for hash in self.extra_hashes {
             m.update(hash.as_bytes());
         }

src/compiler/preprocessor_cache.rs

@@ -38,10 +38,11 @@ use crate::{
 };
 
 use super::Language;
+use super::c::hash_arguments;
 
 /// The current format is 1 header byte for the version + bincode encoding
 /// of the [`PreprocessorCacheEntry`] struct.
-const FORMAT_VERSION: u8 = 0;
+const FORMAT_VERSION: u8 = 1;
 const MAX_PREPROCESSOR_CACHE_ENTRIES: usize = 100;
 const MAX_PREPROCESSOR_CACHE_FILE_INFO_ENTRIES: usize = 10000;
 
@@ -391,9 +392,7 @@ pub fn preprocessor_cache_entry_hash_key(
     m.update(&[plusplus as u8]);
     m.update(&[FORMAT_VERSION]);
     m.update(language.as_str().as_bytes());
-    for arg in arguments {
-        arg.hash(&mut HashToDigest { digest: &mut m });
-    }
+    hash_arguments(&mut m, arguments, basedirs);
     for hash in extra_hashes {
         m.update(hash.as_bytes());
     }

src/util.rs

@@ -1209,6 +1209,123 @@ pub fn strip_basedirs<'a>(preprocessor_output: &'a [u8], basedirs: &[Vec<u8>]) -
     Cow::Owned(result)
 }
 
+/// Strip the base directories from a single compiler argument, for hashing.
+///
+/// [`strip_basedirs`] looks for a basedir where preprocessor output would put
+/// one: at the start of a line, after a quote, after whitespace.  A compiler
+/// argument spells the same paths differently.  `-I/home/user/project/include`
+/// has no boundary at all before the path, and `-ffile-prefix-map` names a
+/// directory without a trailing slash, as in
+/// `-ffile-prefix-map=/home/user/project=.`, which is what keeps two checkouts
+/// of the same tree from agreeing on a hash.
+///
+/// Only the places where an argument is expected to spell a pathname are
+/// considered, rather than any offset that happens to match:
+///
+/// * the whole argument, as in a source file or the path of a separated
+///   option such as `-include-pch <path>`;
+/// * the value of an option written with an `=`, as in `--sysroot=<path>` or
+///   either half of `-ffile-prefix-map=<path>=<path>`;
+/// * the value glued to a short option, as in `-I<path>` or `-L<path>`.
+///
+/// A basedir sitting anywhere else is left alone, so `-DROOT="/home/user/project"`
+/// keeps a definition the compiler will bake into the output verbatim.  A match
+/// also has to end where a path component ends, so `/home/user/project` does not
+/// match the start of `/home/user/project-docs`.  The longest basedir wins, so a
+/// nested one takes precedence over the tree that contains it.
+pub fn strip_basedirs_from_arg<'a>(arg: &'a [u8], basedirs: &[Vec<u8>]) -> Cow<'a, [u8]> {
+    if basedirs.is_empty() || arg.is_empty() {
+        return Cow::Borrowed(arg);
+    }
+
+    // We must return the original argument on all platforms, so we only
+    // normalize a copy for searching.
+    #[cfg(not(target_os = "windows"))]
+    let haystack: &[u8] = arg;
+    #[cfg(target_os = "windows")]
+    let normalized = normalize_win_path(arg);
+    #[cfg(target_os = "windows")]
+    let haystack: &[u8] = &normalized;
+
+    let mut order: Vec<usize> = (0..basedirs.len()).collect();
+    order.sort_by_key(|&i| std::cmp::Reverse(basedirs[i].len()));
+
+    let mut result: Option<Vec<u8>> = None;
+    let mut copied = 0;
+    for pos in pathname_positions(haystack) {
+        if pos < copied {
+            // Inside a basedir we already stripped.
+            continue;
+        }
+        let rest = &haystack[pos..];
+        let mut matched = 0;
+        for &i in &order {
+            // Basedirs carry a trailing slash, see Config::basedirs. A path
+            // component can also end at the end of the argument, or at a
+            // separator such as the `=` of -ffile-prefix-map.
+            let with_slash = basedirs[i].as_slice();
+            let bare = &with_slash[..with_slash.len() - 1];
+            if rest.starts_with(with_slash) {
+                matched = with_slash.len();
+            } else if rest.starts_with(bare) && matches!(rest.get(bare.len()), None | Some(b'=')) {
+                matched = bare.len();
+            }
+            if matched > 0 {
+                trace!(
+                    "Matched basedir {} at position {} of argument",
+                    String::from_utf8_lossy(with_slash),
+                    pos
+                );
+                break;
+            }
+        }
+        if matched > 0 {
+            let out = result.get_or_insert_with(|| Vec::with_capacity(arg.len()));
+            out.extend_from_slice(&arg[copied..pos]);
+            copied = pos + matched;
+        }
+    }
+
+    match result {
+        Some(mut out) => {
+            out.extend_from_slice(&arg[copied..]);
+            Cow::Owned(out)
+        }
+        None => Cow::Borrowed(arg),
+    }
+}
+
+/// The offsets in a compiler argument where a pathname can begin.
+///
+/// See [`strip_basedirs_from_arg`] for what they are and why the rest of the
+/// argument is off limits.
+fn pathname_positions(arg: &[u8]) -> impl Iterator<Item = usize> + '_ {
+    // The value glued to a short option: everything up to the first byte that
+    // cannot be part of an option name. `-I/path` yields 2, `-isystem/path` 8.
+    let glued = if arg.first() == Some(&b'-') {
+        let name_len = arg[1..]
+            .iter()
+            .take_while(|b| b.is_ascii_alphanumeric() || **b == b'-' || **b == b'_')
+            .count();
+        Some(1 + name_len)
+    } else {
+        None
+    };
+
+    // The whole argument, the value of each `=`-separated option, and the one
+    // glued position, in ascending order and without duplicates.
+    let equals = arg
+        .iter()
+        .enumerate()
+        .filter(|(_, b)| **b == b'=')
+        .map(|(i, _)| i + 1);
+
+    let mut positions: Vec<usize> = std::iter::once(0).chain(glued).chain(equals).collect();
+    positions.sort_unstable();
+    positions.dedup();
+    positions.into_iter().filter(move |&p| p < arg.len())
+}
+
 /// Double every `/` in a normalized path.
 ///
 /// Paths inside preprocessor output are C string literals, so on Windows
@@ -1652,6 +1769,126 @@ mod tests {
         assert!(empty_result.is_empty(), "{:?}", empty_result);
     }
 
+    #[test]
+    fn test_strip_basedirs_from_arg() {
+        let basedir = b"/home/user/project/".to_vec();
+        let strip = |arg: &[u8]| {
+            super::strip_basedirs_from_arg(arg, std::slice::from_ref(&basedir)).into_owned()
+        };
+
+        // The flag that ties an object file to one checkout: the tree is named
+        // without a trailing slash, so the path component ends at the `=`.
+        assert_eq!(
+            strip(b"-ffile-prefix-map=/home/user/project=."),
+            b"-ffile-prefix-map==."
+        );
+        assert_eq!(
+            strip(b"-ffile-prefix-map=/home/user/project/build=."),
+            b"-ffile-prefix-map=build=."
+        );
+        assert_eq!(
+            strip(b"-ffile-prefix-map=/home/user/project/build/=build/"),
+            b"-ffile-prefix-map=build/=build/"
+        );
+
+        // Paths with no boundary before them.
+        assert_eq!(strip(b"-I/home/user/project/include"), b"-Iinclude");
+        assert_eq!(strip(b"/home/user/project/src/main.c"), b"src/main.c");
+
+        // A sibling directory that merely starts with the same bytes is not a
+        // match, in either spelling.
+        assert_eq!(
+            strip(b"-I/home/user/project-docs/include"),
+            b"-I/home/user/project-docs/include"
+        );
+        assert_eq!(
+            strip(b"-ffile-prefix-map=/home/user/project-docs=."),
+            b"-ffile-prefix-map=/home/user/project-docs=."
+        );
+
+        // Untouched without basedirs, and when nothing matches.
+        assert_eq!(
+            &*super::strip_basedirs_from_arg(b"-ffile-prefix-map=/home/user/project=.", &[]),
+            b"-ffile-prefix-map=/home/user/project=."
+        );
+        assert_eq!(strip(b"-O2"), b"-O2");
+        assert_eq!(strip(b""), b"");
+    }
+
+    #[test]
+    fn test_strip_basedirs_from_arg_only_at_pathname_positions() {
+        let basedir = b"/home/user/project/".to_vec();
+        let strip = |arg: &[u8]| {
+            super::strip_basedirs_from_arg(arg, std::slice::from_ref(&basedir)).into_owned()
+        };
+
+        // A macro definition the compiler bakes into the output verbatim: the
+        // path is inside a string literal, not where a pathname is expected.
+        assert_eq!(
+            strip(b"-DROOT=\"/home/user/project\""),
+            b"-DROOT=\"/home/user/project\""
+        );
+
+        // Not the value of an option, just a path further along in one.
+        assert_eq!(
+            strip(b"-Wl,-rpath,/home/user/project/lib"),
+            b"-Wl,-rpath,/home/user/project/lib"
+        );
+        assert_eq!(
+            strip(b"-fplugin-arg-p=x/home/user/project/y"),
+            b"-fplugin-arg-p=x/home/user/project/y"
+        );
+
+        // The value glued to a short option, however long its name.
+        assert_eq!(
+            strip(b"-isystem/home/user/project/abseil"),
+            b"-isystemabseil"
+        );
+        assert_eq!(strip(b"-L/home/user/project/lib"), b"-Llib");
+
+        // The value of an =-separated option, and both halves of a prefix map.
+        assert_eq!(strip(b"--sysroot=/home/user/project/sys"), b"--sysroot=sys");
+        assert_eq!(
+            strip(b"-ffile-prefix-map=/home/user/project/build=/home/user/project"),
+            b"-ffile-prefix-map=build="
+        );
+    }
+
+    #[test]
+    fn test_strip_basedirs_from_arg_skips_positions_inside_a_match() {
+        // A directory whose name contains an `=` is legal, and it puts one of
+        // the positions where an option value could start inside the basedir
+        // itself. If a second basedir then matches at that position, the two
+        // matches overlap, and the tail of the first directory name must not
+        // be stripped a second time.
+        let basedirs = vec![b"/home/user/a=/b/".to_vec(), b"/b/".to_vec()];
+        let strip = |arg: &[u8]| super::strip_basedirs_from_arg(arg, &basedirs).into_owned();
+
+        assert_eq!(strip(b"-I/home/user/a=/b/include"), b"-Iinclude");
+        assert_eq!(strip(b"/home/user/a=/b/src/main.c"), b"src/main.c");
+        assert_eq!(strip(b"--sysroot=/home/user/a=/b/sys"), b"--sysroot=sys");
+    }
+
+    #[test]
+    fn test_strip_basedirs_from_arg_longest_match_wins() {
+        // A build directory nested inside the tree, both listed.
+        let basedirs = vec![
+            b"/home/user/project/".to_vec(),
+            b"/home/user/project/build/".to_vec(),
+        ];
+        assert_eq!(
+            &*super::strip_basedirs_from_arg(
+                b"-ffile-prefix-map=/home/user/project/build=.",
+                &basedirs
+            ),
+            b"-ffile-prefix-map==."
+        );
+        assert_eq!(
+            &*super::strip_basedirs_from_arg(b"-ffile-prefix-map=/home/user/project=.", &basedirs),
+            b"-ffile-prefix-map==."
+        );
+    }
+
     #[test]
     fn test_strip_basedir_simple() {
         // Simple cases

tests/integration/Makefile

@@ -31,7 +31,7 @@ endgroup =
 endif
 
 BACKENDS := redis redis-deprecated memcached memcached-deprecated s3 azblob webdav basedirs multilevel multilevel-chain
-TOOLS := gcc clang cmake cmake-modules cmake-modules-v4 autotools coverage zstd
+TOOLS := gcc clang cmake cmake-modules cmake-modules-v4 autotools coverage zstd file-prefix-map
 
 # Map backends to their compose profiles
 PROFILES_autotools := autotools
@@ -42,6 +42,7 @@ PROFILES_cmake := cmake
 PROFILES_cmake-modules := cmake-modules
 PROFILES_cmake-modules-v4 := cmake-modules-v4
 PROFILES_coverage := coverage
+PROFILES_file-prefix-map := file-prefix-map
 PROFILES_gcc := gcc
 PROFILES_memcached := memcached
 PROFILES_memcached-deprecated := memcached
@@ -62,6 +63,7 @@ SERVICES_cmake :=
 SERVICES_cmake-modules :=
 SERVICES_cmake-modules-v4 :=
 SERVICES_coverage :=
+SERVICES_file-prefix-map :=
 SERVICES_gcc :=
 SERVICES_memcached := memcached
 SERVICES_memcached-deprecated := memcached
@@ -102,6 +104,7 @@ help:
 	@echo "  make test-coverage           Run Rust coverage instrumentation test"
 	@echo "  make test-zstd               Run ZSTD compression levels test"
 	@echo "  make test-basedirs           Run basedirs test across all backends"
+	@echo "  make test-file-prefix-map    Run basedirs vs -ffile-prefix-map test"
 	@echo "  make test-multilevel         Run multi-level cache test across all backends"
 	@echo "  make test-multilevel-chain   Run multi-level backfill chain test (4 levels)"
 	@echo ""