From: Joel Rosdahl Date: Sat, 16 Nov 2019 21:59:33 +0000 (+0100) Subject: Always include input file path in direct mode hash X-Git-Tag: v3.7.6~2 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=404eedde2b8c7a048ea1f68a8e70e49c5ea74df2;p=thirdparty%2Fccache.git Always include input file path in direct mode hash 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 ” 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. --- diff --git a/doc/MANUAL.adoc b/doc/MANUAL.adoc index 2ca82c0c4..0964152c2 100644 --- a/doc/MANUAL.adoc +++ b/doc/MANUAL.adoc @@ -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 diff --git a/src/ccache.c b/src/ccache.c index 7857be4ad..dbfd82134 100644 --- a/src/ccache.c +++ b/src/ccache.c @@ -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); diff --git a/src/ccache.h b/src/ccache.h index ac12a14b0..cabc980d6 100644 --- a/src/ccache.h +++ b/src/ccache.h @@ -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) \ diff --git a/src/confitems.c b/src/confitems.c index 4b70db4ac..15064e71f 100644 --- a/src/confitems.c +++ b/src/confitems.c @@ -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); } diff --git a/test/suites/direct.bash b/test/suites/direct.bash index 137304433..fca072f75 100644 --- a/test/suites/direct.bash +++ b/test/suites/direct.bash @@ -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 <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 <file.h -#define file __FILE__ -int test; -EOF - backdate file.h - cat <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 <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 <file.h -#define file __FILE__ -int test; + mkdir a + cat <a/file.c +#include "file.h" +EOF + cat <a/file.h +int x = 1; EOF - backdate file.h - cat <file_h.c + backdate a/file.h + + mkdir b + cat <b/file.c #include "file.h" EOF + cat <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" diff --git a/unittest/test_conf.c b/unittest/test_conf.c index da57bb4a0..bc458fce2 100644 --- a/unittest/test_conf.c +++ b/unittest/test_conf.c @@ -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",