Report Windows COFF archive output failures - #9440
Open
gregcotten wants to merge 2 commits into
Open
Conversation
Reject archive open/write failures before generation can report success or publish stale output to the generator cache. Flush before archive seeks so buffered write failures remain visible in the output stream state. Extend the existing compile-to and cache correctness tests with failed opening, failure after opening, cache publication, and recovery coverage. Fixes halide#9439 Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> [skip ci]
alexreinking
approved these changes
Sep 10, 2026
Request the normal pull-request checks now that the fix is submitted for upstream review. Preserve the reviewed patch and its existing history. Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Author
|
@alexreinking, sorry these were committed with [skip ci] - I made a dummy commit to trigger workflows. Please approve! Needless to say this should be a squash merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9439. Related: #9438.
Direct COFF archive generation can return success after failing to write its destination. An old readable
.libmay then be cached under the requested pipeline's key. This also reproduces with the generator cache disabled.Check archive opening and final stream state, and flush explicitly before the writer's seeks so a buffered write failure is recorded before seeking. A Windows byte-range-lock regression demonstrates why checking only opening and final state is insufficient on the tested MSVC implementation. Failed writes now stop compilation before cache publication; successful archives remain byte-identical and the fix adds no archive-sized buffer.
The change adds eight executable lines and a two-line comment in the COFF writer, plus regressions in the existing
compile_toandgenerator_cachetests. They cover blocked opening, failure after opening, recovery, absence of a cache entry after failed generation, normal cache hits and changed pipelines. The path depends on the Windows COFF output target, not exclusively on the host operating system.Verified locally with a full shared Halide build, both independently and together with the restore fix in #9438: three affected correctness tests pass; the combined build also passes both normal CMake cache integration tests, including generated-code execution. The standalone integer-add reproducer confirms locked
.libfailures with caching disabled and with a cold cache, correct recovery, and unchanged successful control bytes. The original wheel fails the new regressions; the after-open test also rejects the incomplete open/end-only fix. clang-format 21.1.8 andgit diff --checkpass.Environment: Windows x64, C++20, serialization enabled, LLVM 22.1.8, MSVC 19.42 compiler, isolated 14.44 linker/CRT libraries and SDK 10.0.22621.0. Patched macOS/Linux, static Halide linkage and full CI remain unverified. This patch handles generation errors; failed cache restoration is covered separately by #9438.
Normal upstream CI has now been requested for the unchanged patch; the fork workflows are awaiting maintainer approval to run. The remaining cross-platform, static-linkage and CI checks are required before claiming merge readiness.