Feat/integer signedness by r41k0u · Pull Request #101 · pythonbpf/Python-BPF

r41k0u · 2026-09-18T03:33:33Z

Fix the inconsistent zext-sext integer signedness and expansion/contraction.

Eight cases, each reading operands from .bss globals (so clang cannot fold) and writing its result to a .bss global (so the runtime proof is a bpftool map dump). The IR clang emits is the specification for the signedness work: s64 = u32 zext i32 -> i64 (widening is source-driven) u64 = s32 sext i32 -> i64 u32 / s32 udiv i32 (usual arithmetic conversions) u64 > s64 icmp ugt i64 u32 * u32 -> u64 mul i32, then zext (wraps at 32 before widening) u32 >> 4 lshr i32 s32 >> 4 ashr i32 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU

LLVM integer types are sign-agnostic; the sign lives in the operations. The frontend therefore has to carry it, and until now it did not: c_uint32 became a bare i32 at declaration and every later choice (sext, sdiv, icmp_signed) assumed signed. This commit adds the pieces without changing any emitted IR: - type_deducer.IntTy, an ir.IntType subclass carrying 'signed'. It renders, compares and hashes as the plain type, so the 40 isinstance checks and every == ir.IntType(64) comparison keep working; signedness() reads it and treats a plain IntType as signed (today's behaviour). Constructed outside IntType's per-width instance cache, which is shared with subclasses and would have merged the two signs of a width into one object. - ctypes_to_ir now returns IntTy, which types five declaration kinds at once: locals from ctypes constructors, struct fields, globals, context annotations, and map key/value types. is_signed_ctype replaces the private copy in globals_pass. - operators.usual_arithmetic_conversions: C's rule for the type a binary operation is performed in. - type_normalization.convert (source-driven zext/sext when widening, trunc when narrowing) and canonicalise (bring a value to the working width holding exactly a given type's value). Nothing consults the sign yet. The corpus of 67 compilable test programs produces byte-identical .ll before and after. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU

The same widen-to-i64 / truncate-to-slot logic was written out by hand in six places: local assignment, struct-field assignment, augmented assignment, helper-argument temporaries, binary-operation operands, and the comparison normaliser. Each now calls type_normalization.convert, which is the single point where the sign decides between zext and sext. Across the 67-program corpus this changes exactly one emitted instruction, and it is a correction rather than a regression: augmented assignment on a c_uint32 struct field now widens the field's current value with zext instead of sext, because that slot's descriptor already carries its sign from ctypes_to_ir. Every other site still sees plain (signed-by-default) descriptors and is byte-identical. The remaining hand-written widenings -- the printk formatter, the vmlinux field loader, return handling -- have their own rules and are migrated together with the sign sources. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU

Sign now originates wherever a type is declared or inferred, and emitted IR changes only where a value is widened out of a type now known to be unsigned: - Helper return types are typed per the kernel signature: ktime_get_ns, get_current_pid_tgid, get_current_uid_gid, get_current_cgroup_id, get_prandom_u32 and get_smp_processor_id are unsigned; the probe_read family, perf_event_output and get_stack return long. - Undeclared locals are 64-bit with the sign of their initializer -- from the helper registry for helper results, and for binary operations from a small static inference (expr/type_inference.py) applying the same usual arithmetic conversions the code generator will. - Integer literals are typed as C types them (int if it fits, else long long) while remaining 64-bit constants; this rank is what makes u32 / -2 promote to an unsigned 32-bit division. - signedness() understands vmlinux Field descriptors, and load_ctx_field sign-extends signed sub-64-bit context fields instead of always zero-extending them. - printk widening goes through convert. Two sites decided whether to emit a conversion from descriptor *equality*. With literal descriptors now narrower than the constants that carry them, that skipped required truncations (storing an i64 into an i32 slot). Assignment and the ctypes cast handler now always run convert, which is a no-op when the physical widths already agree. Corpus: the following programs change, every changed line a sext->zext swap on a value of known unsigned type: failing_tests/xdp/xdp_test_1.py.ll passing_tests/assign/augassign_struct_field.py.ll passing_tests/helpers/smp_processor_id.py.ll Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU

