JosephFerano/flan
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

An enum member is checked against the i32 it will be, before anything reasons about it

Browse Source 
The members of a defenum are i32 at run time, but the reader hands the parser
an int64, so a value too large for the type arrived looking ordinary: truncated
by the x86 backend, malformed in the LLVM IR, and -- the reason this is a
correctness hole and not a nicety -- invisible to the duplicate-value rule
sitting right below it. That rule compares int64s, so (defenum E [A 0
B 4294967296]) passed it: the two differ as int64 and are both 0 as i32, and
the one check written to catch two names for one number waved through exactly
the case it exists for.

Each value is now checked where it is resolved, which is before the collision
scan runs, so the scan compares the numbers the program will actually have. A
value that does not fit is refused rather than quietly made to fit, naming the
member, its enum, and the value, with a different sentence for a value that was
written and one autoincrement walked into -- nothing in the source wrote
2147483648, so the refusal has to say where it came from before it can say it
is wrong.

The check is bound with a let rather than inlined into the cons, and that is
load-bearing: OCaml leaves :: operand order unspecified and takes the tail
first, so an inlined check would run after the recursive Int64.add and let
(defenum E [A 9223372036854775807 B]) wrap to min_int and refuse B for a number
in no one's source. Bound first, A is refused and the wrap is unreachable.

The parser is the only place this needs to happen: Parse.decl is the sole
constructor of Ast.Defenum's member values, and Load only re-qualifies the
enum's name.

Explicit-duplicate aliasing is untouched; that rule is deliberate.
This commit is contained in:
Joseph Ferano 2026-09-18 07:29:09 +07:00
parent ce3e879e83
commit 9585d8c2fe
3 changed files with 87 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
docs/BUGS-2026-09-18.md
View File

@ -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

59
lib/parse.ml
View File

@ -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 \

26
test/test_flan.ml
View File

@ -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])"