diff --git a/Makefile b/Makefile index 7874b44e..5e3be67b 100644 --- a/Makefile +++ b/Makefile @@ -10,12 +10,13 @@ test: pytest tests/ -W ignore::DeprecationWarning -v --tb=short -m "not verifier" test-cov: - pytest tests/ -v --tb=short -m "not verifier" \ + pytest tests/ -W ignore::DeprecationWarning -v --tb=short -m "not verifier" \ --cov=pythonbpf --cov-report=term-missing --cov-report=html test-verifier: - @echo "NOTE: verifier tests require sudo and bpftool. Uses sudo .venv/bin/python3." - pytest tests/test_verifier.py -v --tb=short -m verifier + @echo "NOTE: verifier tests shell out to 'sudo bpftool'; run 'sudo -v' first so" + @echo " the timestamp does not lapse mid-run. bpftool must be installed." + pytest tests/test_verifier.py -W ignore::DeprecationWarning -v --tb=short -m verifier all: clean install diff --git a/pythonbpf/maps/maps_pass.py b/pythonbpf/maps/maps_pass.py index 083b24a6..ca078454 100644 --- a/pythonbpf/maps/maps_pass.py +++ b/pythonbpf/maps/maps_pass.py @@ -175,10 +175,15 @@ def process_bpf_map(func_node, compilation_context): if isinstance(rval, ast.Call) and isinstance(rval.func, ast.Name): handler = MapProcessorRegistry.get_processor(rval.func.id) - if handler: - return handler(map_name, rval, compilation_context) - else: - logger.warning(f"Unknown map type {rval.func.id}, defaulting to HashMap") - return process_hash_map(map_name, rval, compilation_context) + if handler is None: + # Raise an exception and fail the build because the + # map carried the wrong type. A misspelled map type + # is a program error. + known = ", ".join(sorted(MapProcessorRegistry.known_types())) + raise ValueError( + f"Unknown map type '{rval.func.id}' returned by '{map_name}'. " + f"Known map types: {known}" + ) + return handler(map_name, rval, compilation_context) else: raise ValueError("Function under @map must return a map") diff --git a/pythonbpf/maps/maps_utils.py b/pythonbpf/maps/maps_utils.py index 194b408e..a271697c 100644 --- a/pythonbpf/maps/maps_utils.py +++ b/pythonbpf/maps/maps_utils.py @@ -33,3 +33,8 @@ def decorator(func): def get_processor(cls, map_type_name): """Get the processor function for a map type""" return cls._processors.get(map_type_name) + + @classmethod + def known_types(cls): + """Names of every registered map type, for error messages""" + return list(cls._processors.keys()) diff --git a/tests/README.md b/tests/README.md index 2861f4f3..6b63fd45 100644 --- a/tests/README.md +++ b/tests/README.md @@ -69,8 +69,16 @@ Known-broken tests are declared in `tests/test_config.toml`: "failing_tests/my_test.py" = {reason = "...", level = "ir"} ``` -- `level = "ir"` — fails during IR generation; both IR and LLC tests are marked xfail. -- `level = "llc"` — IR generates fine but `llc` rejects it; only the LLC test is marked xfail. +- `level = "ir"` — fails during IR generation. +- `level = "llc"` — IR generates fine but `llc` rejects it. +- `level = "verifier"` — IR and `llc` both succeed, but the kernel verifier rejects the object. + +A failure at one level implies failure at every later one, so the declared level marks +that level **and all later ones** xfail. An `"ir"` entry is xfail at all three levels; a +`"verifier"` entry is xfail at level 3 only and must still pass levels 1 and 2. + +Every test file runs at every level, including the ones declared here — level 3 does not +skip declared failures, it reports them as expected ones. All xfails use `strict = True`: if a test starts **passing** it shows up as **XPASS** and is treated as a test failure. This is intentional — it means the bug was fixed and the test should be promoted to `passing_tests/`. diff --git a/tests/conftest.py b/tests/conftest.py index ce92d1dd..42ab30ed 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -19,6 +19,7 @@ import pytest +from tests.framework.bpf_test_case import level_index from tests.framework.collector import collect_all_test_files # ── vmlinux availability ──────────────────────────────────────────────────── @@ -70,14 +71,19 @@ def pytest_collection_modifyitems(items): # xfail (strict: XPASS counts as a test failure, alerting us to fixed bugs) if case.is_expected_fail: - # Level "ir" → fails at IR generation: xfail both IR and LLC tests - # Level "llc" → IR succeeds but LLC fails: only xfail the LLC test - is_llc_test = item.nodeid.startswith("tests/test_llc_compilation.py") - - apply_xfail = (case.xfail_level == "ir") or ( - case.xfail_level == "llc" and is_llc_test - ) - if apply_xfail: + # A failure at one level implies failure at every later one, so mark + # this item xfail whenever the declared level is at or before it: + # "ir" → IR, LLC and verifier + # "llc" → LLC and verifier (IR is expected to succeed) + # "verifier" → verifier only + if item.nodeid.startswith("tests/test_verifier.py"): + item_level = "verifier" + elif item.nodeid.startswith("tests/test_llc_compilation.py"): + item_level = "llc" + else: + item_level = "ir" + + if level_index(case.xfail_level) <= level_index(item_level): item.add_marker( pytest.mark.xfail( reason=case.xfail_reason, diff --git a/tests/framework/bpf_test_case.py b/tests/framework/bpf_test_case.py index d80a7134..a993166e 100644 --- a/tests/framework/bpf_test_case.py +++ b/tests/framework/bpf_test_case.py @@ -1,6 +1,20 @@ from dataclasses import dataclass from pathlib import Path +# The three test levels, in pipeline order. A test declared as failing at one +# level is also expected to fail at every later level: a program that cannot +# generate IR cannot reach llc, and one that llc rejects never reaches the +# kernel. Used by conftest to decide which items to mark xfail. +LEVELS = ("ir", "llc", "verifier") + + +def level_index(level: str) -> int: + """Position of a level in the pipeline. Unknown levels sort first ("ir").""" + try: + return LEVELS.index(level) + except ValueError: + return 0 + @dataclass class BpfTestCase: @@ -8,7 +22,7 @@ class BpfTestCase: rel_path: str is_expected_fail: bool = False xfail_reason: str = "" - xfail_level: str = "ir" # "ir" or "llc" + xfail_level: str = "ir" # one of LEVELS needs_vmlinux: bool = False skip_reason: str = "" diff --git a/tests/failing_tests/if.py b/tests/passing_tests/if.py similarity index 84% rename from tests/failing_tests/if.py rename to tests/passing_tests/if.py index 638c2ce3..8949d253 100644 --- a/tests/failing_tests/if.py +++ b/tests/passing_tests/if.py @@ -3,7 +3,7 @@ @bpf -@section("sometag1") +@section("tracepoint/syscalls/sys_enter_execve") def sometag(ctx: c_void_p) -> c_int64: if 3 + 2 == 5: return c_int64(5) diff --git a/tests/passing_tests/return.py b/tests/passing_tests/return.py index 9bd048b6..67a0bb05 100644 --- a/tests/passing_tests/return.py +++ b/tests/passing_tests/return.py @@ -3,7 +3,7 @@ @bpf -@section("sometag1") +@section("tracepoint/syscalls/sys_enter_execve") def sometag(ctx: c_void_p) -> c_int64: return c_int64(1 - 1) diff --git a/tests/passing_tests/ringbuf.py b/tests/passing_tests/ringbuf.py index 0566d855..05652cf1 100644 --- a/tests/passing_tests/ringbuf.py +++ b/tests/passing_tests/ringbuf.py @@ -1,13 +1,13 @@ from pythonbpf import bpf, BPF, map, bpfglobal, section, compile, compile_to_ir -from pythonbpf.maps import RingBuf, HashMap +from pythonbpf.maps import RingBuffer, HashMap from ctypes import c_int32, c_void_p # Define a map @bpf @map -def mymap() -> RingBuf: - return RingBuf(max_entries=(1024)) +def mymap() -> RingBuffer: + return RingBuffer(max_entries=4096) @bpf diff --git a/tests/passing_tests/var_rval.py b/tests/passing_tests/var_rval.py index ee1735e7..0742c040 100644 --- a/tests/passing_tests/var_rval.py +++ b/tests/passing_tests/var_rval.py @@ -5,7 +5,7 @@ @bpf -@section("sometag1") +@section("tracepoint/syscalls/sys_enter_execve") def sometag(ctx: c_void_p) -> c_int64: a = 1 - 1 return c_int64(a) diff --git a/tests/test_config.toml b/tests/test_config.toml index edcd90ce..8a6255a3 100644 --- a/tests/test_config.toml +++ b/tests/test_config.toml @@ -2,15 +2,27 @@ # # [xfail] — tests expected to fail. # key = path relative to tests/ -# value = {reason = "...", level = "ir" | "llc"} -# level "ir" = fails during pythonbpf IR generation (exception or ERROR log) -# level "llc" = IR generates but llc rejects it +# value = {reason = "...", level = "ir" | "llc" | "verifier"} +# level "ir" = fails during pythonbpf IR generation (exception or ERROR log) +# level "llc" = IR generates but llc rejects it +# level "verifier" = IR and llc both succeed, but the kernel verifier rejects it +# +# A failure at one level implies failure at every later one, so the declared +# level marks that level and all later ones xfail. # [xfail] "failing_tests/conditionals/struct_ptr.py" = {reason = "Struct pointer used directly as boolean condition not supported", level = "ir"} +# Compiles cleanly; rejected by the kernel. The `data + 34 < data_end` guard is +# emitted as a signed compare over values round-tripped through the stack, so the +# verifier never narrows the packet range (it stays r=0) and the later +# `iph.saddr` read fails with "invalid access to packet, off=26 size=4" / +# "R1 offset is outside of the packet". Direct packet access needs bounds checks +# in a form the verifier can follow. +"failing_tests/xdp/xdp_test_1.py" = {reason = "XDP direct packet access: the data/data_end guard does not establish a packet range the verifier can use", level = "verifier"} + "failing_tests/license.py" = {reason = "Missing LICENSE global produces IR that llc rejects — should be caught earlier with a clear error message", level = "llc"} "failing_tests/undeclared_values.py" = {reason = "Undeclared variable used in f-string — should raise SyntaxError (correct behaviour, test documents it)", level = "ir"} diff --git a/tests/test_verifier.py b/tests/test_verifier.py index 413ef453..3966e3f6 100644 --- a/tests/test_verifier.py +++ b/tests/test_verifier.py @@ -24,12 +24,12 @@ from tests.framework.verifier import verify_object -def _passing_test_files(): - return [c.path for c in collect_all_test_files() if not c.is_expected_fail] +def _verifier_test_files(): + return [c.path for c in collect_all_test_files()] -def _passing_test_ids(): - return [c.rel_path for c in collect_all_test_files() if not c.is_expected_fail] +def _verifier_test_ids(): + return [c.rel_path for c in collect_all_test_files()] def _get_rejection_reason(verifier_test_file: Path, output) -> str: @@ -43,11 +43,16 @@ def _get_rejection_reason(verifier_test_file: Path, output) -> str: return errstr +# Every test file runs at this level, including the ones declared in +# test_config.toml. conftest marks those xfail, so an "ir"- or "llc"-level +# failure is still reported as an expected failure rather than being silently +# dropped from the level-3 run — and a "verifier"-level entry becomes possible +# at all. @pytest.mark.verifier @pytest.mark.parametrize( "verifier_test_file", - _passing_test_files(), - ids=_passing_test_ids(), + _verifier_test_files(), + ids=_verifier_test_ids(), ) def test_kernel_verifier(verifier_test_file: Path, tmp_path, caplog): """Compile the BPF test and verify it passes the kernel verifier."""