From: Joel Rosdahl <joel@rosdahl.net>
Date: Sat, 16 Mar 2024 13:42:00 +0000 (+0100)
Subject: fix: Reduce the risk of false positive direct mode hits
X-Git-Tag: v4.10~80
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b672f7ebad0967551d48cf93a65e052fb5cb3049;p=thirdparty%2Fccache.git

fix: Reduce the risk of false positive direct mode hits

To reduce the risk of getting a false positive hit when a directory
early in the include path now exists, record whether include directories
and similar input paths exist. Note that this is not 100% waterproof
since it only detects newly appearing directories and not newly
appearing header files.
---

diff --git a/src/ccache/ccache.cpp b/src/ccache/ccache.cpp
index 6a91a209..bd0497cd 100644
--- a/src/ccache/ccache.cpp
+++ b/src/ccache/ccache.cpp
@@ -1722,6 +1722,29 @@ hash_argument(const Context& ctx,
     }
   }
 
+  // From the manual: "files that were used by the compiler are recorded, but
+  // header files that were *not* used, but would have been used if they
+  // existed, are not". To reduce the risk of getting a false positive hit when
+  // a directory early in the include path now exists, record whether include
+  // directories and similar input paths exist. Note that this is not 100%
+  // waterproof since it only detects newly appearing directories and not newly
+  // appearing header files.
+  if (direct_mode) {
+    std::optional<std::string_view> path;
+    if (compopt_takes_path(args[i]) && i + 1 < args.size()) {
+      path = args[i + 1];
+    } else {
+      auto p = compopt_prefix_takes_path(args[i]);
+      if (p) {
+        path = *p;
+      };
+    }
+    if (path) {
+      hash.hash_delimiter("path exists");
+      hash.hash(FMT("{} {}", *path, fs::exists(*path) ? "1" : "0"));
+    }
+  }
+
   if (ctx.args_info.generating_dependencies) {
     std::optional<std::string_view> option;
     std::optional<std::string_view> value;
diff --git a/src/ccache/compopt.cpp b/src/ccache/compopt.cpp
index f3c3be0f..a112ab9b 100644
--- a/src/ccache/compopt.cpp
+++ b/src/ccache/compopt.cpp
@@ -288,8 +288,6 @@ compopt_takes_concat_arg(std::string_view option)
   return co && (co->type & TAKES_CONCAT_ARG);
 }
 
-// Determines if the prefix of the option matches any option and affects the
-// preprocessor.
 bool
 compopt_prefix_affects_cpp_output(std::string_view option)
 {
@@ -298,8 +296,6 @@ compopt_prefix_affects_cpp_output(std::string_view option)
   return co && (co->type & TAKES_CONCAT_ARG) && (co->type & AFFECTS_CPP);
 }
 
-// Determines if the prefix of the option matches any option and affects the
-// preprocessor.
 bool
 compopt_prefix_affects_compiler_output(std::string_view option)
 {
@@ -307,3 +303,15 @@ compopt_prefix_affects_compiler_output(std::string_view option)
   const CompOpt* co = find_prefix(option);
   return co && (co->type & TAKES_CONCAT_ARG) && (co->type & AFFECTS_COMP);
 }
+
+std::optional<std::string_view>
+compopt_prefix_takes_path(std::string_view option)
+{
+  // Prefix options have to take concatenated args.
+  const CompOpt* co = find_prefix(option);
+  if (co && (co->type & TAKES_CONCAT_ARG) && (co->type & TAKES_PATH)) {
+    return option.substr(co->name.length());
+  } else {
+    return std::nullopt;
+  }
+}
diff --git a/src/ccache/compopt.hpp b/src/ccache/compopt.hpp
index 3adfe46d..99ce2566 100644
--- a/src/ccache/compopt.hpp
+++ b/src/ccache/compopt.hpp
@@ -18,6 +18,7 @@
 
 #pragma once
 
+#include <optional>
 #include <string_view>
 
 bool compopt_short(bool (*fn)(std::string_view option),
@@ -31,3 +32,5 @@ bool compopt_takes_arg(std::string_view option);
 bool compopt_takes_concat_arg(std::string_view option);
 bool compopt_prefix_affects_cpp_output(std::string_view option);
 bool compopt_prefix_affects_compiler_output(std::string_view option);
+std::optional<std::string_view>
+compopt_prefix_takes_path(std::string_view option);
diff --git a/test/suites/direct.bash b/test/suites/direct.bash
index aa3670d0..806521e9 100644
--- a/test/suites/direct.bash
+++ b/test/suites/direct.bash
@@ -1361,4 +1361,42 @@ EOF
     expect_stat files_in_cache 2
 
     expect_equal_content $manifest_file saved.manifest
+
+    # -------------------------------------------------------------------------
+    TEST "Detection of appearing include directories"
+
+    cat <<EOF >main.c
+#include <foo.h>
+EOF
+    backdate main.c
+    mkdir a
+    cat <<EOF >a/foo.h
+char x[] = "content_a";
+EOF
+    backdate a/foo.h
+
+    $CCACHE_COMPILE -c -Ib -Ia main.c
+    expect_contains main.o content_a
+    expect_stat direct_cache_hit 0
+    expect_stat cache_miss 1
+
+    $CCACHE_COMPILE -c -Ib -Ia main.c
+    expect_contains main.o content_a
+    expect_stat direct_cache_hit 1
+    expect_stat cache_miss 1
+
+    mkdir b
+    cat <<EOF >b/foo.h
+char x[] = "content_b";
+EOF
+
+    $CCACHE_COMPILE -c -Ib -Ia main.c
+    expect_contains main.o content_b
+    expect_stat direct_cache_hit 1
+    expect_stat cache_miss 2
+
+    $CCACHE_COMPILE -c -Ib -Ia main.c
+    expect_contains main.o content_b
+    expect_stat direct_cache_hit 2
+    expect_stat cache_miss 2
 }
diff --git a/unittest/test_compopt.cpp b/unittest/test_compopt.cpp
index 965d5597..21aeeb0c 100644
--- a/unittest/test_compopt.cpp
+++ b/unittest/test_compopt.cpp
@@ -20,6 +20,8 @@
 
 #include <doctest.h>
 
+#include <iostream> // for doctest stringification of std::string_view
+
 bool compopt_verify_sortedness_and_flags();
 
 TEST_SUITE_BEGIN("compopt");
@@ -93,4 +95,10 @@ TEST_CASE("prefix_affects_compiler_output")
   CHECK(!compopt_prefix_affects_compiler_output("-Wa"));
 }
 
+TEST_CASE("prefix_takes_path")
+{
+  CHECK(compopt_prefix_takes_path("-Dfoo") == std::nullopt);
+  CHECK(*compopt_prefix_takes_path("-Ifoo") == "foo");
+}
+
 TEST_SUITE_END();