Incremental build reuses stale artifacts after a module-interface layout change, producing a corrupt binary #27

New issue

Closed

opened 2026-07-30 16:51:34 +00:00 by jorijnvdgraaf · 0 comments

jorijnvdgraaf commented

2026-07-30 16:51:34 +00:00

Owner

Summary

Adding a data member to a class defined in a module interface does not cause every dependent
translation unit to be rebuilt. The resulting binary mixes translation units compiled against the
old and new class layouts, which is undefined behaviour — in practice a SIGSEGV inside a
destructor.

The failure mode is what makes this worth fixing rather than the inconvenience: there is no link
error and no warning. The build succeeds, and the corruption surfaces later as a crash that looks
like a bug in the library you just pulled. I spent a while preparing a regression report against
Crafter.Network's HTTPS merge before establishing that the merge was fine and the build was
stale.

Reproduction

Deterministic, using Crafter.Network at the commits below. 219a31a is the commit before the
HTTPS merge; 0827e1f is the merge. The merge adds a std::string defaultScheme member to
HTTP1::MessageParser in interfaces/Crafter.Network-HTTP1.cppm.

cd Crafter.Network

# 1. clean build of the older commit
git checkout 219a31a
rm -rf bin build
crafter-build test            # -> 14 passed, 0 crashed

# 2. move to the commit that adds the interface member, and build INCREMENTALLY
git checkout master           # 0827e1f
crafter-build test            # -> 13 passed, 5 crashed (SIGSEGV)

# 3. same source, clean build
rm -rf bin build
crafter-build test            # -> 18 passed, 0 crashed

Step 2 and step 3 build identical source. The only difference is the stale bin/ and build/.

Crash

All five failures are the tests that exercise ClientHTTP1 over plaintext. Under gdb:

Thread 1 "ShouldSendRecie" received signal SIGSEGV, Segmentation fault.
0x0000555555567b60 in Crafter::HTTP1::MessageParser@Crafter.Network::~MessageParser() ()
#0  ~MessageParser()
#1  Crafter::ClientHTTP1@Crafter.Network::Impl::Exchange(...)
#2  Crafter::ClientHTTP1@Crafter.Network::Send(Crafter::HTTPRequest const&)
#3  main

A crash in the destructor of exactly the class whose layout changed is consistent with a
size/offset mismatch between TUs: one side constructs an object of the old size, another destroys
it believing the new layout, and the std::string member added at the end is destroyed at the
wrong address.

Note that ShouldInteropCurlHTTP1 passes in step 2. It drives the listener with curl
rather than ClientHTTP1, so it never constructs the mismatched object — which is why the failure
pattern looks like "the plaintext client is broken" rather than "the build is stale."

Diagnosis (inferred, not isolated)

I have not narrowed down which specific artifact is stale, so treat the mechanism as a hypothesis
supported by the evidence rather than a confirmed root cause. What is confirmed is that identical
source produces a crashing binary incrementally and a working binary from clean.

The likely shape: the dependency tracking treats an interface change as requiring a rebuild of the
module's own BMI and of TUs that import it, but something in the chain — a cached BMI, an object
file, or a static archive member — is reused when it should not be. Adding a member is the
interesting case because it changes layout while keeping every signature and mangled name
identical, so nothing downstream fails to link.

Suggested direction

Some options, roughly in increasing order of effort:

Hash the interface content into the identity of dependent artifacts, so any change to a
.cppm invalidates everything that imports it, not just its own BMI. Coarse but correct, and
module interfaces do not change often enough for the rebuild cost to matter.
Fail loudly instead of silently. If a BMI and a consuming object disagree about the
interface they were built against, that should be a build error. A wrong answer is much worse
than a slow one here.
A --clean / crafter-build clean subcommand would at least make the workaround discoverable.
Right now the fix is rm -rf bin build, which you only try if you already suspect the build.

Option 2 seems most valuable regardless of which of the others is taken: this class of bug should
never reach a running binary.

Separate symptom, possibly the same underlying theme of "the variant id does not capture everything
that affects the output", so mentioning it here — happy to split it into its own issue.

crafter-build and crafter-build -- --no-webgpu (a Crafter.Graphics project arg that changes
which files cfg.files contains) both produce
bin/Catcrafts.Net-wasm32-wasip1-native-native-df37fe0fe124fe57/. Because the directory is shared
and reused rather than replaced, switching between the two leaves a bundle matching neither config:
index.html is regenerated without the dom-webgpu.js script tag while the stale dom-webgpu.js
file is still sitting in the directory. rm -rf bin between flag changes is the workaround.

Environment

Arch Linux, clang + libc++, wasi-libc for the wasm target
Crafter.Network at 0827e1f, reproduced against 219a31a
Reproduced twice before the clean build, and once more via the three-step sequence above

