From 4d6a2ce25809620f73e61852d0e252e1b2942006 Mon Sep 17 00:00:00 2001 From: Greg Cotten <924470+gregcotten@users.noreply.github.com> Date: Thu, 10 Sep 2026 11:39:27 -0700 Subject: [PATCH 1/2] Reject failed generator-cache output replacement Check the fallback copy before reporting a hit, preserve its error, and clean pending temporary files. An output that cannot be replaced must fail the generator instead of leaving stale bytes behind a successful command. Extend the existing correctness test with a portable output obstruction and successful restoration after the obstruction is removed. Fixes halide/Halide#9437 Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> --- src/GeneratorCache.cpp | 8 ++++++++ src/GeneratorCache.h | 5 +++-- test/correctness/generator_cache.cpp | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/GeneratorCache.cpp b/src/GeneratorCache.cpp index 94c023b1af89..80c8ab2ece59 100644 --- a/src/GeneratorCache.cpp +++ b/src/GeneratorCache.cpp @@ -637,6 +637,14 @@ bool GeneratorCache::try_restore(const std::string &key, if (ec) { // Fall back to a copy if rename across the tmp/dest pair failed. fs::copy_file(p.first, p.second, fs::copy_options::overwrite_existing, ec); + if (ec) { + const std::string message = ec.message(); + for (const auto &pending_file : pending) { + fs::remove(pending_file.first, ec); + } + user_error << "GeneratorCache: failed to restore " << p.second.string() + << ": " << message << "\n"; + } fs::remove(p.first, ec); } } diff --git a/src/GeneratorCache.h b/src/GeneratorCache.h index 2942b9c1be0e..f6bed084b133 100644 --- a/src/GeneratorCache.h +++ b/src/GeneratorCache.h @@ -79,8 +79,9 @@ struct GeneratorCache { /** If a complete entry for `key` exists in the cache, copy each cached * file into the destination path given by `output_files` and return - * true. Otherwise leave the filesystem untouched and return false. A - * partial or corrupt entry is treated as a miss. */ + * true only if all outputs were restored. An output replacement failure + * is reported as a user error; some outputs may already have been replaced. + * A partial or corrupt entry is treated as a miss. */ static bool try_restore(const std::string &key, const std::map &output_files); diff --git a/test/correctness/generator_cache.cpp b/test/correctness/generator_cache.cpp index b6d48ebb8ad9..d3c6ddc5bacf 100644 --- a/test/correctness/generator_cache.cpp +++ b/test/correctness/generator_cache.cpp @@ -162,6 +162,25 @@ int main(int argc, char **argv) { check(read_all(obj_c) != sentinel); check(read_all(obj_c) != real_obj); + // A complete cache entry is not a hit if its output cannot be replaced. + // A nonempty directory blocks both rename and copy on every platform, + // including when the test runs with permission to write read-only files. + const fs::path blocked_dir = tmp / "blocked"; + const fs::path blocked_obj = blocked_dir / ("cache_add" + obj_ext); + fs::create_directories(blocked_obj); + { std::ofstream marker(blocked_obj / "keep"); } + const int blocked_status = Internal::run_process( + {self, "--gen", blocked_dir.string(), "2"}); + check(blocked_status != 0); + check(fs::exists(blocked_obj / "keep")); + check(!fs::exists(blocked_obj.string() + ".hlcache.tmp")); + check(!fs::exists((blocked_dir / "cache_add.h").string() + ".hlcache.tmp")); + + // Once the obstruction is removed, the cached implementation restores. + fs::remove_all(blocked_obj); + check(Internal::run_process({self, "--gen", blocked_dir.string(), "2"}) == 0); + check(read_all(blocked_obj) == read_all(obj_c)); + // 4. Corrupt every cache entry (drop manifests): the next offset=1 run must // treat the entry as a miss and recompile the real object. for (const auto &e : fs::recursive_directory_iterator(entries)) { From b61f6995c0f24b8b7fcc76ee08fe90acd1210cf8 Mon Sep 17 00:00:00 2001 From: Greg Cotten <924470+gregcotten@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:01:20 -0700 Subject: [PATCH 2/2] Preserve coverage from expected cache-test errors Catch CompileError in the test child and return a nonzero status, allowing its coverage profile to be written instead of losing it when the child aborts. Cache failure, cleanup, recovery, and reuse checks remain active. An instrumented Windows comparison confirms that the failed child's profile is absent before this change and present afterward. Production error handling is unchanged. Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> --- test/correctness/generator_cache.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/correctness/generator_cache.cpp b/test/correctness/generator_cache.cpp index d3c6ddc5bacf..c71c268c50a1 100644 --- a/test/correctness/generator_cache.cpp +++ b/test/correctness/generator_cache.cpp @@ -51,7 +51,16 @@ int generate_one(const std::string &outdir, const std::string &offset) { args.targets = {get_host_target()}; args.generator_name = "cache_add"; args.generator_params = {{"offset", offset}}; - Internal::execute_generator(args); +#ifdef HALIDE_WITH_EXCEPTIONS + try { +#endif + Internal::execute_generator(args); +#ifdef HALIDE_WITH_EXCEPTIONS + } catch (const CompileError &e) { + std::cerr << e.what(); + return 1; + } +#endif return 0; }