]> git.ipfire.org Git - thirdparty/ccache.git/commitdiff
Always include input file path in direct mode hash
authorJoel Rosdahl <joel@rosdahl.net>
Sat, 16 Nov 2019 21:59:33 +0000 (22:59 +0100)
committerJoel Rosdahl <joel@rosdahl.net>
Sun, 17 Nov 2019 14:34:37 +0000 (15:34 +0100)
The “file_macro sloppiness” mode is a way of opting out of inclusion of
the input file path in the direct mode hash. This can produce a false
cache hit in the following scenario:

- a/r.h exists.
- a/x.c has #include "r.h".
- b/x.c is identical to a/x.c.
- Compiling a/x.c records a/r.h in the manifest.
- Compiling b/x.c results in a false cache hit since a/x.c and b/x.c
  share manifests and a/r.h exists.

Therefore, ditch the file_macro sloppiness mode so that the input file
path is always included in the direct mode hash.

This bug has existed ever since the file_macro sloppiness was introduced
in eb5d9bd3beb5 (ccache 3.0). It has remained undetected since
compilations tend to use .d files and ccache before 3.7.5 added an
implicit “-MQ <output_file_path>” argument, thus in practice including
the output file path in the hash and therefore making manifests unique
when the object file path mirrors the source file path. However, ccache
3.7.5 no longer adds the implicit -MQ option, thus exposing the bug.

Fixes #489.

doc/MANUAL.adoc
src/ccache.c
src/ccache.h
src/confitems.c
test/suites/direct.bash
unittest/test_conf.c

index 2ca82c0c4ae241f358e13d84ffbf679b76ece7cf..0964152c2cc3622ecd1e2e6b87ab927dd834448e 100644 (file)
@@ -558,13 +558,6 @@ still has to do _some_ preprocessing (like macros).
     computing the manifest hash. This is useful if you use Xcode, which uses an
     index store path derived from the local project path. Note that the index
     store won't be updated correctly on cache hits if you enable this option.
-*file_macro*::
-    ccache normally includes the input file path in the hash in order to be
-    able to produce the correct object file if the source code includes a
-    `__FILE__` macro. If you know that `__FILE__` isn't used in practise, or
-    don't care if ccache produces objects where `__FILE__` is expanded to the
-    wrong path, you can set *sloppiness* to *file_macro*. ccache will then
-    exclude the input file path from the hash.
 *file_stat_matches*::
     ccache normally examines a file's contents to determine whether it matches
     the cached version. With this option set, ccache will consider a file as
@@ -1227,12 +1220,10 @@ problems and what may be done to increase the hit rate:
    output. If you know that `__DATE__` isn't used in practise, or don't care if
    ccache produces objects where `__DATE__` is expanded to something in the
    past, you can set *sloppiness* to *time_macros*.
-** The input file path has changed. ccache normally includes the input file
-   path in the hash in order to be able to produce the correct object file if
-   the source code includes a `__FILE__` macro. If you know that `__FILE__`
-   isn't used in practise, or don't care if ccache produces objects where
-   `__FILE__` is expanded to the wrong path, you can set *sloppiness* to
-   *file_macro*. ccache will then exclude the input file path from the hash.
+** The input file path has changed. ccache includes the input file path in the
+   direct mode hash to be able to take relative include files into account and
+   to produce a correct object file if the source code includes a `__FILE__`
+   macro.
 * If ``cache miss'' has been incremented even though the same code has been
   compiled and cached before, ccache has either detected that something has
   changed anyway or a cleanup has been performed (either explicitly or
index 7857be4adb5ce84d6fca3ceacef689d8678ab069..dbfd82134454b88a7e64e83e18d0c0f89975e681 100644 (file)
@@ -2234,12 +2234,19 @@ calculate_object_hash(struct args *args, struct hash *hash, int direct_mode)
                        }
                }
 
-               if (!(conf->sloppiness & SLOPPY_FILE_MACRO)) {
-                       // The source code file or an include file may contain __FILE__, so make
-                       // sure that the hash is unique for the file name.
-                       hash_delimiter(hash, "inputfile");
-                       hash_string(hash, input_file);
-               }
+               // Make sure that the direct mode hash is unique for the input file path.
+               // If this would not be the case:
+               //
+               // * An false cache hit may be produced. Scenario:
+               //   - a/r.h exists.
+               //   - a/x.c has #include "r.h".
+               //   - b/x.c is identical to a/x.c.
+               //   - Compiling a/x.c records a/r.h in the manifest.
+               //   - Compiling b/x.c results in a false cache hit since a/x.c and b/x.c
+               //     share manifests and a/r.h exists.
+               // * The expansion of __FILE__ may be incorrect.
+               hash_delimiter(hash, "inputfile");
+               hash_string(hash, input_file);
 
                hash_delimiter(hash, "sourcecode");
                int result = hash_source_code_file(conf, hash, input_file);
