Skip to content

Commit 95b579b

Browse files
committed
painpoints: add nodejs#8 — extern struct layout is an unverified claim, wrong offsets fail silently. affects every milo user of the feature, not just this port: compiler takes the layout on faith, a wrong field reads a neighbouring one and returns plausible garbage w/ no error. workaround (c _Static_assert) needs a c file most pure-milo users don't have. ranked fixes: docs -> #[c_layout] compiler-emitted guards (~1-2d, 90% of safety) -> @cImport (weeks). also note the same faith applies to 341 hand-written extern fn decls, and that unsafe deliberately doesn't cover either (tracks provenance, not layout/effects). mark nodejs#3 unsafe-lint resolved+shipped
1 parent aea032f commit 95b579b

1 file changed

Lines changed: 65 additions & 6 deletions

File tree

src/milo/MILO_PAINPOINTS.md

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,72 @@ documented index enum) instead of bare positional arrays would prevent off-by-on
135135

136136
---
137137

138+
## 8. `extern struct` layout is an unverified claim — wrong offsets fail SILENTLY
139+
140+
**Status 2026-07-16.** The one that bites every Milo user, not just this port.
141+
142+
**Symptom.** `extern struct` reads as though the compiler knows the C type's layout. It does not —
143+
it's an assertion the compiler takes on faith. Get a field's order/type/size wrong and there is no
144+
error and no crash: the read lands on a *neighbouring field* and returns plausible garbage. This is
145+
the worst failure shape available — looks safe, fails silently, corrupts data quietly.
146+
147+
**Repro.** Declare a struct with one field's type wrong (`st_ino: u32` instead of `u64`). Everything
148+
after it shifts by 4 bytes. `stat().size` returns some other field's bytes. Compiles clean, runs,
149+
returns wrong numbers forever. Nothing on the Milo side can catch it — Milo never sees `<sys/stat.h>`.
150+
151+
**Why it's worse than it looks.** The workaround requires you to (a) already know the trap exists,
152+
and (b) have a C compilation unit in your build to put `_Static_assert(offsetof(...))` in. A pure-Milo
153+
program has neither. We only found it here because this port happens to have `entry.c`.
154+
155+
**What we did (node-milo, commits 5c28a8f5d93 + aea032fc35c).** Hand-wrote 23 `_Static_assert`s in
156+
`entry.c` guarding `Stat`/`Timespec`/`Timeval`/`Rusage` — C sees the real headers, so a drifted layout
157+
now breaks the build with a named error. Verified the guard bites by deliberately breaking an offset.
158+
This works but it does NOT generalize: it's manual, opt-in, per-struct, per-field, and unavailable to
159+
anyone without a C file. Nothing warns when a *new* `extern struct` ships with no guard.
160+
161+
**Fixes, cheapest first:**
162+
163+
1. **Docs** (~1hr) — the `extern struct` section must state plainly that layout is unchecked and a wrong
164+
field silently reads garbage. Right now nothing warns you. Even node-milo's own CLAUDE.md presents
165+
`extern struct` as the *safe* option vs manual offsets — true for readability, silent on verification.
166+
2. **Compiler-emitted layout guards** (~1-2 days) — best cost/benefit. Let the user annotate:
167+
```milo
168+
#[c_layout("struct stat", "sys/stat.h")]
169+
extern struct Stat { st_dev: i32, ... }
170+
```
171+
Compiler computes each field's offset (it already does this for codegen), emits a throwaway C TU of
172+
`_Static_assert(offsetof(struct stat, st_dev) == 0, ...)`, and compiles it with the system cc as part
173+
of the build. Turns a faith-based claim into a compile-time-checked one, for every user, with no C
174+
file of their own. Field names already match in practice; annotation carries the header + C type name.
175+
3. **`@cImport`-style header ingestion** (weeks) — derive the layout from the header, delete the
176+
hand-transcription entirely. What zig does. Correct endgame, big lift. #2 gets ~90% of the safety
177+
for ~5% of the work, and is a stepping stone (same offset-computing machinery).
178+
179+
**Adjacent.** Same faith-based hole applies to `extern fn` decls: node-milo has **341** hand-written
180+
extern signatures, none checked against the real symbol. A wrong arity/type is UB that no `unsafe`
181+
marker would flag — the mistake is in the *description* of the boundary, not the crossing of it.
182+
`#[c_layout]`-style checking could extend to signatures (`_Static_assert(sizeof(&fn) ...)`-ish, or
183+
just emitting a C TU that takes the function's address at the declared type — a mismatched decl then
184+
fails to compile). Lower priority than structs; scalar ABI mismatches are usually loud-ish, struct
185+
layout drift is always silent.
186+
187+
**Related.** `unsafe` correctly does NOT cover this (it tracks memory *provenance*, not layout claims
188+
or side effects) — which is itself worth a docs note, since "no unsafe" reads as "verified" to newcomers.
189+
190+
---
191+
138192
## Summary priority for the language/tooling agent
139193

140-
1. **Parse error quality** (#1, #2) — source line + caret + "expected" set. Biggest daily friction.
141-
2. **JS error stack attribution** (#6) — biggest *correctness* blocker; causes hours lost on
194+
1. **`extern struct` layout unverified** (#8) — **silent data corruption**, affects every user of the
195+
feature, and the workaround needs a C file most users don't have. Highest *severity* on this list;
196+
fix #2 (compiler-emitted guards) is ~1-2 days.
197+
2. **Parse error quality** (#1, #2) — source line + caret + "expected" set. Biggest daily friction.
198+
[RESOLVED]
199+
3. **JS error stack attribution** (#6) — biggest *correctness* blocker; causes hours lost on
142200
non-reproducible-in-isolation failures.
143-
3. **`unsafe` redundancy lint** (#3) — removes guess-and-rebuild cycles.
144-
4. Build failure summarization (#4), native-rebuild signaling (#5), struct returns (#7) — nice-to-haves.
201+
4. **`unsafe` redundancy lint** (#3) — removes guess-and-rebuild cycles. [RESOLVED — shipped; used it
202+
to strip 91 redundant `unsafe` from node-milo, commit 489fbd941dc. Worked exactly as asked.]
203+
5. Build failure summarization (#4), native-rebuild signaling (#5), struct returns (#7) — nice-to-haves.
145204

146-
None of these are blockers for *shipping* features (I've landed ~20 fs commits this session).
147-
They're velocity + debuggability taxes. #1 and #6 are where an hour here and there keeps going.
205+
Most of these are velocity + debuggability taxes, not shipping blockers. **#8 is the exception** — it's
206+
a correctness/silent-corruption issue in a feature the docs actively recommend.

0 commit comments

Comments
 (0)