From: Joel Rosdahl Date: Wed, 20 Jul 2022 20:55:56 +0000 (+0200) Subject: fix: Work around Clang bug when outputting to full NFS disk X-Git-Tag: v4.6.2~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=be93d81e814efa7899e3237ea70278923b31a7bc;p=thirdparty%2Fccache.git fix: Work around Clang bug when outputting to full NFS disk See . Fixes #1115. (cherry picked from commit 64fc42ca2c5c9fe60ef4f1dc3882edb5f35579d6) --- diff --git a/src/Context.hpp b/src/Context.hpp index b681ddeaf..241ad455d 100644 --- a/src/Context.hpp +++ b/src/Context.hpp @@ -80,8 +80,8 @@ public: // The name of the temporary preprocessed file. std::string i_tmpfile; - // The name of the cpp stderr file. - std::string cpp_stderr; + // The preprocessor's stderr output. + std::string cpp_stderr_data; // Headers (or directories with headers) to ignore in manifest mode. std::vector ignore_header_paths; diff --git a/src/ccache.cpp b/src/ccache.cpp index d83fb1a80..561b5c3d8 100644 --- a/src/ccache.cpp +++ b/src/ccache.cpp @@ -662,13 +662,40 @@ result_key_from_depfile(Context& ctx, Hash& hash) return hash.digest(); } + +struct GetTmpFdResult +{ + Fd fd; + std::string path; +}; + +static GetTmpFdResult +get_tmp_fd(Context& ctx, + const nonstd::string_view description, + const bool capture_output) +{ + if (capture_output) { + TemporaryFile tmp_stdout( + FMT("{}/tmp.{}", ctx.config.temporary_dir(), description)); + ctx.register_pending_tmp_file(tmp_stdout.path); + return {std::move(tmp_stdout.fd), std::move(tmp_stdout.path)}; + } else { + const auto dev_null_path = util::get_dev_null_path(); + return {Fd(open(dev_null_path, O_WRONLY | O_BINARY)), dev_null_path}; + } +} + +struct DoExecuteResult +{ + int exit_status; + std::string stdout_data; + std::string stderr_data; +}; + // Execute the compiler/preprocessor, with logic to retry without requesting // colored diagnostics messages if that fails. -static nonstd::expected -do_execute(Context& ctx, - Args& args, - TemporaryFile&& tmp_stdout, - TemporaryFile&& tmp_stderr) +static nonstd::expected +do_execute(Context& ctx, Args& args, const bool capture_stdout = true) { UmaskScope umask_scope(ctx.original_umask); @@ -676,6 +703,10 @@ do_execute(Context& ctx, DEBUG_ASSERT(ctx.config.compiler_type() == CompilerType::gcc); args.erase_last("-fdiagnostics-color"); } + + auto tmp_stdout = get_tmp_fd(ctx, "stdout", capture_stdout); + auto tmp_stderr = get_tmp_fd(ctx, "stderr", true); + int status = execute(ctx, args.to_argv().data(), std::move(tmp_stdout.fd), @@ -694,26 +725,22 @@ do_execute(Context& ctx, // error message.) LOG_RAW("-fdiagnostics-color is unsupported; trying again without it"); - tmp_stdout.fd = Fd(open( - tmp_stdout.path.c_str(), O_RDWR | O_CREAT | O_TRUNC | O_BINARY, 0600)); - if (!tmp_stdout.fd) { - LOG("Failed to truncate {}: {}", tmp_stdout.path, strerror(errno)); - return nonstd::make_unexpected(Statistic::internal_error); - } - - tmp_stderr.fd = Fd(open( - tmp_stderr.path.c_str(), O_RDWR | O_CREAT | O_TRUNC | O_BINARY, 0600)); - if (!tmp_stderr.fd) { - LOG("Failed to truncate {}: {}", tmp_stderr.path, strerror(errno)); - return nonstd::make_unexpected(Statistic::internal_error); - } - ctx.diagnostics_color_failed = true; - return do_execute( - ctx, args, std::move(tmp_stdout), std::move(tmp_stderr)); + return do_execute(ctx, args, capture_stdout); } } - return status; + + try { + return DoExecuteResult{ + status, + capture_stdout ? Util::read_file(tmp_stdout.path) : std::string(), + Util::read_file(tmp_stderr.path), + }; + } catch (core::Error&) { + // The stdout or stderr file was removed - cleanup in progress? Better bail + // out. + return nonstd::make_unexpected(Statistic::missing_cache_file); + } } static core::Manifest @@ -991,18 +1018,9 @@ to_cache(Context& ctx, LOG_RAW("Running real compiler"); MTR_BEGIN("execute", "compiler"); - TemporaryFile tmp_stdout(FMT("{}/tmp.stdout", ctx.config.temporary_dir())); - ctx.register_pending_tmp_file(tmp_stdout.path); - std::string tmp_stdout_path = tmp_stdout.path; - - TemporaryFile tmp_stderr(FMT("{}/tmp.stderr", ctx.config.temporary_dir())); - ctx.register_pending_tmp_file(tmp_stderr.path); - std::string tmp_stderr_path = tmp_stderr.path; - - nonstd::expected status; + nonstd::expected result; if (!ctx.config.depend_mode()) { - status = - do_execute(ctx, args, std::move(tmp_stdout), std::move(tmp_stderr)); + result = do_execute(ctx, args); args.pop_back(3); } else { // Use the original arguments (including dependency options) in depend @@ -1013,45 +1031,32 @@ to_cache(Context& ctx, add_prefix(ctx, depend_mode_args, ctx.config.prefix_command()); ctx.time_of_compilation = time(nullptr); - status = do_execute( - ctx, depend_mode_args, std::move(tmp_stdout), std::move(tmp_stderr)); + result = do_execute(ctx, depend_mode_args); } MTR_END("execute", "compiler"); - if (!status) { - return nonstd::make_unexpected(status.error()); + if (!result) { + return nonstd::make_unexpected(result.error()); } - // Merge stderr from the preprocessor (if any) and stderr from - // the real compiler into tmp_stderr. - if (!ctx.cpp_stderr.empty()) { - std::string combined_stderr = - Util::read_file(ctx.cpp_stderr) + Util::read_file(tmp_stderr_path); - Util::write_file(tmp_stderr_path, combined_stderr); + // Merge stderr from the preprocessor (if any) and stderr from the real + // compiler. + if (!ctx.cpp_stderr_data.empty()) { + result->stderr_data = ctx.cpp_stderr_data + result->stderr_data; } - std::string stdout_data; - std::string stderr_data; - try { - stdout_data = Util::read_file(tmp_stdout_path); - stderr_data = Util::read_file(tmp_stderr_path); - } catch (core::Error&) { - // The stdout or stderr file was removed - cleanup in progress? Better bail - // out. - return nonstd::make_unexpected(Statistic::missing_cache_file); - } - - stdout_data = rewrite_stdout_from_compiler(ctx, std::move(stdout_data)); + result->stdout_data = + rewrite_stdout_from_compiler(ctx, std::move(result->stdout_data)); - if (status != 0) { - LOG("Compiler gave exit status {}", *status); + if (result->exit_status != 0) { + LOG("Compiler gave exit status {}", result->exit_status); // We can output stderr immediately instead of rerunning the compiler. - Util::send_to_fd(ctx, stderr_data, STDERR_FILENO); - Util::send_to_fd(ctx, stdout_data, STDOUT_FILENO); + Util::send_to_fd(ctx, result->stderr_data, STDERR_FILENO); + Util::send_to_fd(ctx, result->stdout_data, STDOUT_FILENO); auto failure = Failure(Statistic::compile_failed); - failure.set_exit_code(*status); + failure.set_exit_code(result->exit_status); return nonstd::make_unexpected(failure); } @@ -1091,7 +1096,8 @@ to_cache(Context& ctx, MTR_BEGIN("result", "result_put"); const bool added = ctx.storage.put( *result_key, core::CacheEntryType::result, [&](const auto& path) { - return write_result(ctx, path, obj_stat, stdout_data, stderr_data); + return write_result( + ctx, path, obj_stat, result->stdout_data, result->stderr_data); }); MTR_END("result", "result_put"); if (!added) { @@ -1099,9 +1105,9 @@ to_cache(Context& ctx, } // Everything OK. - Util::send_to_fd(ctx, stderr_data, STDERR_FILENO); + Util::send_to_fd(ctx, result->stderr_data, STDERR_FILENO); // Send stdout after stderr, it makes the output clearer with MSVC. - Util::send_to_fd(ctx, stdout_data, STDOUT_FILENO); + Util::send_to_fd(ctx, result->stdout_data, STDOUT_FILENO); return *result_key; } @@ -1113,76 +1119,69 @@ get_result_key_from_cpp(Context& ctx, Args& args, Hash& hash) { ctx.time_of_compilation = time(nullptr); - std::string stderr_path; - std::string stdout_path; + std::string preprocessed_path; + std::string cpp_stderr_data; + if (ctx.args_info.direct_i_file) { // We are compiling a .i or .ii file - that means we can skip the cpp stage // and directly form the correct i_tmpfile. - stdout_path = ctx.args_info.input_file; + preprocessed_path = ctx.args_info.input_file; + cpp_stderr_data = ""; } else { // Run cpp on the input file to obtain the .i. - // stdout_path needs the proper cpp_extension for the compiler to do its - // thing correctly. + // preprocessed_path needs the proper cpp_extension for the compiler to do + // its thing correctly. TemporaryFile tmp_stdout( FMT("{}/tmp.cpp_stdout", ctx.config.temporary_dir()), FMT(".{}", ctx.config.cpp_extension())); - stdout_path = tmp_stdout.path; - ctx.register_pending_tmp_file(stdout_path); - - TemporaryFile tmp_stderr( - FMT("{}/tmp.cpp_stderr", ctx.config.temporary_dir())); - stderr_path = tmp_stderr.path; - ctx.register_pending_tmp_file(stderr_path); + preprocessed_path = tmp_stdout.path; + tmp_stdout.fd.close(); // We're only using the path. + ctx.register_pending_tmp_file(preprocessed_path); - size_t args_added = 2; + const size_t orig_args_size = args.size(); args.push_back("-E"); - if (ctx.args_info.actual_language == "hip") { - args.push_back("-o"); - args.push_back("-"); - args_added += 2; - } if (ctx.config.keep_comments_cpp()) { args.push_back("-C"); - args_added++; } + + // Pass -o instead of sending the preprocessor output to stdout to work + // around compilers that don't exit with a proper status on write error to + // stdout. See also . + args.push_back("-o"); + args.push_back(preprocessed_path); + args.push_back(ctx.args_info.input_file); + add_prefix(ctx, args, ctx.config.prefix_command_cpp()); LOG_RAW("Running preprocessor"); MTR_BEGIN("execute", "preprocessor"); - const auto status = - do_execute(ctx, args, std::move(tmp_stdout), std::move(tmp_stderr)); + const auto result = do_execute(ctx, args, false); MTR_END("execute", "preprocessor"); - args.pop_back(args_added); + args.pop_back(args.size() - orig_args_size); - if (!status) { - return nonstd::make_unexpected(status.error()); - } else if (*status != 0) { - LOG("Preprocessor gave exit status {}", *status); + if (!result) { + return nonstd::make_unexpected(result.error()); + } else if (result->exit_status != 0) { + LOG("Preprocessor gave exit status {}", result->exit_status); return nonstd::make_unexpected(Statistic::preprocessor_error); } - } - hash.hash_delimiter("cpp"); - TRY(process_preprocessed_file(ctx, hash, stdout_path)); + cpp_stderr_data = result->stderr_data; + } hash.hash_delimiter("cppstderr"); - if (!ctx.args_info.direct_i_file && !hash.hash_file(stderr_path)) { - // Somebody removed the temporary file? - LOG("Failed to open {}: {}", stderr_path, strerror(errno)); - return nonstd::make_unexpected(Statistic::internal_error); - } + hash.hash(cpp_stderr_data); - if (ctx.args_info.direct_i_file) { - ctx.i_tmpfile = ctx.args_info.input_file; - } else { - ctx.i_tmpfile = stdout_path; - } + hash.hash_delimiter("cpp"); + TRY(process_preprocessed_file(ctx, hash, preprocessed_path)); + + ctx.i_tmpfile = preprocessed_path; if (!ctx.config.run_second_cpp()) { // If we are using the CPP trick, we need to remember this stderr data and // output it just before the main stderr from the compiler pass. - ctx.cpp_stderr = stderr_path; + ctx.cpp_stderr_data = std::move(cpp_stderr_data); hash.hash_delimiter("runsecondcpp"); hash.hash("false"); } diff --git a/src/util/path.cpp b/src/util/path.cpp index 86d9d0202..4f136333e 100644 --- a/src/util/path.cpp +++ b/src/util/path.cpp @@ -22,13 +22,21 @@ #include #ifdef _WIN32 +const char k_dev_null_path[] = "nul:"; const char k_path_delimiter[] = ";"; #else +const char k_dev_null_path[] = "/dev/null"; const char k_path_delimiter[] = ":"; #endif namespace util { +const char* +get_dev_null_path() +{ + return k_dev_null_path; +} + bool is_absolute_path(nonstd::string_view path) { diff --git a/src/util/path.hpp b/src/util/path.hpp index 51fcd5815..13b8d2285 100644 --- a/src/util/path.hpp +++ b/src/util/path.hpp @@ -1,4 +1,4 @@ -// Copyright (C) 2021 Joel Rosdahl and other contributors +// Copyright (C) 2021-2022 Joel Rosdahl and other contributors // // See doc/AUTHORS.adoc for a complete list of contributors. // @@ -27,6 +27,8 @@ namespace util { // --- Interface --- +const char* get_dev_null_path(); + // Return whether `path` is absolute. bool is_absolute_path(nonstd::string_view path); diff --git a/test/run b/test/run index f2fb3dfe9..074c02957 100755 --- a/test/run +++ b/test/run @@ -271,6 +271,18 @@ expect_content() { fi } +expect_content_pattern() { + local file="$1" + local pattern="$2" + + if [ ! -e "$file" ]; then + test_failed_internal "$file not found" + fi + if [[ "$(<$file)" != $pattern ]]; then + test_failed_internal "Bad content of $file\nExpected pattern: $pattern\nActual: $(<$file)" + fi +} + expect_contains() { local file="$1" local string="$2" diff --git a/test/suites/base.bash b/test/suites/base.bash index 0c32fab5d..2ec443ebd 100644 --- a/test/suites/base.bash +++ b/test/suites/base.bash @@ -1416,7 +1416,7 @@ EOF cat >compiler.sh <>compiler.args +printf "(%s)" "\$*" >>compiler.args [ \$1 = -E ] && echo test || echo test >test1.o EOF chmod +x compiler.sh @@ -1427,7 +1427,7 @@ EOF expect_stat cache_miss 1 expect_stat files_in_cache 1 if [ -z "$CCACHE_NOCPP2" ]; then - expect_content compiler.args "[-E test1.c][-c -o test1.o test1.c]" + expect_content_pattern compiler.args "(-E -o * test1.c)(-c -o test1.o test1.c)" fi rm compiler.args @@ -1435,7 +1435,7 @@ EOF expect_stat preprocessed_cache_hit 1 expect_stat cache_miss 1 expect_stat files_in_cache 1 - expect_content compiler.args "[-E test1.c]" + expect_content_pattern compiler.args "(-E -o * test1.c)" rm compiler.args # Even though -Werror is not passed to the preprocessor, it should be part @@ -1445,7 +1445,7 @@ EOF expect_stat cache_miss 2 expect_stat files_in_cache 2 if [ -z "$CCACHE_NOCPP2" ]; then - expect_content compiler.args "[-E test1.c][-Werror -rdynamic -c -o test1.o test1.c]" + expect_content_pattern compiler.args "(-E -o * test1.c)(-Werror -rdynamic -c -o test1.o test1.c)" fi rm compiler.args diff --git a/test/suites/multi_arch.bash b/test/suites/multi_arch.bash index 9a705cd3e..28cb1f459 100644 --- a/test/suites/multi_arch.bash +++ b/test/suites/multi_arch.bash @@ -40,8 +40,8 @@ SUITE_multi_arch() { CCACHE_DEBUG=1 $CCACHE_COMPILE -arch x86_64 -Xarch_x86_64 -I. -c test1.c expect_stat direct_cache_hit 2 expect_stat cache_miss 4 - expect_contains test1.o.ccache-log "clang -Xarch_x86_64 -I. -arch x86_64 -E test1.c" - expect_not_contains test1.o.ccache-log "clang -Xarch_x86_64 -I. -I. -arch x86_64 -E test1.c" + expect_contains test1.o.ccache-log "clang -Xarch_x86_64 -I. -arch x86_64 -E" + expect_not_contains test1.o.ccache-log "clang -Xarch_x86_64 -I. -I. -arch x86_64 -E" $CCACHE_COMPILE -arch x86_64 -Xarch_x86_64 -I. -c test1.c expect_stat direct_cache_hit 3