…ions Arithmetic is now typed bottom-up through the expression tree the way C does it. get_typed_operand evaluates an operand to (value, IntTy); a binary node computes its type with usual_arithmetic_conversions, converts each operand to that type per the operand's own sign (source-driven, folding literals), runs the operation in the i64 working register, and narrows the result back to the node's type. That last step is what makes a u32 * u32 wrap at 32 bits before it is widened into a u64, exactly as clang emits (mul i32, zext) -- verified against tests/c-form/signedness.bpf.c. There is no expression-wide signed or unsigned mode; each node decides from its operands, and the assignment target only converts the finished result. Augmented assignment is typed identically (x op= v is x = x op v), unary minus negates in the operand's promoted type (2^N - x for unsigned), and return statements now convert the value to the declared return type instead of emitting whatever width the expression happened to have. Corpus: 13 programs change; with SSA numbering normalised every changed line is a trunc/sext/zext inserted by canonicalisation where an operation's type is narrower than 64 bits (int-typed literals and c_int32/c_uint32 operands). No program changes compile status. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU

…moted type The operator table now carries both variants for the sign-sensitive operations: / and // -> sdiv/udiv, % -> srem/urem, >> -> ashr/lshr. The ring operations are unchanged since two's complement makes their low bits sign-blind. apply_binop picks the variant from the promoted type computed by the usual arithmetic conversions, so uint32(10) / int32(-2) is a udiv in u32 (0, as C) rather than a signed division (-5). Comparisons between two integers now go through the same promotion and pick icmp_signed or icmp_unsigned from the promoted type, so uint64(10) > int64(-1) is icmp ugt (false, as C). Pointer and struct comparisons keep the depth-normalising path unchanged. Corpus: only an XDP test comparing c_uint context fields changes, to an unsigned predicate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU

-2 parses as USub(Constant 2), so u32 / -2 reached the divider as a mul-by-minus-one instruction rather than the constant clang folds it to. Fold it at the unary operator; the result is a literal like any other, with a literal's C rank, so the usual arithmetic conversions still make u32 / -2 an unsigned division by 0xFFFFFFFE. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU

A comparison's descriptor was a plain i1, which reads as signed, so storing or returning a truth value would sign-extend true to -1. C's relational operators yield int 0 or 1; give the results an unsigned descriptor so any widening zero-extends. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU

Seven programs under passing_tests/signedness, one per case in tests/c-form/signedness.bpf.c: unsigned and signed widening, mixed division and remainder, unsigned comparison, u32 * u32 wrapping before the widening, right shifts, and literal rank. Levels 1 and 2 compile them like any other case; test_signedness_ir.py asserts the shape of the IR against what clang emits for the C (zext vs sext, udiv vs sdiv, icmp ugt vs sgt, lshr vs ashr, the trunc/zext pair around the wrap). The XDP xfail note no longer claims the guard is a signed compare. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU

C's rules over the ctypes types: what a declaration means, literal rank, source-driven widening with the destination typing the stored value, the usual arithmetic conversions applied per operation, which operators and predicates the sign selects, and the list of deliberate divergences from Python (/ and // truncate, % takes the dividend's sign, fixed width, unsigned types exist). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU

…ness

A pointer used where a value is expected is dereferenced to it, with a null check, in helper arguments, printk arguments, binary-operation operands and comparisons, all through deref_to_depth. return was the one consumer that never joined that convention, so `return p` on a map lookup emitted `ret i64*` and left it to llc to reject. handle_return now goes through get_typed_operand, which is that path, and converts the result to the declared return type like the augmented-assignment store does. convert() no longer passes a non-integer through silently: every caller either checks both sides or is on an integer-only path, so what reaches that branch is a type error, and it is raised with both types named instead of surfacing from llc with an IR line number. Returning a struct value is the case that now fails at compile time. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

