Skip to content

Commit 32c1aff

Browse files
r41k0uclaude
andcommitted
Tests: Make the integer properties cover every type on every run
Review of the first version of the suite found four holes. - The conversion sites returned the converted value, which shows its bits but not the type it was stored as. The untyped_local site passed over a real bug that way. Each site now reads the value back through an expression whose result depends on its sign and width. - With the type drawn at random, 25 examples per site left a single type with about 3. A deliberately broken c_bool conversion went undetected under some seeds. The conversion property is now parametrized by destination type and the operator property by left operand type, a sixteenth of the profile's examples each. - The global_init site was a strict xfail. Hypothesis shrank its expected failures, which took 115 of the property's 187 seconds. Its two bugs are now pinned in test_known_gaps.py, and the site steps around them. - Gaps are now stepped around by drawing only inputs that avoid them, not by assume(). Filtering tripped Hypothesis's filter_too_much health check. untyped-copy-keeps-narrow-type is renamed untyped-copy-keeps-source-type. `t = a` keeps a's type for any source that is not 64-bit, c_uint32 and c_int32 included, not only narrow ones. Under the ci profile, each of these deliberate compiler bugs fails a property: - >> always logical - % always unsigned - / always unsigned - equal-width mixed signs going signed - promotion keeping an unsigned sign - widening always by zero-extension - c_bool conversion by truncation Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 7b328fb commit 32c1aff

5 files changed

Lines changed: 154 additions & 78 deletions

File tree

‎tests/README.md‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ programs are generated, and each one is compiled two ways and run.
154154
| File | Property |
155155
|---|---|
156156
| `test_int_expressions.py` | Random straight-line programs over every ctypes integer type and `c_bool`: `+ - * / // % << >> & \| ^`, comparisons as values, unary `-`, `not`, constructor casts, typed and untyped temporaries, nested. The worked examples of `integers.md` are pinned with `@example`. |
157-
| `test_int_operators.py` | Every binary operator on every pair of operand types, values drawn edge-first. Random trees rarely reach a negative dividend under a signed `%` and return it undisturbed; this property always does. |
158-
| `test_int_conversions.py` | Every (source type, destination type, site) triple, where a site is each place a conversion happens: constructor call, declaration, reassignment, untyped local, global store, `return`, global initialiser, and each augmented assignment. |
157+
| `test_int_operators.py` | Every binary operator, parametrized by the left operand's type, with the right type and both values drawn (edge values first). Random trees rarely reach a negative dividend under a signed `%` and return it undisturbed; this property always does. |
158+
| `test_int_conversions.py` | Every conversion site, parametrized by the destination type: constructor call, declaration, reassignment, untyped local, global store, `return`, global initialiser, and each augmented assignment. The converted value is read back through an expression whose result depends on its sign and width, not just returned, so a value stored with the right bits but the wrong type is caught too. |
159159
| `test_known_gaps.py` | Each known divergence's smallest program, as a strict xfail. |
160160

161161
### Known gaps
@@ -164,8 +164,10 @@ programs are generated, and each one is compiled two ways and run.
164164
had fixed. Each has a predicate, so the properties step around it and keep
165165
finding *new* bugs, and a pinned program that `test_known_gaps.py` runs as a
166166
strict xfail. A fix makes that test XPASS, failing the run until the gap is
167-
deleted, at which point the properties cover that ground again. A failing
168-
conversion site is a strict xfail on its parameter instead.
167+
deleted, at which point the properties cover that ground again. The
168+
operator and conversion properties step around a gap by drawing only what
169+
avoids it (not by filtering, which Hypothesis rightly complains about), with
170+
a comment naming the gap.
169171

170172
### Running
171173