index ac12a14b0e362d21403674d76c367ba7695e4c08..cabc980d653f392d27fff3277962fd59c23d69df 100644 (file)
@@ -87,22 +87,21 @@ extern enum guessed_compiler guessed_compiler;
 
 #define SLOPPY_INCLUDE_FILE_MTIME (1U << 0)
 #define SLOPPY_INCLUDE_FILE_CTIME (1U << 1)
-#define SLOPPY_FILE_MACRO (1U << 2)
-#define SLOPPY_TIME_MACROS (1U << 3)
-#define SLOPPY_PCH_DEFINES (1U << 4)
+#define SLOPPY_TIME_MACROS (1U << 2)
+#define SLOPPY_PCH_DEFINES (1U << 3)
 // Allow us to match files based on their stats (size, mtime, ctime), without
 // looking at their contents.
-#define SLOPPY_FILE_STAT_MATCHES (1U << 5)
+#define SLOPPY_FILE_STAT_MATCHES (1U << 4)
 // Allow us to not include any system headers in the manifest include files,
 // similar to -MM versus -M for dependencies.
-#define SLOPPY_SYSTEM_HEADERS (1U << 6)
+#define SLOPPY_SYSTEM_HEADERS (1U << 5)
 // Allow us to ignore ctimes when comparing file stats, so we can fake mtimes
 // if we want to (it is much harder to fake ctimes, requires changing clock)
-#define SLOPPY_FILE_STAT_MATCHES_CTIME (1U << 7)
+#define SLOPPY_FILE_STAT_MATCHES_CTIME (1U << 6)
 // Allow us to not include the -index-store-path option in the manifest hash.
-#define SLOPPY_CLANG_INDEX_STORE (1U << 8)
+#define SLOPPY_CLANG_INDEX_STORE (1U << 7)
 // Ignore locale settings.
-#define SLOPPY_LOCALE (1U << 9)
+#define SLOPPY_LOCALE (1U << 8)
 
 #define str_eq(s1, s2) (strcmp((s1), (s2)) == 0)
 #define str_startswith(s, prefix) \
