diff --git a/docs/BUGS-2026-09-18.md b/docs/BUGS-2026-09-18.md index 1a3b14c..a143d87 100644 --- a/docs/BUGS-2026-09-18.md +++ b/docs/BUGS-2026-09-18.md @@ -96,9 +96,10 @@ territory; fix or record, the lane's call. - **`reg at` TOCTOU** (`dev.ml:1488`): the stopped-only gate is checked three round trips before the render thunk runs; a `restart` in between lets the thunk chase freed memory with the gate's blessing. -- **defenum values never range-checked to i32** (`parse.ml:994`, `check.ml:1666`): the - collision rule compares i64s, so `[A 0 B 4294967296]` passes and both are 0 at runtime; - autoincrement can overflow silently on --x86. +- ~~**defenum values never range-checked to i32**~~ — FIXED. Every resolved member value, + explicit or autoincremented, is range-checked against i32 in the parser before the collision scan, + so the scan compares the numbers the program will actually have. Out of range is refused + by name (`parse/enum-value-out-of-range`); explicit-duplicate aliasing stays legal. - **NaN sign**: LLVM constant-folds `0.0/0.0` to `nan`, x86 computes `-nan` — a stdout DIFFER on a two-line program. Runtime paths agree (`-nan` both). - **`emit.ml:3369` transient test ignores `new_globals`**: on the `retains=false` path a diff --git a/lib/parse.ml b/lib/parse.ml index a511938..2569ec6 100644 --- a/lib/parse.ml +++ b/lib/parse.ml @@ -987,14 +987,69 @@ let rec decl (f : Form.t) : Ast.decl = | List ({ v = Sym "defenum"; _ } :: args) -> (match args with | [ n; { v = Form.Vec ms; _ } ] -> + let ename = sym n in + (* An enum member is an [i32] at run time. [Shim] lowers the type to + int32_t for C's benefit and [Check] builds every member as a + [Tast.Int (v, I32)] -- but the reader hands this pass an [int64], so + a value the type cannot hold arrives here looking perfectly ordinary. + Left alone it is truncated by the x86 backend and malformed in the + LLVM IR, and worse than either, it defeats the duplicate-value rule + below: that rule compares [int64]s, so in + [(defenum E [A 0 B 4294967296])] the two values differ and the scan + passes, while at run time both members are 0 and a [match] on one is + unreachable through the other. The one rule written to catch two + names for one number waves through the case it exists for. + + So every value is checked against its run-time type here -- refused + if it does not fit, never truncated to fit -- the moment it is + resolved and before any of that reasoning runs, and the scan below + therefore compares the numbers the program will actually have. + + The bounds are the signed 32-bit ones and they are spelled out rather + than borrowed. [Check.in_range] is the function that does exactly + this for every ordinary literal, and it is the right one -- but + [Check] is built above this module and reading its output, so there + is no call this file could make. If one of the two changes, change + the other. *) + let fits m loc ~explicit (v : int64) = + if Int64.compare v (-2147483648L) >= 0 + && Int64.compare v 2147483647L <= 0 + then v + else if explicit then + Loc.failk "parse/enum-value-out-of-range" loc + "the member %s of %s is %Ld, which does not fit i32 — an enum's \ + discriminant is an i32, so its members run from -2147483648 to \ + 2147483647. Give %s a value in that range, or a defconst of a \ + wider type if the number itself is what matters" + m ename v m + else + Loc.failk "parse/enum-value-out-of-range" loc + "the member %s of %s has no value of its own, so it \ + autoincrements to %Ld, which does not fit i32 — an enum's \ + discriminant is an i32, so its members run from -2147483648 to \ + 2147483647. Write %s's value out, or lower the member above it" + m ename v m + in (* Each member becomes its name, its value, whether that value was written, and where the name is. The last two exist only so the - refusal below can be made; neither reaches the AST. *) + refusals here and below can be made; neither reaches the AST. *) let rec members next = function | [] -> [] | { v = Form.Sym m; loc } :: { v = Form.Int k; _ } :: rest -> + (* The [let] is load-bearing rather than tidiness. OCaml leaves the + evaluation order of [::]'s two operands unspecified and in + practice takes the tail first, so an inlined [fits ... k] would + run *after* the recursive call -- and for + [(defenum E [A 9223372036854775807 B])] that recursive call is + [Int64.add max_int 1L], which wraps quietly to min_int and would + have the refusal name B and a number written nowhere in the + source. Binding first refuses A, whose value is the one actually + wrong, and in doing so makes the wrap unreachable: [k] is inside + i32 by the time it is incremented, so the sum cannot overflow. *) + let k = fits m loc ~explicit:true k in (m, k, true, loc) :: members (Int64.add k 1L) rest | { v = Form.Sym m; loc } :: rest -> + let next = fits m loc ~explicit:false next in (m, next, false, loc) :: members (Int64.add next 1L) rest | bad :: _ -> fail bad @@ -1041,7 +1096,7 @@ let rec decl (f : Form.t) : Ast.decl = names, or a value no other member holds" m v other m) indexed; - mk (Ast.Defenum (sym n, List.map (fun (m, v, _, _) -> (m, v)) ms)) + mk (Ast.Defenum (ename, List.map (fun (m, v, _, _) -> (m, v)) ms)) | _ -> fail f "defenum is (defenum Name [member value? ...]). A member with no \ diff --git a/test/test_flan.ml b/test/test_flan.ml index 6dd8849..167c9c1 100644 --- a/test/test_flan.ml +++ b/test/test_flan.ml @@ -589,6 +589,32 @@ let () = "(defenum E [A B 0])" ~needle:"A has no value of its own, so it autoincrements to 0"; + (* A member is an i32 at run time, so a value outside i32 is refused where it + is resolved -- and the first of these is why the check has to happen + *before* the collision scan above rather than after it. 0 and 2^32 are + different int64s and the same i32, so the scan compares them, finds them + unequal, and passes a program in which both members are 0; the range + refusal is what stops it ever reaching that comparison. *) + parse_rejects "an explicit member too large for i32" + "(defenum E [A 0 B 4294967296])" + ~needle:"the member B of E is 4294967296, which does not fit i32"; + parse_rejects "the out-of-range refusal says what the range is" + "(defenum E [A 0 B 4294967296])" + ~needle:"its members run from -2147483648 to 2147483647"; + (* Nothing in the source wrote 2147483648, so the sentence has to say where + it came from before it can say it is wrong. *) + parse_rejects "an autoincrement off the top of i32" + "(defenum E [A 2147483647 B])" + ~needle:"B of E has no value of its own, so it autoincrements to 2147483648"; + (* The int64 end of the same problem. The member refused is [A], the one + whose value is actually wrong: were the range check to run after the + recursive call rather than before it, [Int64.add] would wrap past max_int + and the refusal would name B and the number min_int, which appears nowhere + in the program. This needle is the pin on that ordering. *) + parse_rejects "an explicit member at the top of i64 does not wrap" + "(defenum E [A 9223372036854775807 B])" + ~needle:"the member A of E is 9223372036854775807, which does not fit i32"; + (* Genuinely malformed input still says what a member is, in the grammar the form now has. *) parse_rejects "an enum member that is not a name" "(defenum E [1 A])"