Skip to content

Commit 198fe92

Browse files
committed
fix(type-checker): restrict if (var x = expr) RHS to presence-producing exprs
Reported by @SiyuanSun0736 on PR #19. Signed-off-by: Cong Wang <cwang@multikernel.io>
1 parent ccd2324 commit 198fe92

2 files changed

Lines changed: 73 additions & 4 deletions

File tree

src/type_checker.ml

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2054,12 +2054,37 @@ and type_check_statement ctx stmt =
20542054

20552055
| IfLet (name, expr, then_stmts, else_opt) ->
20562056
(* `if (var name = expr) { ... }` — bind `name` only inside then-branch.
2057-
The bound type matches what `var name = expr` would normally produce:
2058-
the value type for map access (auto-deref via IRMapAccess), and
2059-
the pointer type for raw pointer expressions. We just type-check the
2060-
RHS and use its inferred type as the binding type. *)
2057+
The bound type matches what `var name = expr` would normally
2058+
produce: the value type for map access (auto-deref via
2059+
IRMapAccess), and the pointer type for raw pointer expressions.
2060+
We restrict the RHS to "presence-producing" expressions, since
2061+
the construct's truthiness is defined as "expr produced a present
2062+
value" — i.e., a map hit or a non-null pointer. Allowing arbitrary
2063+
scalar / struct RHS would let the codegen emit `x != NULL`
2064+
against a non-pointer value (clang -Wpointer-integer-compare,
2065+
invalid C for struct types) and would let the evaluator's general
2066+
truthy-falsy rule diverge from the codegen's pointer presence
2067+
check. The legal shapes are:
2068+
- `m[k]` where `m` is a known map (auto-deref'd value type at
2069+
this layer, but underlying-pointer-checked at codegen)
2070+
- any expression of pointer type. *)
20612071
let typed_expr = type_check_expression ctx expr in
20622072
let bound_type = typed_expr.texpr_type in
2073+
let is_map_access_rhs = match expr.expr_desc with
2074+
| ArrayAccess ({ expr_desc = Identifier mn; _ }, _) ->
2075+
Hashtbl.mem ctx.maps mn
2076+
| _ -> false
2077+
in
2078+
let is_pointer_rhs = match bound_type with
2079+
| Pointer _ -> true
2080+
| _ -> false
2081+
in
2082+
if not (is_map_access_rhs || is_pointer_rhs) then
2083+
type_error
2084+
("`if (var " ^ name ^ " = expr)` requires expr to be a map access " ^
2085+
"(`m[k]`) or a pointer-typed expression; got " ^
2086+
string_of_bpf_type bound_type)
2087+
stmt.stmt_pos;
20632088
let saved = Hashtbl.find_opt ctx.variables name in
20642089
Hashtbl.replace ctx.variables name bound_type;
20652090
let typed_then = List.map (type_check_statement ctx) then_stmts in

tests/test_iflet.ml

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,48 @@ var stats : hash<u32, Stats>(1024)
273273
check bool "field write goes through the underlying lookup pointer" true
274274
(contains_substr c "->count =")
275275

276+
(** 11a. Reject: int-literal RHS — `if (var x = 5)` is not a presence check.
277+
The construct only makes sense when the RHS is a map access (auto-
278+
deref'd to a value but underlying-pointer-checked) or a pointer-typed
279+
expression. An integer RHS would lower to `__u32 x; if (x != NULL)`,
280+
which warns under -Wpointer-integer-compare and is semantically
281+
incoherent — also the evaluator's truthiness rules diverge from the
282+
codegen's `!= NULL` for non-pointer types. *)
283+
let test_reject_int_literal_rhs () =
284+
let source = {|
285+
@xdp fn probe(ctx: *xdp_md) -> xdp_action {
286+
if (var x = 5) {
287+
return XDP_PASS
288+
}
289+
return XDP_DROP
290+
}
291+
|} in
292+
try
293+
let _ = typecheck source in
294+
fail "expected rejection of integer-literal RHS"
295+
with
296+
| Kernelscript.Type_checker.Type_error _ -> ()
297+
298+
(** 11b. Reject: non-pointer-returning function RHS. *)
299+
let test_reject_non_pointer_call_rhs () =
300+
let source = {|
301+
@helper fn returns_zero() -> u32 {
302+
return 0
303+
}
304+
305+
@xdp fn probe(ctx: *xdp_md) -> xdp_action {
306+
if (var x = returns_zero()) {
307+
return XDP_PASS
308+
}
309+
return XDP_DROP
310+
}
311+
|} in
312+
try
313+
let _ = typecheck source in
314+
fail "expected rejection of non-pointer-returning call as RHS"
315+
with
316+
| Kernelscript.Type_checker.Type_error _ -> ()
317+
276318
(** 11. Codegen (shadowing): an outer binding of the same name as the IfLet
277319
binding must survive both branches and remain referenceable after the
278320
if. The branch-local invariant the frontend enforces (binding visible
@@ -325,6 +367,8 @@ let suite = [
325367
"codegen_scalar_value_binding", `Quick, test_codegen_scalar_value_binding;
326368
"codegen_struct_value_binding_shape", `Quick, test_codegen_struct_value_binding_shape;
327369
"codegen_shadow_outer_binding", `Quick, test_codegen_shadow_outer_binding;
370+
"reject_int_literal_rhs", `Quick, test_reject_int_literal_rhs;
371+
"reject_non_pointer_call_rhs", `Quick, test_reject_non_pointer_call_rhs;
328372
]
329373

330374
let () =

0 commit comments

Comments
 (0)