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: v4.0~708 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=02bcef0d82ac8f1397c9b13bf1215cf770e6b7da;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. (cherry picked from commit 404eedde2b8c7a048ea1f68a8e70e49c5ea74df2) --- diff --git a/doc/MANUAL.adoc b/doc/MANUAL.adoc index 87d0cc642..9c3ac4577 100644 --- a/doc/MANUAL.adoc +++ b/doc/MANUAL.adoc @@ -646,13 +646,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 @@ -1386,13 +1379,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 <> 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 - <> 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/Config.cpp b/src/Config.cpp index 6e07bc900..1aba8916f 100644 --- a/src/Config.cpp +++ b/src/Config.cpp @@ -246,9 +246,7 @@ parse_sloppiness(const std::string& value) end = value.find_first_of(", ", start); std::string token = Util::strip_whitespace(value.substr(start, end - start)); - if (token == "file_macro") { - result |= SLOPPY_FILE_MACRO; - } else if (token == "file_stat_matches") { + if (token == "file_stat_matches") { result |= SLOPPY_FILE_STAT_MATCHES; } else if (token == "file_stat_matches_ctime") { result |= SLOPPY_FILE_STAT_MATCHES_CTIME; @@ -278,9 +276,6 @@ std::string format_sloppiness(uint32_t sloppiness) { std::string result; - if (sloppiness & SLOPPY_FILE_MACRO) { - result += "file_macro, "; - } if (sloppiness & SLOPPY_INCLUDE_FILE_MTIME) { result += "include_file_mtime, "; } diff --git a/src/ccache.cpp b/src/ccache.cpp index 53f8b2ff0..064c3611f 100644 --- a/src/ccache.cpp +++ b/src/ccache.cpp @@ -2115,12 +2115,19 @@ calculate_result_name(struct args* args, struct hash* hash, int direct_mode) } } - if (!(g_config.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(g_config, hash, input_file); diff --git a/src/ccache.hpp b/src/ccache.hpp index 8243aebc7..b7c638cb3 100644 --- a/src/ccache.hpp +++ b/src/ccache.hpp @@ -94,24 +94,23 @@ 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) // Allow caching even if -fmodules is used. -#define SLOPPY_MODULES (1U << 10) +#define SLOPPY_MODULES (1U << 9) #define str_eq(s1, s2) (strcmp((s1), (s2)) == 0) #define str_startswith(s, prefix) \ diff --git a/test/suites/direct.bash b/test/suites/direct.bash index e250ad8f0..f60a4b4e7 100644 --- a/test/suites/direct.bash +++ b/test/suites/direct.bash @@ -531,12 +531,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 @@ -548,47 +549,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 @@ -600,39 +588,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_Config.cpp b/unittest/test_Config.cpp index a6bf5fd38..2521421bd 100644 --- a/unittest/test_Config.cpp +++ b/unittest/test_Config.cpp @@ -116,10 +116,9 @@ TEST_CASE("Config::update_from_file") "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" @@ -158,7 +157,7 @@ TEST_CASE("Config::update_from_file") CHECK_FALSE(config.run_second_cpp()); CHECK(config.sloppiness() == (SLOPPY_INCLUDE_FILE_MTIME | SLOPPY_INCLUDE_FILE_CTIME - | SLOPPY_FILE_MACRO | SLOPPY_TIME_MACROS | SLOPPY_FILE_STAT_MATCHES + | SLOPPY_TIME_MACROS | SLOPPY_FILE_STAT_MATCHES | SLOPPY_FILE_STAT_MATCHES_CTIME | SLOPPY_SYSTEM_HEADERS | SLOPPY_PCH_DEFINES | SLOPPY_CLANG_INDEX_STORE)); CHECK_FALSE(config.stats()); @@ -394,9 +393,9 @@ TEST_CASE("Config::visit_items") "read_only_direct = true\n" "recache = true\n" "run_second_cpp = false\n" - "sloppiness = file_macro, include_file_mtime, include_file_ctime," - " time_macros, file_stat_matches, file_stat_matches_ctime, pch_defines," - " system_headers, clang_index_store\n" + "sloppiness = include_file_mtime, include_file_ctime, time_macros," + " file_stat_matches, file_stat_matches_ctime, pch_defines, system_headers," + " clang_index_store\n" "stats = false\n" "temporary_dir = td\n" "umask = 022\n" @@ -444,9 +443,9 @@ TEST_CASE("Config::visit_items") "(test.conf) read_only_direct = true", "(test.conf) recache = true", "(test.conf) run_second_cpp = false", - "(test.conf) sloppiness = file_macro, include_file_mtime," - " include_file_ctime, time_macros, pch_defines, file_stat_matches," - " file_stat_matches_ctime, system_headers, clang_index_store", + "(test.conf) sloppiness = include_file_mtime, include_file_ctime," + " time_macros, pch_defines, file_stat_matches, file_stat_matches_ctime," + " system_headers, clang_index_store", "(test.conf) stats = false", "(test.conf) temporary_dir = td", "(test.conf) umask = 022",