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.

diff --git a/src/ccache.c b/src/ccache.c
index f56445beef24bf4ca2a768e45b634d9513ae8c56..9c605934fa97629dfc594a736fa38325b83050a4 100644 (file)
--- a/src/ccache.c
+++ b/src/ccache.c
@@ -2492,10 +2492,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,
+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;
@@ -3580,6 +3585,10 @@ cc_process_args(struct args *args, struct args **preprocessor_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);
@@ -3918,10 +3927,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");
@@ -3980,13 +3993,16 @@ ccache(int argc, char *argv[])
        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_object_in_manifest = false;
        struct file_hash *object_hash = NULL;
        struct file_hash *object_hash_from_manifest = NULL;
        if (conf->direct_mode) {
                cc_log("Trying direct lookup");
                MTR_BEGIN("hash", "direct_hash");
-               object_hash = calculate_object_hash(preprocessor_args, direct_hash, 1);
+               object_hash = calculate_object_hash(args_to_hash, direct_hash, 1);
                MTR_END("hash", "direct_hash");
                if (object_hash) {
                        update_cached_result_globals(object_hash);
@@ -4018,7 +4034,7 @@ ccache(int argc, char *argv[])
                        cpp_hash, output_obj, 'p', "PREPROCESSOR MODE", debug_text_file);
 
                MTR_BEGIN("hash", "cpp_hash");
-               object_hash = calculate_object_hash(preprocessor_args, cpp_hash, 0);
+               object_hash = calculate_object_hash(args_to_hash, cpp_hash, 0);
                MTR_END("hash", "cpp_hash");
                if (!object_hash) {
                        fatal("internal error: object hash from cpp returned NULL");

diff --git a/src/ccache.h b/src/ccache.h
index a2694d1a6112973e6a7aa10781e32ba5c3f8d97b..ac12a14b0e362d21403674d76c367ba7695e4c08 100644 (file)
--- a/src/ccache.h
+++ b/src/ccache.h
@@ -270,8 +270,10 @@ extern time_t time_of_compilation;
 extern bool output_is_precompiled_header;
 void block_signals(void);
 void unblock_signals(void);
-bool cc_process_args(struct args *args, struct args **preprocessor_args,
-                    struct args **compiler_args);
+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 5e9cd4d0c430c04ac1285759d830c161d12611a8..fac44ea670e83b8eacf715821ce03daf0a739add 100644 (file)
--- a/test/suites/base.bash
+++ b/test/suites/base.bash
@@ -980,6 +980,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.c b/unittest/test_argument_processing.c
index 6993e42354cac6076cad9e82ef16a2e63a180d48..ca48c2a533c2e5ccf048031be368f316629664b1 100644 (file)
--- a/unittest/test_argument_processing.c
+++ b/unittest/test_argument_processing.c
@@ -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", "");
 
        conf->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", "");
 
        conf->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);
 
@@ -188,7 +188,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);
@@ -211,7 +211,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");
 
@@ -230,7 +230,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);
 
@@ -247,7 +247,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);
 
@@ -264,7 +264,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);
 
@@ -281,7 +281,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);
 
@@ -298,7 +298,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);
 
@@ -315,7 +315,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);
 
@@ -332,7 +332,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);
 
@@ -359,7 +359,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);
 
@@ -378,7 +378,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);
 
@@ -400,7 +400,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);
@@ -426,7 +426,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);
@@ -453,7 +453,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);
@@ -471,7 +471,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);
 
@@ -487,7 +487,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);
 
@@ -499,14 +499,19 @@ 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);