An enum constant has type int in C whatever the enum's underlying type is; clang emits `sub nsw i32` + sext for `XDP_PASS - k` and reserves the enum's u32 for variables of the enum type. The vmlinux handler was typing constants as i64, which made `XDP_PASS - k` with an unsigned k a signed 64-bit operation instead of C's u32 one. The constant's C rank is now decided once, in type_deducer.int_literal_type, shared by literals and enum constants, and the handler returns it as the descriptor so both operand typing and local inference read it from the source. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

return/map_value.py is the C shape `if (p) return *p;`; return_struct.py is the type error convert() now raises; vmlinux/enum_rank.py pins the u32 trunc/zext pair for `XDP_PASS - k`. test_signedness_ir.py takes paths under passing_tests so it can assert on all three, and skips the vmlinux case when vmlinux.py is not importable, as the harness does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

… zero A bool is ir.IntType(1), so convert() handled it as a 1-bit integer: a bool slot's plain descriptor read as signed and True sign-extended to -1 on the way out, and an integer narrowed to a bool by truncation, so 2 became false. Both predate this branch. C widens a bool to 0 or 1 and narrows to it with != 0, and Python's truthiness agrees on the second. signedness() now answers unsigned for any 1-bit integer whatever it is wrapped in, convert() narrows to width 1 with icmp ne 0, and a bool constant's slot carries the unsigned descriptor like the comparison results already do. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved moderate issues remain in signedness propagation, conversions, binding checks, and literal typing.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 6

Open (6)

Preserve sub-register field width in descriptors · New
Use declared map value signedness after dereference · New
Propagate unsignedness in augmented assignments · New
Preserve helper return signedness in expression descriptors · New
Retain source descriptor for helper argument conversion · New
Pass expression signedness to integer argument preparation · New

What changed in this PR

Adds C-like integer signedness, width conversion, and arithmetic handling across PythonBPF, with tests and documentation.

Changes:

Adds signed/unsigned integer descriptors and conversion rules.
Updates expression, assignment, helper, vmlinux, global, and return handling.
Adds signedness tests, reference C cases, failure coverage, and documentation.

File	Summary
`tests/test_signedness_ir.py`	Validates generated signedness-related IR.
`tests/test_config.toml`	Registers signedness test expectations.
`tests/passing_tests/vmlinux/enum_rank.py`	Tests enum literal ranking.
`tests/passing_tests/signedness/widen_unsigned.py`	Tests unsigned widening.
`tests/passing_tests/signedness/widen_signed.py`	Tests signed widening.
`tests/passing_tests/signedness/unsigned_compare.py`	Tests signed and unsigned comparisons.
`tests/passing_tests/signedness/right_shift.py`	Tests arithmetic and logical shifts.
`tests/passing_tests/signedness/narrow_wrap.py`	Tests narrow-width wrapping.
`tests/passing_tests/signedness/mixed_division.py`	Tests mixed-sign division and remainder.
`tests/passing_tests/signedness/literal_rank.py`	Tests integer literal ranking.
`tests/passing_tests/signedness/bool_int.py`	Tests boolean/integer conversions.
`tests/passing_tests/return/map_value.py`	Tests map-value returns.
`tests/failing_tests/return_struct.py`	Tests invalid struct returns.
`tests/c-form/signedness.bpf.c`	Provides the C reference implementation.
`pythonbpf/vmlinux_parser/vmlinux_exports_handler.py`	Handles signed context-field widening. Moderate issue (1 vote): signedness is lost when assigning fields to locals.
`pythonbpf/type_deducer.py`	Adds signed integer descriptors and literal typing. Moderate issue (1 vote): hexadecimal/octal literal rules are not preserved.
`pythonbpf/helper/printk_formatter.py`	Converts integer `bpf_printk` arguments. Moderate issue (4 votes): expression signedness is discarded before formatting.
`pythonbpf/helper/helper_utils.py`	Converts helper arguments. Moderate issue (4 votes): source signedness is discarded during conversion.
`pythonbpf/helper/bpf_helper_handler.py`	Adds signedness metadata to helper results. Moderate issue (4 votes): direct helper emitters still return signed descriptors.
`pythonbpf/globals_pass.py`	Reuses signedness metadata for globals.
`pythonbpf/functions/functions_pass.py`	Applies typed operations and returns. Moderate issue (4 votes): unsigned augmented operations use signed division, remainder, and right shift.
`pythonbpf/expr/type_normalization.py`	Implements integer conversion and normalization.
`pythonbpf/expr/type_inference.py`	Infers expression integer types.
`pythonbpf/expr/operators.py`	Adds C arithmetic conversions and signed operators.
`pythonbpf/expr/expr_pass.py`	Propagates typed operands. Moderate issues: field signedness/width can be lost (1–2 votes), map value signedness can be lost (3 votes), and binding checks are omitted for typed names (1 vote).
`pythonbpf/expr/__init__.py`	Exports expression helpers.
`pythonbpf/assign_pass.py`	Applies typed assignment conversion.
`pythonbpf/allocation_pass.py`	Infers signed-aware local allocations.
`docs/user-guide/integers.md`	Documents integer semantics.
`docs/user-guide/index.md`	Links integer documentation.
`docs/index.md`	Adds integer documentation to navigation.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    if isinstance(ty, ir.IntType):
+        return IntTy(ty.width, signedness(ty))
+    if val is not None and isinstance(val.type, ir.IntType):
+        return IntTy(val.type.width, signedness(ty))


