@@ -135,13 +135,72 @@ documented index enum) instead of bare positional arrays would prevent off-by-on |
135 | 135 |
|
136 | 136 | --- |
137 | 137 |
|
| 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 | + |
138 | 192 | ## Summary priority for the language/tooling agent |
139 | 193 |
|
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 |
142 | 200 | 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. |
145 | 204 |
|
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. |