Merge: an enum member is checked against the i32 it will be
This commit is contained in:
commit
a2449ed7c1
@ -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
59
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 \
|
||||
|
||||
@ -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])"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user