+                if depth == 1
+                else deref_to_depth(func, builder, var, depth)
+            )
+            return val, _descriptor(val, sym.ir_type if depth == 1 else base_type)


+    result = canonicalise(
+        builder, apply_binop(builder, stmt.op, current, rhs), result_ty
+    )


    "ktime",
    param_types=[],
-    return_type=ir.IntType(64),
+    return_type=IntTy(64, False),


+        if expected_type is not None and isinstance(expected_type, ir.IntType):
+            val = convert(builder, val, val.type, expected_type)
        builder.store(val, ptr)


+def _handle_int_arg(val, builder, ty=None):
+    """Widen an integer for bpf_printk to i64, per the value's sign."""
+    return convert(builder, val, ty if ty is not None else val.type, ir.IntType(64))


r41k0u · 2026-09-23T12:28:24Z

+    if methods is None:
        raise SyntaxError(f"Unsupported binary operation: {type(op).__name__}")
-    return getattr(builder, method)(left, right)
+    return getattr(builder, methods[0] if signed else methods[1])(left, right)


This is fine for now, but if BINOP_METHODS are referenced at many other places in the future then we need to use a NamedTuple instead.

r41k0u · 2026-09-23T14:58:36Z

-            # A @bpfglobal: plain load off the global symbol.
-            return builder.load(compilation_context.bpf_globals[operand.id].var)
+            sym = compilation_context.bpf_globals[operand.id]
+            return builder.load(sym.var), _descriptor(None, sym.ir_type)


Is sending a None here fine? can we not just send the actual val?

r41k0u · 2026-09-23T15:01:55Z

+    """Extract the value from an operand, handling variables and constants."""
+    return get_typed_operand(
+        func, compilation_context, operand, builder, local_sym_tab
+    )[0]


Yeah, I don't like this. Might work for now. But we should either use NamedTuples or just hide get_typed_operand as _get_typed_operand.

r41k0u · 2026-09-24T11:15:27Z