index 4b70db4ac8eadcd9bf999e55fa528ebd109a1953..15064e71fef7ace3879b3909e48a0711c5d301b9 100644 (file)
@@ -122,9 +122,7 @@ confitem_parse_sloppiness(const char *str, void *result, char **errmsg)
        char *word;
        char *saveptr = NULL;
        while ((word = strtok_r(q, ", ", &saveptr))) {
-               if (str_eq(word, "file_macro")) {
-                       *value |= SLOPPY_FILE_MACRO;
-               } else if (str_eq(word, "file_stat_matches")) {
+               if (str_eq(word, "file_stat_matches")) {
                        *value |= SLOPPY_FILE_STAT_MATCHES;
                } else if (str_eq(word, "file_stat_matches_ctime")) {
                        *value |= SLOPPY_FILE_STAT_MATCHES_CTIME;
@@ -156,9 +154,6 @@ confitem_format_sloppiness(const void *value)
 {
        const unsigned *sloppiness = (const unsigned *)value;
        char *s = x_strdup("");
-       if (*sloppiness & SLOPPY_FILE_MACRO) {
-               reformat(&s, "%sfile_macro, ", s);
-       }
        if (*sloppiness & SLOPPY_INCLUDE_FILE_MTIME) {
                reformat(&s, "%sinclude_file_mtime, ", s);
        }
index 137304433119b1a88ecc1d483a0828b086c3bc21..fca072f7574f69c6b4d85ab2ed80e5c13bbdd401 100644 (file)
@@ -554,12 +554,13 @@ EOF
     expect_stat 'cache miss' 1
 
     # -------------------------------------------------------------------------
-    TEST "__FILE__ in source file disables direct mode"
+    TEST "The source file path is included in the hash"
 
     cat <<EOF >file.c
 #define file __FILE__
 int test;
 EOF
+    cp file.c file2.c
 
     $CCACHE_COMPILE -c file.c
     expect_stat 'cache hit (direct)' 0
@@ -571,47 +572,34 @@ EOF
     expect_stat 'cache hit (preprocessed)' 0
     expect_stat 'cache miss' 1
 
-    $CCACHE_COMPILE -c `pwd`/file.c
+    $CCACHE_COMPILE -c file2.c
     expect_stat 'cache hit (direct)' 1
     expect_stat 'cache hit (preprocessed)' 0
     expect_stat 'cache miss' 2
 
-    # -------------------------------------------------------------------------
-    TEST "__FILE__ in include file disables direct mode"
-
-    cat <<EOF >file.h
-#define file __FILE__
-int test;
-EOF
-    backdate file.h
-    cat <<EOF >file_h.c
-#include "file.h"
-EOF
-
-    $CCACHE_COMPILE -c file_h.c
-    expect_stat 'cache hit (direct)' 0
+    $CCACHE_COMPILE -c file2.c
+    expect_stat 'cache hit (direct)' 2
     expect_stat 'cache hit (preprocessed)' 0
-    expect_stat 'cache miss' 1
+    expect_stat 'cache miss' 2
 
-    $CCACHE_COMPILE -c file_h.c
-    expect_stat 'cache hit (direct)' 1
+    $CCACHE_COMPILE -c $(pwd)/file.c
+    expect_stat 'cache hit (direct)' 2
     expect_stat 'cache hit (preprocessed)' 0
-    expect_stat 'cache miss' 1
-
-    mv file_h.c file2_h.c
+    expect_stat 'cache miss' 3
 
-    $CCACHE_COMPILE -c `pwd`/file2_h.c
-    expect_stat 'cache hit (direct)' 1
+    $CCACHE_COMPILE -c $(pwd)/file.c
+    expect_stat 'cache hit (direct)' 3
     expect_stat 'cache hit (preprocessed)' 0
-    expect_stat 'cache miss' 2
+    expect_stat 'cache miss' 3
 
     # -------------------------------------------------------------------------
-    TEST "__FILE__ in source file ignored if sloppy"
+    TEST "The source file path is included even if sloppiness = file_macro"
 
     cat <<EOF >file.c
 #define file __FILE__
 int test;
 EOF
+    cp file.c file2.c
 
     CCACHE_SLOPPINESS="$DEFAULT_SLOPPINESS file_macro" $CCACHE_COMPILE -c file.c
     expect_stat 'cache hit (direct)' 0
@@ -623,39 +611,66 @@ EOF
     expect_stat 'cache hit (preprocessed)' 0
     expect_stat 'cache miss' 1
 
-    CCACHE_SLOPPINESS="$DEFAULT_SLOPPINESS file_macro" $CCACHE_COMPILE -c `pwd`/file.c
+    CCACHE_SLOPPINESS="$DEFAULT_SLOPPINESS file_macro" $CCACHE_COMPILE -c file2.c
+    expect_stat 'cache hit (direct)' 1
+    expect_stat 'cache hit (preprocessed)' 0
+    expect_stat 'cache miss' 2
+
+    CCACHE_SLOPPINESS="$DEFAULT_SLOPPINESS file_macro" $CCACHE_COMPILE -c file2.c
     expect_stat 'cache hit (direct)' 2
     expect_stat 'cache hit (preprocessed)' 0
-    expect_stat 'cache miss' 1
+    expect_stat 'cache miss' 2
+
+    CCACHE_SLOPPINESS="$DEFAULT_SLOPPINESS file_macro" $CCACHE_COMPILE -c $(pwd)/file.c
+    expect_stat 'cache hit (direct)' 2
+    expect_stat 'cache hit (preprocessed)' 0
+    expect_stat 'cache miss' 3
+
+    CCACHE_SLOPPINESS="$DEFAULT_SLOPPINESS file_macro" $CCACHE_COMPILE -c $(pwd)/file.c
+    expect_stat 'cache hit (direct)' 3
+    expect_stat 'cache hit (preprocessed)' 0
+    expect_stat 'cache miss' 3
 
     # -------------------------------------------------------------------------
-    TEST "__FILE__ in include file ignored if sloppy"
+    TEST "Relative includes for identical source code in different directories"
 
-    cat <<EOF >file.h
-#define file __FILE__
-int test;
+    mkdir a
+    cat <<EOF >a/file.c
+#include "file.h"
+EOF
+    cat <<EOF >a/file.h
+int x = 1;
 EOF
-    backdate file.h
-    cat <<EOF >file_h.c
+    backdate a/file.h
+
+    mkdir b
+    cat <<EOF >b/file.c
 #include "file.h"
 EOF
+    cat <<EOF >b/file.h
+int x = 2;
+EOF
+    backdate b/file.h
 
-    CCACHE_SLOPPINESS="$DEFAULT_SLOPPINESS file_macro" $CCACHE_COMPILE -c file_h.c
+    $CCACHE_COMPILE -c a/file.c
     expect_stat 'cache hit (direct)' 0
     expect_stat 'cache hit (preprocessed)' 0
     expect_stat 'cache miss' 1
 
-    CCACHE_SLOPPINESS="$DEFAULT_SLOPPINESS file_macro" $CCACHE_COMPILE -c file_h.c
+    $CCACHE_COMPILE -c a/file.c
     expect_stat 'cache hit (direct)' 1
     expect_stat 'cache hit (preprocessed)' 0
     expect_stat 'cache miss' 1
 
-    mv file_h.c file2_h.c
+    $CCACHE_COMPILE -c b/file.c
+    expect_stat 'cache hit (direct)' 1
+    expect_stat 'cache hit (preprocessed)' 0
+    expect_stat 'cache miss' 2
 
-    CCACHE_SLOPPINESS="$DEFAULT_SLOPPINESS file_macro" $CCACHE_COMPILE -c `pwd`/file2_h.c
+    $CCACHE_COMPILE -c b/file.c
     expect_stat 'cache hit (direct)' 2
     expect_stat 'cache hit (preprocessed)' 0
-    expect_stat 'cache miss' 1
+    expect_stat 'cache miss' 2
 
     # -------------------------------------------------------------------------
     TEST "__TIME__ in source file disables direct mode"
index da57bb4a0ca548e813405d9efa40c84f3803bb6b..bc458fce247e1d717678e0292f97a45888f882a8 100644 (file)
@@ -132,7 +132,7 @@ TEST(conf_read_valid_config)
                "read_only_direct = true\n"
                "recache = true\n"
                "run_second_cpp = false\n"
-               "sloppiness =     file_macro   ,time_macros,  include_file_mtime,include_file_ctime,file_stat_matches,file_stat_matches_ctime,pch_defines ,  no_system_headers,system_headers,clang_index_store\n"
+               "sloppiness =     time_macros   ,include_file_mtime  include_file_ctime,file_stat_matches,file_stat_matches_ctime,pch_defines ,  no_system_headers,system_headers,clang_index_store\n"
                "stats = false\n"
                "temporary_dir = ${USER}_foo\n"
                "umask = 777\n"
@@ -175,7 +175,6 @@ TEST(conf_read_valid_config)
        CHECK_INT_EQ(
                SLOPPY_INCLUDE_FILE_MTIME
                |SLOPPY_INCLUDE_FILE_CTIME
-               |SLOPPY_FILE_MACRO
                |SLOPPY_TIME_MACROS
                |SLOPPY_FILE_STAT_MATCHES
                |SLOPPY_FILE_STAT_MATCHES_CTIME
@@ -493,7 +492,7 @@ TEST(conf_print_items)
                true,
                true,
                .run_second_cpp = false,
-               SLOPPY_FILE_MACRO|SLOPPY_INCLUDE_FILE_MTIME|
+               SLOPPY_INCLUDE_FILE_MTIME|
                SLOPPY_INCLUDE_FILE_CTIME|SLOPPY_TIME_MACROS|
                SLOPPY_FILE_STAT_MATCHES|SLOPPY_FILE_STAT_MATCHES_CTIME|
                SLOPPY_PCH_DEFINES|SLOPPY_SYSTEM_HEADERS|SLOPPY_CLANG_INDEX_STORE,
@@ -546,7 +545,7 @@ TEST(conf_print_items)
        CHECK_STR_EQ("read_only_direct = true", received_conf_items[n++].descr);
        CHECK_STR_EQ("recache = true", received_conf_items[n++].descr);
        CHECK_STR_EQ("run_second_cpp = false", received_conf_items[n++].descr);
-       CHECK_STR_EQ("sloppiness = file_macro, include_file_mtime,"
+       CHECK_STR_EQ("sloppiness = include_file_mtime,"
                     " include_file_ctime, time_macros, pch_defines,"
                     " file_stat_matches, file_stat_matches_ctime, system_headers,"
                     " clang_index_store",