@@ -175,9 +177,10 @@ HYPOTHESIS_PROFILE=thorough pytest tests/property # 3000 examples, for bug hunt
175177
```
176178

177179
CI uses the `ci` profile (100 examples, derandomized, so a failure always
178-
reproduces), about 90 seconds. The operator and conversion properties run a
179-
quarter of the profile's count per parameter. Profiles are registered in
180-
`conftest.py`. A failure prints both programs and the model's value;
180+
reproduces): about two and a half minutes on a 16-core machine. The
181+
operator and conversion properties are parametrized by type, so every type
182+
is tested on every run, and run a sixteenth of the profile's count per
183+
parameter. Profiles are registered in `conftest.py`. A failure prints both programs and the model's value;
181184
Hypothesis shrinks it to the smallest program that still fails.
182185

183186
### Plan

‎tests/property/known_gaps.py‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,16 @@ class Gap:
104104
),
105105
),
106106
Gap(
107-
"untyped-copy-keeps-narrow-type",
108-
"`t = a` gives t a's own type, so a c_bool/c_uint8/c_uint16 copy stays "
109-
"narrow and promotes to int; integers.md says an undeclared local is "
110-
"always 64 bits with the expression's sign (unsigned here). Code or "
111-
"doc: a decision (pythonbpf/allocation_pass.py, _allocate_for_name)",
112-
lambda p: any(isinstance(e, Var) and e.ty.bits < 32 for _, _, e in p.temps),
107+
"untyped-copy-keeps-source-type",
108+
"`t = a` gives t a's own type, whatever it is: a c_uint32 copy stays "
109+
"u32 and a c_int8 copy stays narrow and promotes to int, while "
110+
"integers.md says an undeclared local is always 64 bits with the "
111+
"expression's sign. Only a 64-bit source is unaffected. Code or doc: "
112+
"a decision (pythonbpf/allocation_pass.py, _allocate_for_name)",
113+
lambda p: any(
114+
isinstance(e, Var) and e.ty != untyped_local_type(e.ty)
115+
for _, _, e in p.temps
116+
),
113117
# C, by integers.md: -128 + (u64)1 is a u64, so 0 < it holds. PythonBPF:
114118
# t1 stays a u8, the sum is the int -127, and the compare is false.
115119
ExpressionProgram(

‎tests/property/test_int_conversions.py‎

Lines changed: 109 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"""
1212

1313
import pytest
14-
from hypothesis import assume, given, settings
14+
from hypothesis import given, settings
1515
from hypothesis import strategies as st
1616

1717
from tests.property.cints import (
@@ -29,8 +29,11 @@
2929
from tests.property.harness import CLANG, check_against_clang
3030
from tests.property.programs import (
3131
INT64_MAX,
32-
INT64_MIN,
32+
Bin,
33+
Cast,
34+
Lit,
3335
Program,
36+
Var,
3437
as_literal,
3538
input_decl,
3639
literal_source,
@@ -39,70 +42,100 @@
3942

4043
pytestmark = pytest.mark.skipif(CLANG is None, reason="clang is not installed")
4144

42-
TYPES = st.sampled_from(ALL_TYPES)
45+
46+
# How the converted value is read back. `return x` alone shows the value but
47+
# not the type it was stored as, and a later use depends on that type: these
48+
# read it through an operation whose result changes with its sign and width.
49+
PROBES = {
50+
"value": lambda x: x,
51+
# -1 in x's type (promoted), halved: 0 if signed, and u32 and u64 differ
52+
"sign_and_width": lambda x: Bin("/", Bin("-", Bin("*", x, Lit(0)), Lit(1)), Lit(2)),
53+
# INT_MAX + 1 wraps if x's type promotes to a 32-bit int, not if 64-bit
54+
"int_overflow": lambda x: Bin(
55+
"+", Bin("+", Bin("*", x, Lit(0)), Lit(2**31 - 1)), Lit(1)
56+
),
57+
}
4358

4459

45-
def _site_program(site, src, dst, value, other):
60+
def _site_program(site, src, dst, value, other, probe):
4661
"""(Program, model) for converting `a0`, a `src` holding `value`, to `dst`
47-
at `site`. `other` is a second value, used where a site needs one."""
62+
at `site`, then returning probe(the converted value). `other` is a second
63+
value, used where a site needs one."""
4864
decl_py, decl_c = input_decl("a0", src, value)
49-
as_i64 = lambda v: convert(v, C_INT64) # noqa: E731 (prog returns c_int64)
65+
66+
def read_back(name, ty, stored, py, c, globals=()):
67+
"""Finish a program whose statements stored `stored` into `name`."""
68+
expr = probe(Var(name, ty))
69+
program = Program(
70+
py + [f"return {expr.py()}"],
71+
c + [f"return {expr.c()};"],
72+
globals=list(globals),
73+
)
74+
return program, lambda: convert(expr.eval({name: stored()}), C_INT64)
5075

5176
if site == "constructor":
77+
expr = probe(Cast(dst, Var("a0", src)))
5278
return Program(
53-
[decl_py, f"return {dst.name}(a0)"],
54-
[decl_c, f"return ({dst.c_name})a0;"],
55-
), lambda: as_i64(convert(value, dst))
79+
[decl_py, f"return {expr.py()}"], [decl_c, f"return {expr.c()};"]
80+
), lambda: convert(expr.eval({"a0": value}), C_INT64)
5681
if site == "declare":
57-
return Program(
58-
[decl_py, f"x = {dst.name}(a0)", "return x"],
59-
[decl_c, f"{dst.c_name} x = a0;", "return x;"],
60-
), lambda: as_i64(convert(value, dst))
82+
return read_back(
83+
"x",
84+
dst,
85+
lambda: convert(value, dst),
86+
[decl_py, f"x = {dst.name}(a0)"],
87+
[decl_c, f"{dst.c_name} x = a0;"],
88+
)
6189
if site == "reassign":
62-
return Program(
63-
[decl_py, f"x = {dst.name}(0)", "x = a0", "return x"],
64-
[decl_c, f"{dst.c_name} x = 0;", "x = a0;", "return x;"],
65-
), lambda: as_i64(convert(value, dst))
90+
return read_back(
91+
"x",
92+
dst,
93+
lambda: convert(value, dst),
94+
[decl_py, f"x = {dst.name}(0)", "x = a0"],
95+
[decl_c, f"{dst.c_name} x = 0;", "x = a0;"],
96+
)
6697
if site == "untyped_local":
6798
# dst plays no part: an undeclared local is 64 bits with a0's sign
6899
local = untyped_local_type(src)
69-
return Program(
70-
[decl_py, "x = a0", "return x"],
71-
[decl_c, f"{local.c_name} x = a0;", "return x;"],
72-
), lambda: as_i64(convert(value, local))
100+
return read_back(
101+
"x",
102+
local,
103+
lambda: convert(value, local),
104+
[decl_py, "x = a0"],
105+
[decl_c, f"{local.c_name} x = a0;"],
106+
)
73107
if site == "global_store":
74-
return Program(
75-
[decl_py, "global out", "out = a0", "return out"],
76-
[decl_c, "out = a0;", "return out;"],
108+
return read_back(
109+
"out",
110+
dst,
111+
lambda: convert(value, dst),
112+
[decl_py, "global out", "out = a0"],
113+
[decl_c, "out = a0;"],
77114
globals=[("out", dst, 0)],
78-
), lambda: as_i64(convert(value, dst))
115+
)
79116
if site == "return":
117+
# Read through the function's own return type, so no probe.
80118
return Program([decl_py, "return a0"], [decl_c, "return a0;"], ret=dst), (
81119
lambda: convert(value, dst)
82120
)
83121
if site == "global_init":
84122
# `value` is used as the initialiser literal itself
85123
lit = as_literal(value, src)
86-
return Program(
87-
["return g"], ["return g;"], globals=[("g", dst, lit)]
88-
), lambda: as_i64(convert(lit, dst))
124+
return read_back("g", dst, lambda: convert(lit, dst), [], [], [("g", dst, lit)])
89125
if site.startswith("augassign"):
90126
op = site.split(" ")[1]
91127
init = as_literal(other, dst)
92-
return Program(
93-
[
94-
decl_py,
95-
f"x = {dst.name}({literal_source(init)})",
96-
f"x {op}= a0",
97-
"return x",
98-
],
128+
return read_back(
129+
"x",
130+
dst,
131+
lambda: convert(binary(op, convert(init, dst), dst, value, src), dst),
132+
[decl_py, f"x = {dst.name}({literal_source(init)})", f"x {op}= a0"],
99133
[
100134
decl_c,
101135
f"{dst.c_name} x = {literal_source(init)};",
102136
f"x {'/' if op == '//' else op}= a0;",
103-
"return x;",
104137
],
105-
), lambda: as_i64(convert(binary(op, convert(init, dst), dst, value, src), dst))
138+
)
106139
raise ValueError(site)
107140

108141

@@ -113,33 +146,48 @@ def _site_program(site, src, dst, value, other):
113146
"untyped_local",
114147
"global_store",
115148
"return",
116-
pytest.param(
117-
"global_init",
118-
marks=pytest.mark.xfail(
119-
strict=True,
120-
raises=AssertionError,
121-
reason="a @bpfglobal initialiser that is not a bare constant (as "
122-
"c_int64(-5) is not: -5 is a UnaryOp) is emitted as 0, and a "
123-
"constant is not converted to the type (c_bool(2) keeps the low "
124-
"bit, 0, where C gives 1) (pythonbpf/globals_pass.py, _emit_global)",
125-
),
126-
),
149+
"global_init",
127150
] + [f"augassign {op}" for op in ARITH_OPS + DIV_OPS + SHIFT_OPS]
128151

129152

130-
# A quarter of the profile's examples per site: there are 18 sites, and each
131-
# example is cheap to state but costs a clang run (~40 ms).
153+
# Parametrized over the destination type as well as the site, so every type
154+
# gets examples on every run: drawn at random, a type-specific rule (such as
155+
# c_bool's `!= 0`) went untested in some runs. 162 parameters, so a
156+
# sixteenth of the profile's examples each.
157+
@pytest.mark.parametrize("dst", ALL_TYPES, ids=lambda t: t.name)
132158
@pytest.mark.parametrize("site", SITES)
133-
@settings(max_examples=max(10, settings.default.max_examples // 4))
134-
@given(src=TYPES, dst=TYPES, data=st.data())
135-
def test_conversion_matches_clang(site, src, dst, data):
136-
value = data.draw(values_of(src), label="value")
137-
other = data.draw(values_of(dst), label="other")
138-
if site == "global_init":
139-
value = data.draw(st.integers(INT64_MIN, INT64_MAX), label="initialiser")
159+
@settings(max_examples=max(5, settings.default.max_examples // 16))
160+
@given(data=st.data())
161+
def test_conversion_matches_clang(site, dst, data):
162+
# Known gaps are stepped around by drawing only what avoids them, not by
163+
# filtering (test_known_gaps.py pins each one):
164+
sources = ALL_TYPES
165+
if site in ("augassign <<", "augassign >>"):
166+
# shift-result-type: `x >>= a` is typed by both operands, so only a
167+
# source that leaves the type at x's own promoted type
168+
sources = [t for t in ALL_TYPES if common_type(dst, t) == promote(dst)]
169+
if site == "untyped_local":
170+
# untyped-copy-keeps-source-type: `x = a0` keeps a0's type, which is
171+
# right only for a 64-bit a0 (and dst plays no part here)
172+
sources = [t for t in ALL_TYPES if untyped_local_type(t) == t]
173+
if site == "global_init" and dst.is_bool:
174+
pytest.skip("known gap global-initialiser: c_bool(2) keeps its low bit")
175+
src = data.draw(st.sampled_from(sources), label="src")
176+
140177
if site in ("augassign <<", "augassign >>"):
141-
# known gap shift-result-type: `x >>= a` is typed by both operands
142-
# (test_known_gaps.py pins this case)
143-
assume(promote(dst) == common_type(dst, src))
144-
program, expected = _site_program(site, src, dst, value, other)
178+
# a count out of range is undefined in C
179+
top = min(promote(dst).bits - 1, src.max)
180+
value = data.draw(st.integers(0, top), label="count")
181+
elif site == "global_init":
182+
# global-initialiser: a negative initialiser is emitted as 0. `value`
183+
# is the initialiser literal itself, out-of-range ones included.
184+
value = data.draw(st.integers(0, INT64_MAX), label="initialiser")
185+
else:
186+
value = data.draw(values_of(src), label="value")
187+
other = data.draw(values_of(dst), label="other")
188+
# global_init's gap is the value itself, which the type probes multiply
189+
# away (global_store reads a global's type through them)
190+
probes = ["value"] if site == "global_init" else sorted(PROBES)
191+
probe = data.draw(st.sampled_from(probes), label="probe")
192+
program, expected = _site_program(site, src, dst, value, other, PROBES[probe])
145193
check_against_clang(program, expected)

‎tests/property/test_int_operators.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,13 @@
3232
TYPES = st.sampled_from(ALL_TYPES)
3333

3434

35-
# A quarter of the profile's examples per operator, as there are 17 of them.
35+
# Parametrized over the left operand's type as well as the operator, so every
36+
# type gets examples on every run (153 parameters, a sixteenth of the
37+
# profile's examples each).
38+
@pytest.mark.parametrize("left_ty", ALL_TYPES, ids=lambda t: t.name)
3639
@pytest.mark.parametrize("op", ARITH_OPS + DIV_OPS + SHIFT_OPS + COMPARE_OPS)
37-
@settings(max_examples=max(10, settings.default.max_examples // 4))
38-
@given(left_ty=TYPES, right_ty=TYPES, data=st.data())
40+
@settings(max_examples=max(5, settings.default.max_examples // 16))
41+
@given(right_ty=TYPES, data=st.data())
3942
def test_binary_operator_matches_clang(op, left_ty, right_ty, data):
4043
left = data.draw(values_of(left_ty), label="left")
4144
if op in SHIFT_OPS:

‎tests/property/test_known_gaps.py‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import pytest
88

9-
from tests.property.cints import C_INT64, convert
9+
from tests.property.cints import C_BOOL, C_INT64, convert
1010
from tests.property.harness import CLANG, check_against_clang
1111
from tests.property.known_gaps import GAPS
1212
from tests.property.programs import Program
@@ -44,3 +44,21 @@ def test_known_gap_augassign_shift():
4444
["unsigned int a0 = 1;", "int x = -2147483648;", "x >>= a0;", "return x;"],
4545
)
4646
check_against_clang(program, lambda: convert(-(2**31) >> 1, C_INT64))
47+
48+
49+
GLOBAL_INIT_REASON = (
50+
"global-initialiser: a @bpfglobal initialiser that is not a bare constant "
51+
"(c_int64(-5) is not: -5 is a UnaryOp) is emitted as 0, and a constant is "
52+
"not converted to the type, so c_bool(2) keeps the low bit, 0, where C "
53+
"gives 1 (pythonbpf/globals_pass.py, _emit_global). "
54+
"test_int_conversions' global_init site steps around both."
55+
)
56+
57+
58+
@pytest.mark.xfail(strict=True, raises=AssertionError, reason=GLOBAL_INIT_REASON)
59+
@pytest.mark.parametrize(
60+
"ty, literal", [(C_INT64, -5), (C_BOOL, 2)], ids=["negative", "c_bool"]
61+
)
62+
def test_known_gap_global_initialiser(ty, literal):
63+
program = Program(["return g"], ["return g;"], globals=[("g", ty, literal)])
64+
check_against_clang(program, lambda: convert(convert(literal, ty), C_INT64))

0 commit comments

Comments
 (0)