Skip to content

Report Windows COFF archive output failures - #9440

Merged
mcourteaux merged 1 commit into
halide:mainfrom
gregcotten:fix-coff-archive-output-errors
Sep 13, 2026
Merged

mcourteaux merged 1 commit into
halide:mainfrom
gregcotten:fix-coff-archive-output-errors

Conversation

@gregcotten

@gregcotten gregcotten commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #9439. Related: #9438.

Direct COFF archive generation can return success after failing to write its destination. An old readable .lib may 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_to and generator_cache tests. 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. Checks that catch CompileError run only when Halide supports exceptions; the normal object and assembly checks still run in exception-disabled builds.

Before rebasing, 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 .lib failures 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 and git diff --check pass.

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. Static Halide linkage was not tested locally. This patch handles generation errors; failed cache restoration is covered separately by #9438.

The branch is rebased onto current main, including #9436. The updated compile_to test passes locally with its exception guards enabled and disabled, using the previously validated COFF-fixed DLL. The updated head passes the GitHub Actions suite, including the Linux Makefile job; the remaining buildbot jobs are pending.

Coverage is 87.5% for changed lines in the macOS report. The sole partial line is the final stream-state assertion; its failure branch is exercised by the Windows byte-range-lock regression, and Windows CI passes. The coverage report does not collect Windows execution.

@gregcotten

Copy link
Copy Markdown
Contributor 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

@alexreinking

Copy link
Copy Markdown
Member

@gregcotten — could you please rebase on main? GitHub isn't picking up #9436.

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.
Run exception-based checks only when Halide supports exceptions.

Fixes halide#9439

Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
@gregcotten
gregcotten force-pushed the fix-coff-archive-output-errors branch from 4a5f844 to 01d866e Compare September 11, 2026 15:17
@codecov

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 70.12%. Comparing base (58868f0) to head (01d866e).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/LLVM_Output.cpp 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9440      +/-   ##
==========================================
+ Coverage   69.95%   70.12%   +0.17%     
==========================================
  Files         261      261              
  Lines       79596    79604       +8     
  Branches    19400    19402       +2     
==========================================
+ Hits        55678    55825     +147     
+ Misses      18004    17980      -24     
+ Partials     5914     5799     -115     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gregcotten

Copy link
Copy Markdown
Contributor Author

@alexreinking done!

@mcourteaux
mcourteaux merged commit d10a1f2 into halide:main Sep 13, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows COFF archive generation can silently fail and cache stale output

3 participants