Include compiler-only arguments in the hash

After 5d8585b5 (#312), arguments that are not considered affecting the
preprocessor output won’t be passed to the preprocessor. -Werror and
-Wno-error are also not passed to the preprocessor so that options not
properly marked as “compiler only” will only trigger warnings, not
errors. This was a workaround for Clang complaining about unused
arguments in the preprocessor step performed by ccache.

However, it also introduced a regression: -Werror and the other options
were excluded from the hash as well. This means that

  cc -c file_with_warnings.c

would be cached by ccache, including the warning message. A later

  cc -Werror -c file_with_warnings.c

call would then be a cache hit, resulting in a compilation warning
instead of an error.

This commit fixes the problem by also including the compiler-only
arguments in the hash.

(cherry picked from commit 6be00a0070d3898fe7201f6db44a9a3c42627bf8)

diff --git a/src/ccache.cpp b/src/ccache.cpp
index 87599e7945bb00d3b8a0ccce7ea9b1cfe1562650..dc98a79ea4b2d59e8ad53f7176a9566badb1de76 100644 (file)
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -2340,11 +2340,15 @@ detect_pch(const char* option, const char* arg, bool* found_pch)
 }
 
 // Process the compiler options into options suitable for passing to the
-// preprocessor and the real compiler. The preprocessor options don't include
-// -E; this is added later. Returns true on success, otherwise false.
+// preprocessor and the real compiler. preprocessor_args does't include -E;
+// this is added later. extra_args_to_hash are the arguments that are not
+// included in preprocessor_args but that should be included in the hash.
+//
+// Returns true on success, otherwise false.
 bool
 cc_process_args(struct args* args,
                 struct args** preprocessor_args,
+                struct args** extra_args_to_hash,
                 struct args** compiler_args)
 {
   bool found_c_opt = false;
@@ -3429,6 +3433,10 @@ cc_process_args(struct args* args,
   *preprocessor_args = args_copy(common_args);
   args_extend(*preprocessor_args, cpp_args);
 
+  if (extra_args_to_hash) {
+    *extra_args_to_hash = compiler_only_args;
+  }
+
 out:
   args_free(expanded_args);
   args_free(common_args);
@@ -3772,10 +3780,14 @@ ccache(int argc, char* argv[])
 
   // Arguments (except -E) to send to the preprocessor.
   struct args* preprocessor_args;
+  // Arguments not sent to the preprocessor but that should be part of the
+  // hash.
+  struct args* extra_args_to_hash;
   // Arguments to send to the real compiler.
   struct args* compiler_args;
   MTR_BEGIN("main", "process_args");
-  if (!cc_process_args(orig_args, &preprocessor_args, &compiler_args)) {
+  if (!cc_process_args(
+        orig_args, &preprocessor_args, &extra_args_to_hash, &compiler_args)) {
     failed();
   }
   MTR_END("main", "process_args");
@@ -3833,13 +3845,16 @@ ccache(int argc, char* argv[])
   struct hash* direct_hash = hash_copy(common_hash);
   init_hash_debug(direct_hash, output_obj, 'd', "DIRECT MODE", debug_text_file);
 
+  struct args* args_to_hash = args_copy(preprocessor_args);
+  args_extend(args_to_hash, extra_args_to_hash);
+
   bool put_result_in_manifest = false;
   struct digest* result_name = NULL;
   struct digest* result_name_from_manifest = NULL;
   if (g_config.direct_mode()) {
     cc_log("Trying direct lookup");
     MTR_BEGIN("hash", "direct_hash");
-    result_name = calculate_result_name(preprocessor_args, direct_hash, 1);
+    result_name = calculate_result_name(args_to_hash, direct_hash, 1);
     MTR_END("hash", "direct_hash");
     if (result_name) {
       update_cached_result_globals(result_name);
@@ -3871,7 +3886,7 @@ ccache(int argc, char* argv[])
       cpp_hash, output_obj, 'p', "PREPROCESSOR MODE", debug_text_file);
 
     MTR_BEGIN("hash", "cpp_hash");
-    result_name = calculate_result_name(preprocessor_args, cpp_hash, 0);
+    result_name = calculate_result_name(args_to_hash, cpp_hash, 0);
     MTR_END("hash", "cpp_hash");
     if (!result_name) {
       fatal("internal error: calculate_result_name returned NULL for cpp");

diff --git a/src/ccache.hpp b/src/ccache.hpp
index 9ff8e01a7cdd86fc173f338ca0f35359bb14b934..4bf7d7f7a1e9e9d15076bcb78ab7eab2fd39d287 100644 (file)
--- a/src/ccache.hpp
+++ b/src/ccache.hpp
@@ -272,6 +272,7 @@ void block_signals(void);
 void unblock_signals(void);
 bool cc_process_args(struct args* args,
                      struct args** preprocessor_args,
+                     struct args** extra_args_to_hash,
                      struct args** compiler_args);
 void cc_reset(void);
 bool is_precompiled_header(const char* path);

diff --git a/test/suites/base.bash b/test/suites/base.bash
index 7fe30f46b8c1911457268a0170c4c0ce2eb3c855..827fbe154a05206edbcf355973729c684df02d8f 100644 (file)
--- a/test/suites/base.bash
+++ b/test/suites/base.bash
@@ -978,6 +978,26 @@ EOF
     expect_stat 'cache hit (preprocessed)' 1
     expect_stat 'cache miss' 1
 
+    # -------------------------------------------------------------------------
+    TEST "Handling of compiler-only arguments"
+
+    $CCACHE_COMPILE -c test1.c
+    expect_stat 'cache hit (preprocessed)' 0
+    expect_stat 'cache miss' 1
+    expect_stat 'files in cache' 1
+
+    $CCACHE_COMPILE -c test1.c
+    expect_stat 'cache hit (preprocessed)' 1
+    expect_stat 'cache miss' 1
+    expect_stat 'files in cache' 1
+
+    # Even though -Werror is not passed to the preprocessor, it should be part
+    # of the hash, so we expect a cache miss:
+    $CCACHE_COMPILE -c -Werror test1.c
+    expect_stat 'cache hit (preprocessed)' 1
+    expect_stat 'cache miss' 2
+    expect_stat 'files in cache' 2
+
     # -------------------------------------------------------------------------
     TEST "Buggy GCC 6 cpp"
 

diff --git a/unittest/test_argument_processing.cpp b/unittest/test_argument_processing.cpp
index 0ace9fa2581e890c8ac887ef1fa2cd830998438b..1146c5e4c04c18aec18a3cb8fbec0810bf91e8bd 100644 (file)
--- a/unittest/test_argument_processing.cpp
+++ b/unittest/test_argument_processing.cpp
@@ -68,7 +68,7 @@ TEST(dash_E_should_result_in_called_for_preprocessing)
   struct args *preprocessed, *compiler;
 
   create_file("foo.c", "");
-  CHECK(!cc_process_args(orig, &preprocessed, &compiler));
+  CHECK(!cc_process_args(orig, &preprocessed, NULL, &compiler));
   CHECK_INT_EQ(1, stats_get_pending(STATS_PREPROCESSING));
 
   args_free(orig);
@@ -80,7 +80,7 @@ TEST(dash_M_should_be_unsupported)
   struct args *preprocessed, *compiler;
 
   create_file("foo.c", "");
-  CHECK(!cc_process_args(orig, &preprocessed, &compiler));
+  CHECK(!cc_process_args(orig, &preprocessed, NULL, &compiler));
   CHECK_INT_EQ(1, stats_get_pending(STATS_UNSUPPORTED_OPTION));
 
   args_free(orig);
@@ -98,7 +98,7 @@ TEST(dependency_flags_should_only_be_sent_to_the_preprocessor)
   struct args *act_cpp = NULL, *act_cc = NULL;
   create_file("foo.c", "");
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -123,7 +123,7 @@ TEST(cpp_only_flags_to_preprocessor_if_run_second_cpp_is_false)
   create_file("foo.c", "");
 
   g_config.set_run_second_cpp(false);
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -149,7 +149,7 @@ TEST(cpp_only_flags_to_preprocessor_and_compiler_if_run_second_cpp_is_true)
   create_file("foo.c", "");
 
   g_config.set_run_second_cpp(true);
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -166,7 +166,7 @@ TEST(dependency_flags_that_take_an_argument_should_not_require_space_delimiter)
   struct args *act_cpp = NULL, *act_cc = NULL;
   create_file("foo.c", "");
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -187,7 +187,7 @@ TEST(sysroot_should_be_rewritten_if_basedir_is_used)
   orig = args_init_from_string(arg_string);
   free(arg_string);
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_STR_EQ(act_cpp->argv[1], "--sysroot=./foo/bar");
 
   args_free(orig);
@@ -209,7 +209,7 @@ TEST(sysroot_with_separate_argument_should_be_rewritten_if_basedir_is_used)
   orig = args_init_from_string(arg_string);
   free(arg_string);
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_STR_EQ(act_cpp->argv[1], "--sysroot");
   CHECK_STR_EQ(act_cpp->argv[2], "./foo");
 
@@ -227,7 +227,7 @@ TEST(MF_flag_with_immediate_argument_should_work_as_last_argument)
   struct args *act_cpp = NULL, *act_cc = NULL;
   create_file("foo.c", "");
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -244,7 +244,7 @@ TEST(MT_flag_with_immediate_argument_should_work_as_last_argument)
   struct args *act_cpp = NULL, *act_cc = NULL;
   create_file("foo.c", "");
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -261,7 +261,7 @@ TEST(MQ_flag_with_immediate_argument_should_work_as_last_argument)
   struct args *act_cpp = NULL, *act_cc = NULL;
   create_file("foo.c", "");
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -278,7 +278,7 @@ TEST(MQ_flag_without_immediate_argument_should_not_add_MQobj)
   struct args *act_cpp = NULL, *act_cc = NULL;
   create_file("foo.c", "");
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -295,7 +295,7 @@ TEST(MT_flag_without_immediate_argument_should_not_add_MTobj)
   struct args *act_cpp = NULL, *act_cc = NULL;
   create_file("foo.c", "");
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -311,7 +311,7 @@ TEST(MQ_flag_with_immediate_argument_should_not_add_MQobj)
   struct args *act_cpp = NULL, *act_cc = NULL;
   create_file("foo.c", "");
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -327,7 +327,7 @@ TEST(MT_flag_with_immediate_argument_should_not_add_MQobj)
   struct args *act_cpp = NULL, *act_cc = NULL;
   create_file("foo.c", "");
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -354,7 +354,7 @@ TEST(fprofile_flag_with_existing_dir_should_be_rewritten_to_real_path)
   args_add(exp_cc, "-c");
   free(s);
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -373,7 +373,7 @@ TEST(fprofile_flag_with_nonexistent_dir_should_not_be_rewritten)
 
   create_file("foo.c", "");
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -394,7 +394,7 @@ TEST(isystem_flag_with_separate_arg_should_be_rewritten_if_basedir_is_used)
   orig = args_init_from_string(arg_string);
   free(arg_string);
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_STR_EQ("./foo", act_cpp->argv[2]);
 
   args_free(orig);
@@ -419,7 +419,7 @@ TEST(isystem_flag_with_concat_arg_should_be_rewritten_if_basedir_is_used)
   orig = args_init_from_string(arg_string);
   free(arg_string);
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_STR_EQ("-isystem./foo", act_cpp->argv[1]);
 
   free(cwd);
@@ -445,7 +445,7 @@ TEST(I_flag_with_concat_arg_should_be_rewritten_if_basedir_is_used)
   orig = args_init_from_string(arg_string);
   free(arg_string);
 
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_STR_EQ("-I./foo", act_cpp->argv[1]);
 
   free(cwd);
@@ -463,7 +463,7 @@ TEST(debug_flag_order_with_known_option_first)
   struct args* act_cc = NULL;
 
   create_file("foo.c", "");
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -479,7 +479,7 @@ TEST(debug_flag_order_with_known_option_last)
   struct args* act_cc = NULL;
 
   create_file("foo.c", "");
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, NULL, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
@@ -491,14 +491,18 @@ TEST(options_not_to_be_passed_to_the_preprocesor)
   struct args* orig = args_init_from_string(
     "cc -Wa,foo foo.c -g -c -DX -Werror -Xlinker fie -Xlinker,fum -Wno-error");
   struct args* exp_cpp = args_init_from_string("cc -g -DX");
+  struct args* exp_extra = args_init_from_string(
+    " -Wa,foo -Werror -Xlinker fie -Xlinker,fum -Wno-error");
   struct args* exp_cc = args_init_from_string(
     "cc -g -Wa,foo -Werror -Xlinker fie -Xlinker,fum -Wno-error -DX -c");
   struct args* act_cpp = NULL;
+  struct args* act_extra = NULL;
   struct args* act_cc = NULL;
 
   create_file("foo.c", "");
-  CHECK(cc_process_args(orig, &act_cpp, &act_cc));
+  CHECK(cc_process_args(orig, &act_cpp, &act_extra, &act_cc));
   CHECK_ARGS_EQ_FREE12(exp_cpp, act_cpp);
+  CHECK_ARGS_EQ_FREE12(exp_extra, act_extra);
   CHECK_ARGS_EQ_FREE12(exp_cc, act_cc);
 
   args_free(orig);