From 25e73c1f51455dca1cb2d0976bf1f577e717de24 Mon Sep 17 00:00:00 2001 From: Joel Rosdahl Date: Tue, 17 Sep 2019 22:37:57 +0200 Subject: [PATCH] Include compiler-only arguments in the hash MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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) --- src/ccache.cpp | 25 +++++++++++--- src/ccache.hpp | 1 + test/suites/base.bash | 20 +++++++++++ unittest/test_argument_processing.cpp | 50 +++++++++++++++------------ 4 files changed, 68 insertions(+), 28 deletions(-) diff --git a/src/ccache.cpp b/src/ccache.cpp index 87599e794..dc98a79ea 100644 --- 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 9ff8e01a7..4bf7d7f7a 100644 --- 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 7fe30f46b..827fbe154 100644 --- 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 0ace9fa25..1146c5e4c 100644 --- 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); -- 2.47.2