## Summary Adding a data member to a class defined in a module **interface** does not cause every dependent translation unit to be rebuilt. The resulting binary mixes translation units compiled against the old and new class layouts, which is undefined behaviour — in practice a `SIGSEGV` inside a destructor. The failure mode is what makes this worth fixing rather than the inconvenience: there is no link error and no warning. The build succeeds, and the corruption surfaces later as a crash that looks like a bug in the library you just pulled. I spent a while preparing a regression report against `Crafter.Network`'s HTTPS merge before establishing that the merge was fine and the build was stale. ## Reproduction Deterministic, using `Crafter.Network` at the commits below. `219a31a` is the commit before the HTTPS merge; `0827e1f` is the merge. The merge adds a `std::string defaultScheme` member to `HTTP1::MessageParser` in `interfaces/Crafter.Network-HTTP1.cppm`. ```sh cd Crafter.Network # 1. clean build of the older commit git checkout 219a31a rm -rf bin build crafter-build test # -> 14 passed, 0 crashed # 2. move to the commit that adds the interface member, and build INCREMENTALLY git checkout master # 0827e1f crafter-build test # -> 13 passed, 5 crashed (SIGSEGV) # 3. same source, clean build rm -rf bin build crafter-build test # -> 18 passed, 0 crashed ``` Step 2 and step 3 build identical source. The only difference is the stale `bin/` and `build/`. ## Crash All five failures are the tests that exercise `ClientHTTP1` over plaintext. Under gdb: ``` Thread 1 "ShouldSendRecie" received signal SIGSEGV, Segmentation fault. 0x0000555555567b60 in Crafter::HTTP1::MessageParser@Crafter.Network::~MessageParser() () #0 ~MessageParser() #1 Crafter::ClientHTTP1@Crafter.Network::Impl::Exchange(...) #2 Crafter::ClientHTTP1@Crafter.Network::Send(Crafter::HTTPRequest const&) #3 main ``` A crash in the destructor of exactly the class whose layout changed is consistent with a size/offset mismatch between TUs: one side constructs an object of the old size, another destroys it believing the new layout, and the `std::string` member added at the end is destroyed at the wrong address. Note that `ShouldInteropCurlHTTP1` **passes** in step 2. It drives the listener with `curl` rather than `ClientHTTP1`, so it never constructs the mismatched object — which is why the failure pattern looks like "the plaintext client is broken" rather than "the build is stale." ## Diagnosis (inferred, not isolated) I have not narrowed down which specific artifact is stale, so treat the mechanism as a hypothesis supported by the evidence rather than a confirmed root cause. What is confirmed is that identical source produces a crashing binary incrementally and a working binary from clean. The likely shape: the dependency tracking treats an interface change as requiring a rebuild of the module's own BMI and of TUs that import it, but something in the chain — a cached BMI, an object file, or a static archive member — is reused when it should not be. Adding a member is the interesting case because it changes layout while keeping every signature and mangled name identical, so nothing downstream *fails* to link. ## Suggested direction Some options, roughly in increasing order of effort: 1. **Hash the interface content into the identity of dependent artifacts**, so any change to a `.cppm` invalidates everything that imports it, not just its own BMI. Coarse but correct, and module interfaces do not change often enough for the rebuild cost to matter. 2. **Fail loudly instead of silently.** If a BMI and a consuming object disagree about the interface they were built against, that should be a build error. A wrong answer is much worse than a slow one here. 3. A `--clean` / `crafter-build clean` subcommand would at least make the workaround discoverable. Right now the fix is `rm -rf bin build`, which you only try if you already suspect the build. Option 2 seems most valuable regardless of which of the others is taken: this class of bug should never reach a running binary. ## Possibly related: project args are not part of the variant identity Separate symptom, possibly the same underlying theme of "the variant id does not capture everything that affects the output", so mentioning it here — happy to split it into its own issue. `crafter-build` and `crafter-build -- --no-webgpu` (a `Crafter.Graphics` project arg that changes which files `cfg.files` contains) both produce `bin/Catcrafts.Net-wasm32-wasip1-native-native-df37fe0fe124fe57/`. Because the directory is shared and reused rather than replaced, switching between the two leaves a bundle matching neither config: `index.html` is regenerated without the `dom-webgpu.js` script tag while the stale `dom-webgpu.js` file is still sitting in the directory. `rm -rf bin` between flag changes is the workaround. ## Environment - Arch Linux, clang + libc++, wasi-libc for the wasm target - `Crafter.Network` at `0827e1f`, reproduced against `219a31a` - Reproduced twice before the clean build, and once more via the three-step sequence above

jorijnvdgraaf added the

claude:ready

label

2026-07-30 16:55:00 +00:00