+        value = (sym.params or {}).get("value") if sym else None
+        return (
+            _as_intty(ctypes_to_ir(value))
+            if isinstance(value, str) and is_ctypes(value)


Assumes that value can only be a named var, which is wrong

r41k0u · 2026-09-24T11:17:26Z

+    descriptors (see type_deducer.IntTy); the physical width comes from the
+    value itself, which may already be wider than its descriptor says.
+    """
+    if not (isinstance(to_ty, ir.IntType) and isinstance(val.type, ir.IntType)):


What about bools?

The registry types ktime, pid, uid, random, cgroup_id and smp_processor_id as unsigned, and undeclared locals initialised from them already took that sign. The expression value did not: each emitter returns (value, plain i64), so ktime() >> 1 was an ashr and pid() // 3 an sdiv. The dispatcher now substitutes the registered descriptor when the widths agree, in one place instead of in every emitter. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Two descriptors were built from the physical value instead of the declaration. A sub-register vmlinux field is widened to i64 by load_ctx_field, and its descriptor took that width, so ctx.data - k with a c_int32 k was a 64-bit subtraction where C makes it a u32 one. A map-lookup local dereferenced past its slot took the physical pointee (i64, signed by default) and dropped the map's declared value ctype kept in the symbol's metadata, so p >> 63 on a c_uint64 value was an ashr. Both now read the declaration; the physical width still comes from the value, as everywhere else. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

- A global's descriptor is built from the loaded value, not from None. - get_operand_value, the [0] wrapper over get_typed_operand, had no callers left; removed with its export. - infer_int_type's map branch names the map's declared value type for what it is (value_ctype), says that the key argument plays no part, and applies only to lookup: update and delete return a status, not a value of the map's type. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The binary-operation path chooses sdiv/udiv, srem/urem and ashr/lshr from the promoted type; the augmented-assignment path computed the same promoted type and then called apply_binop without it, so u32 >>= 4 was an ashr and u32 //= 3 an sdiv. Same call as the binop path now. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

… field rank One IR-shape case per fix: u32 >>= / //= / %= emit lshr/udiv/urem; ktime() >> 1 and pid() // 3 emit lshr/udiv; p >> 63 on a c_uint64 map value is lshr; ctx.data - c_int32 is a u32 subtraction with the trunc/zext pair rather than a 64-bit one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

xdp_md.data is a u32 in C, but the verifier tracks it as a packet pointer and rejects 32-bit arithmetic on it: "R0 32-bit pointer arithmetic prohibited", the same for a C program, which is why C casts it through (void *)(long) first. The u32 ranking the test pins is the correct one; it just has to be pinned on a field that is a scalar to the verifier too. ingress_ifindex is. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

xdp_md.data/data_end/data_meta and __sk_buff.data/data_end are u32 in C and packet pointers to the verifier, so arithmetic on them directly is rejected as 32-bit pointer arithmetic, for C and for us alike. A TODO at the point where a field's rank is decided describes the planned fix (64-bit pointer rank for those fields, no cast needed), and the user guide documents the workaround until then: copy to a local or cast through c_void_p. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

r41k0u · 2026-09-24T14:28:23Z

LGTM, need to fix the ctx.data thing later.

r41k0u · 2026-09-24T14:28:34Z

@varun-r-mallya pls remember

r41k0u and others added 12 commits September 17, 2026 20:34

Merge branch 'feat/global-variables' into feat/integer-signedness

0273a67

Merge remote-tracking branch 'origin/master' into feat/integer-signed…

5a75c16

…ness

r41k0u requested a review from varun-r-mallya September 18, 2026 03:33

r41k0u and others added 6 commits September 18, 2026 17:46

Merge remote-tracking branch 'origin/master' into feat/integer-signed…

359ddbd

…ness

Tests: Bool widening and narrowing

78ec75a

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings September 24, 2026 11:23

Copilot started reviewing on behalf of r41k0u September 24, 2026 11:23 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

r41k0u commented Sep 24, 2026

View reviewed changes

r41k0u and others added 7 commits September 24, 2026 17:09

Merge branch 'master' into feat/integer-signedness

c52a5f5

r41k0u merged commit dc628ee into master Sep 24, 2026
2 checks passed

Navigation Menu

Repository navigation

Feat/integer signedness - #101

Feat/integer signedness#101
r41k0u merged 26 commits into
masterfrom
feat/integer-signedness

r41k0u commented Sep 18, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

r41k0u Sep 23, 2026

Uh oh!

r41k0u Sep 23, 2026

Uh oh!

r41k0u Sep 23, 2026

Uh oh!

r41k0u Sep 24, 2026

Uh oh!

r41k0u Sep 24, 2026

Uh oh!

r41k0u commented Sep 24, 2026

Uh oh!

r41k0u commented Sep 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

r41k0u commented Sep 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

r41k0u Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

r41k0u Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

r41k0u Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

r41k0u Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

r41k0u Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

r41k0u commented Sep 24, 2026

Uh oh!

r41k0u commented Sep 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants