Report Windows COFF archive output failures

src/LLVM_Output.cpp

@@ -117,8 +117,12 @@ std::map<std::string, size_t> write_string_table(std::ostream &out,
             out.put('\x0A');
         }
         size_t final_offset = out.tellp();
+        // Flush explicitly so a buffered write failure sets the stream state
+        // before seeking; filebuf seek implementations may lose that error.
+        out.flush();
         out.seekp(start_offset - 12);
         emit_padded(out, member_end - start_offset, 10);
+        out.flush();
         out.seekp(final_offset);
     }
     return string_to_offset_map;
@@ -242,14 +246,17 @@ void write_symbol_table(std::ostream &out,
 
     // Patch the size of the symbol table.
     const size_t member_header_size = 60;
+    out.flush();
     out.seekp(symbol_table_size_offset);
     emit_padded(out, member_end - member_header_size - header_start_offset, 10);
 
     // Patch the number of symbols.
+    out.flush();
     out.seekp(symbol_count_offset);
     emit_u32(out, name_to_member_index.size());
 
     // Seek back to where we left off.
+    out.flush();
     out.seekp(final_offset);
 }
 
@@ -298,6 +305,7 @@ void write_coff_archive(std::ostream &out,
     for (auto &it : patchers) {
         size_t i = it.first;
         for (auto &patcher : it.second) {
+            out.flush();
             out.seekp(patcher.pos);
             patcher.emit_u32(out, member_offset.at(i));
         }
@@ -602,9 +610,11 @@ void create_static_library(const std::vector<std::string> &src_files_in, const T
     // the same as GNU ar format.
     if (Internal::get_triple_for_target(target).isWindowsMSVCEnvironment()) {
         std::ofstream f(dst_file, std::ios_base::trunc | std::ios_base::binary);
+        user_assert(f.is_open()) << "Failed to open archive for writing: " << dst_file << "\n";
         Internal::Archive::write_coff_archive(f, new_members);
         f.flush();
         f.close();
+        user_assert(f) << "Failed to write archive: " << dst_file << "\n";
         return;
     }
 

test/correctness/compile_to.cpp

@@ -2,6 +2,9 @@
 #include "halide_test_dirs.h"
 
 #include <cstdio>
+#include <filesystem>
+#include <fstream>
+#include <memory>
 
 using namespace Halide;
 
@@ -31,6 +34,64 @@ void testCompileToOutputAndAssembly(Func j) {
     Internal::assert_file_exists(fn_assembly);
 }
 
+#if HALIDE_WITH_EXCEPTIONS
+void testCoffArchiveOutputError(Func j) {
+    // This tests the output format, including on non-Windows hosts.
+    const Target target("x86-64-windows-no_runtime");
+    if (!target.supported()) {
+        return;
+    }
+    namespace fs = std::filesystem;
+    const fs::path archive = Internal::get_test_tmp_dir() + "compile_to_blocked.lib";
+    fs::create_directory(archive);
+    { std::ofstream marker(archive / "keep"); }
+
+    const std::map<OutputFileType, std::string> outputs = {
+        {OutputFileType::static_library, archive.string()}};
+    bool failed = false;
+    try {
+        j.compile_to(outputs, j.infer_arguments(), "coff_output", target);
+    } catch (const CompileError &e) {
+        failed = true;
+        internal_assert(std::string(e.what()).find(archive.string()) != std::string::npos);
+    }
+    internal_assert(failed) << "An unwritable archive destination must fail compilation.\n";
+    internal_assert(fs::exists(archive / "keep"));
+
+    fs::remove_all(archive);
+    j.compile_to(outputs, j.infer_arguments(), "coff_output", target);
+    internal_assert(fs::is_regular_file(archive) && fs::file_size(archive) > 0);
+
+#ifdef _WIN32
+    {
+        // Allow opening/truncating the file, but block writing its first byte.
+        // Closing the handle releases the byte-range lock, even on exceptions.
+        std::unique_ptr<void, decltype(&CloseHandle)> handle(
+            CreateFileW(archive.c_str(), GENERIC_READ | GENERIC_WRITE,
+                        FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+                        nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr),
+            &CloseHandle);
+        internal_assert(handle.get() != INVALID_HANDLE_VALUE);
+        OVERLAPPED overlapped{};
+        internal_assert(LockFileEx(handle.get(), LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY,
+                                   0, 1, 0, &overlapped));
+        failed = false;
+        try {
+            j.compile_to(outputs, j.infer_arguments(), "coff_output", target);
+        } catch (const CompileError &e) {
+            failed = true;
+            internal_assert(std::string(e.what()).find(archive.string()) != std::string::npos);
+        }
+        internal_assert(failed) << "An archive write failure after opening must fail compilation.\n";
+        internal_assert(fs::file_size(archive) == 0) << "The output was not opened and truncated.\n";
+    }
+    j.compile_to(outputs, j.infer_arguments(), "coff_output", target);
+    internal_assert(fs::file_size(archive) > 0);
+#endif
+    fs::remove(archive);
+}
+#endif
+
 int main(int argc, char **argv) {
     Func f, g, h, j;
     Var x, y;
@@ -47,6 +108,12 @@ int main(int argc, char **argv) {
 
     testCompileToOutputAndAssembly(j);
 
+#if HALIDE_WITH_EXCEPTIONS
+    if (Halide::exceptions_enabled()) {
+        testCoffArchiveOutputError(j);
+    }
+#endif
+
     printf("Success!\n");
     return 0;
 }

test/correctness/generator_cache.cpp

@@ -44,11 +44,12 @@ class CacheAdd : public Generator<CacheAdd> {
 
 // Run one generation into `outdir` with the given offset. Used by the --gen
 // child process. HL_CACHE_DIR is inherited from the parent's environment.
-int generate_one(const std::string &outdir, const std::string &offset) {
+int generate_one(const std::string &outdir, const std::string &offset, bool coff = false) {
     Internal::ExecuteGeneratorArgs args;
     args.output_dir = outdir;
-    args.output_types = {OutputFileType::object, OutputFileType::c_header};
-    args.targets = {get_host_target()};
+    args.output_types = {coff ? OutputFileType::static_library : OutputFileType::object,
+                         OutputFileType::c_header};
+    args.targets = {coff ? Target("x86-64-windows-no_runtime") : get_host_target()};
     args.generator_name = "cache_add";
     args.generator_params = {{"offset", offset}};
     Internal::execute_generator(args);
@@ -110,6 +111,9 @@ int main(int argc, char **argv) {
     if (argc == 4 && std::string(argv[1]) == "--gen") {
         return generate_one(argv[2], argv[3]);
     }
+    if (argc == 5 && std::string(argv[1]) == "--gen" && std::string(argv[4]) == "--coff") {
+        return generate_one(argv[2], argv[3], true);
+    }
 
     const std::string self = fs::absolute(argv[0]).string();
     const fs::path tmp =
@@ -172,6 +176,42 @@ int main(int argc, char **argv) {
     const fs::path obj_d = run("d", "1");
     check(read_all(obj_d) == real_obj);
 
+    if (Target("x86-64-windows-no_runtime").supported()) {
+        // A failed fresh COFF archive write must not publish a cache entry.
+        const fs::path coff_cache = tmp / "coff-cache";
+        const fs::path coff_entries = coff_cache / "entries";
+        const fs::path coff_dir = tmp / "coff";
+        const fs::path archive = coff_dir / "cache_add.lib";
+        fs::create_directories(coff_cache);
+        fs::create_directories(archive);
+        { std::ofstream marker(archive / "keep"); }
+        set_cache_dir(coff_cache.string());
+        const std::vector<std::string> command = {self, "--gen", coff_dir.string(), "1", "--coff"};
+        check(Internal::run_process(command) != 0);
+        check(fs::exists(archive / "keep"));
+        if (fs::exists(coff_entries)) {
+            for (const auto &entry : fs::recursive_directory_iterator(coff_entries)) {
+                check(entry.path().filename() != "manifest.txt");
+            }
+        }
+
+        fs::remove_all(archive);
+        check(Internal::run_process(command) == 0);
+        check(!read_all(archive).empty());
+
+        // Tampering with the published archive makes a normal hit observable.
+        const fs::path archive_blob = find_blob(coff_entries, OutputFileType::static_library);
+        check(!archive_blob.empty());
+        {
+            std::ofstream f(archive_blob, std::ios::binary | std::ios::trunc);
+            f.write((const char *)sentinel.data(), sentinel.size());
+        }
+        check(Internal::run_process(command) == 0);
+        check(read_all(archive) == sentinel);
+        check(Internal::run_process({self, "--gen", coff_dir.string(), "2", "--coff"}) == 0);
+        check(!read_all(archive).empty() && read_all(archive) != sentinel);
+    }
+
     fs::remove_all(tmp);
     std::cout << "Success!\n";
     return 0;