Merge pull request #99 from pythonbpf/feat/global-variables

Add the dev workflow skill, implement support for all global vars using @bpfglobal (both const and non-const)

Author: r41k0u

.claude/skills/ir-first-feature/SKILL.md

@@ -0,0 +1,103 @@
+---
+name: ir-first-feature
+description: PythonBPF's development loop for implementing a new compiler feature — write a minimal C eBPF reference, compile it to LLVM IR and read that as the spec, stop for a human syntax decision, then implement against the reference. Use whenever adding or extending a PythonBPF language feature (new statement/expression support, map types, globals, helpers, program constructs).
+---
+
+# The IR-first feature loop
+
+PythonBPF targets LLVM IR via llvmlite. For any new feature, clang's output for the
+equivalent C is the specification — not documentation, not intuition. Follow the loop
+in order; do not skip steps because the feature "looks simple".
+
+## 1. Write the C reference
+
+A minimal `.bpf.c` in `tests/c-form/` exercising **only** the target feature. Small
+enough that every line of the resulting IR is attributable to the feature. Prefer no
+includes (define `SEC` and the `__u*` typedefs by hand) so nothing else pollutes the IR.
+Cover each variant of the feature in one file (e.g. for globals: zero-init, initialized,
+const, const volatile).
+
+## 2. Compile and read the IR — this is the spec
+
+```bash
+clang -target bpf -O2 -g -emit-llvm -S feature.bpf.c -o feature.ll
+llc -march=bpf -filetype=obj feature.ll -o feature.o
+bpftool btf dump file feature.o        # what must come out the far end
+```
+
+Read `feature.ll` and answer, in writing: What top-level symbols/globals appear? What
+do loads/stores/calls look like in the body? What `!DI*` debug metadata exists, and
+what BTF does llc manufacture from it? What did -O2 fold away, and does that folding
+carry semantics (it did for `const` globals)?
+
+Version discipline: llvmlite ≥0.49 emits LLVM 21/22-era attribute spellings
+(`captures(none)`, not `nocapture`). Use a clang/llc generation that accepts them, and
+compare against what `pythonbpf` + the CI's LLVM actually use.
+
+## 3. Diff against current PythonBPF output
+
+Compile the nearest thing PythonBPF can already express and diff the `.ll`s. The delta
+is the actual work item — often smaller than expected (machinery like section placement
+and BTF generation frequently comes free from llc).
+
+## 4. HARD STOP — syntax is a human decision
+
+Present 2–3 Pythonic syntax candidates with trade-offs (declaration site, usage site,
+failure modes, precedents from FastAPI/typing/Triton-style DSLs). **Wait for a human to
+choose. Never proceed on your own judgment, and never treat silence as consent.** The
+maintainers own the language surface.
+
+## 5. Implement against the reference
+
+Emit IR through the existing passes (`globals_pass`, `expr_pass`, `assign_pass`,
+`allocation_pass`, `debuginfo/`). Verify by **diffing your emitted `.ll` against the
+clang reference for the same shapes** — "it compiles and llc accepts it" is not the
+bar; llc accepts plenty of subtly wrong IR.
+
+### House style: lower, don't desugar
+
+Handlers walk the AST the user wrote and emit IR directly. **Do not synthesize new AST
+nodes mid-compilation and feed them back through other handlers** — no
+`ast.Assign(ast.BinOp(...))` conjured to make `x += v` reuse the assignment path.
+
+The temptation is legitimate, so know the argument you are declining. Desugaring is a
+standard compiler move (CPython itself lowers `x += v` this way), it guarantees semantic
+agreement with the composed form, it is the smallest diff, and any later fix to the
+composed path applies automatically. Those are real benefits.
+
+They lose in this codebase for structural reasons: the passes communicate through the
+source tree. Allocation runs before codegen and walks `Assign` — a synthetic `Assign`
+created during codegen is invisible to it, so the two passes silently disagree about
+what the function contains (it happened to be harmless for augmented assignment only
+because that statement never needs a fresh slot; that is luck, not design). Synthetic
+nodes carry no source location, so diagnostics point nowhere. And `ast.dump` in the logs
+shows statements the user never typed, which turns every debugging session into an
+archaeology exercise.
+
+The resolution is to move sharing down a level: **equivalence should come from shared
+value-level helpers, not shared AST.** When two constructs must agree, extract the common
+logic into a helper both call — the way binary-op evaluation and augmented assignment
+both use `apply_binop` for the operator table and `get_operand_value` for operands —
+and let each handler resolve its own target and emit its own store. Two handlers calling
+one helper is the idiom; one handler manufacturing input for another is not.
+
+The operator tables themselves — binary operators, comparisons, and the supported
+unary/boolean operators — live in exactly one place, `expr/operators.py`. A new operator
+is added there first; if it is not in that file, the compiler does not support it.
+
+## 6. Test at the right tier
+
+- Works now → `tests/passing_tests/<category>/`.
+- Documents a gap → `tests/kernel_selftest_equivalent/` with a strict xfail in
+  `tests/test_config.toml` (level `"ir"`, `"llc"`, or `"verifier"`).
+- Wrong-input behaviour → `tests/failing_tests/` with a config entry.
+- Kernel verifier level runs in CI; locally it needs the user's sudo — ask, don't
+  assume.
+
+## House guardrails (always)
+
+- **Never read/cat/grep `vmlinux.py` or `vmlinux.h`** — generated, enormous, will
+  exhaust context. Probe with one-liners:
+  `.venv/bin/python -c "import vmlinux; print(vmlinux.struct_x._fields_[:3])"`
+- Ask the dev which Python binary to use, and remember that for future. The original authors use `.venv/bin/python` as their system python has no llvmlite.
+- Atomic commits, `Core:`/`Tests:` subject prefixes, one logical change each.

pythonbpf/allocation_pass.py

@@ -2,7 +2,7 @@
 import logging
 import ctypes
 from llvmlite import ir
-from .local_symbol import LocalSymbol
+from .symbols import LocalSymbol
 from pythonbpf.helper import HelperHandlerRegistry
 from pythonbpf.vmlinux_parser.dependency_node import Field
 from .expr import VmlinuxHandlerRegistry
@@ -49,11 +49,23 @@ def handle_assign_allocation(compilation_context, builder, stmt, local_sym_tab):
             continue
 
         var_name = target.id
-        # Skip if already allocated
+
+        # Already bound in this scope: a parameter, an earlier assignment, or a
+        # `global` declaration (whose slot is the GlobalVariable). No slot needed.
         if var_name in local_sym_tab:
-            logger.debug(f"Variable {var_name} already allocated, skipping")
+            logger.debug(f"'{var_name}' already bound, no allocation needed")
             continue
 
+        # Not declared `global`, yet named like one: Python creates a local
+        # that shadows the global for the whole function body, and leaves the
+        # global untouched. Do the same.
+        shadows_global = var_name in compilation_context.bpf_globals
+        if shadows_global:
+            logger.info(
+                f"'{var_name}' is assigned without a 'global' declaration, so it "
+                f"is a local shadowing the @bpfglobal of the same name"
+            )
+
         # Determine type and allocate based on rval
         if isinstance(rval, ast.Call):
             _allocate_for_call(
@@ -65,7 +77,9 @@ def handle_assign_allocation(compilation_context, builder, stmt, local_sym_tab):
             _allocate_for_binop(builder, var_name, local_sym_tab)
         elif isinstance(rval, ast.Name):
             # Variable-to-variable assignment (b = a)
-            _allocate_for_name(builder, var_name, rval, local_sym_tab)
+            _allocate_for_name(
+                builder, var_name, rval, local_sym_tab, compilation_context
+            )
         elif isinstance(rval, ast.Attribute):
             # Struct field-to-variable assignment (a = dat.fld)
             _allocate_for_attribute(
@@ -76,6 +90,14 @@ def handle_assign_allocation(compilation_context, builder, stmt, local_sym_tab):
                 f"Unsupported assignment value type for {var_name}: {type(rval).__name__}"
             )
 
+        if shadows_global and var_name in local_sym_tab:
+            # Where the binding ends, so that a read above it is reported the
+            # way Python reports it. end_lineno, not lineno, so a read on a
+            # continuation line of a multi-line binding counts as above it too.
+            local_sym_tab[var_name].shadows_global_from = (
+                getattr(stmt, "end_lineno", None) or target.lineno
+            )
+
 
 def _allocate_for_call(builder, var_name, rval, local_sym_tab, compilation_context):
     """Allocate memory for variable assigned from a call."""
@@ -298,21 +320,24 @@ def allocate_temp_pool(builder, max_temps, local_sym_tab):
             logger.debug(f"Allocated temp variable: {temp_name}")
 
 
-def _allocate_for_name(builder, var_name, rval, local_sym_tab):
+def _allocate_for_name(builder, var_name, rval, local_sym_tab, compilation_context):
     """Allocate memory for variable-to-variable assignment (b = a)."""
     source_var = rval.id
 
-    if source_var not in local_sym_tab:
+    # Local first, then a BPF global: the copy takes the source's type either
+    # way (a c_uint32 global gives a c_uint32 local).
+    if source_var in local_sym_tab:
+        source_symbol = local_sym_tab[source_var]
+    elif source_var in compilation_context.bpf_globals:
+        source_symbol = compilation_context.bpf_globals[source_var]
+    else:
         logger.error(f"Source variable '{source_var}' not found in symbol table")
         return
 
-    # Get type and metadata from source variable
-    source_symbol = local_sym_tab[source_var]
-
     # Allocate with same type and alignment
     var = _allocate_with_type(builder, var_name, source_symbol.ir_type)
     local_sym_tab[var_name] = LocalSymbol(
-        var, source_symbol.ir_type, source_symbol.metadata
+        var, source_symbol.ir_type, getattr(source_symbol, "metadata", None)
     )
 
     logger.info(

pythonbpf/assign_pass.py

@@ -54,6 +54,14 @@ def handle_struct_field_assignment(
         logger.info(f"Copied string to char array {var_name}.{field_name}")
         return
 
+    # Same implicit widening/truncation as assignment to a local: expressions
+    # evaluate in i64, but a field may be narrower.
+    if isinstance(val_type, ir.IntType) and isinstance(field_type, ir.IntType):
+        if val_type.width < field_type.width:
+            val = builder.sext(val, field_type)
+        elif val_type.width > field_type.width:
+            val = builder.trunc(val, field_type)
+
     # Regular assignment
     builder.store(val, field_ptr)
     logger.info(f"Assigned to struct field {var_name}.{field_name}")

pythonbpf/context.py

@@ -5,6 +5,7 @@
 if TYPE_CHECKING:
     from pythonbpf.structs.struct_type import StructType
     from pythonbpf.maps.maps_utils import MapSymbol
+    from pythonbpf.symbols import BpfGlobalSymbol
 
 logger = logging.getLogger(__name__)
 
@@ -66,6 +67,7 @@ def __init__(self, module: ir.Module):
         self.global_sym_tab: list[ir.GlobalVariable] = []
         self.structs_sym_tab: dict[str, "StructType"] = {}
         self.map_sym_tab: dict[str, "MapSymbol"] = {}
+        self.bpf_globals: dict[str, "BpfGlobalSymbol"] = {}
 
         # Helper management
         self.scratch_pool = ScratchPoolManager()

pythonbpf/expr/__init__.py

@@ -1,6 +1,7 @@
 from .expr_pass import eval_expr, handle_expr, get_operand_value
 from .type_normalization import convert_to_bool, get_base_type_and_depth
 from .ir_ops import deref_to_depth, access_struct_field
+from .operators import apply_binop
 from .call_registry import CallHandlerRegistry
 from .vmlinux_registry import VmlinuxHandlerRegistry
 
@@ -10,6 +11,7 @@
     "convert_to_bool",
     "get_base_type_and_depth",
     "deref_to_depth",
+    "apply_binop",
     "access_struct_field",
     "get_operand_value",
     "CallHandlerRegistry",

pythonbpf/expr/expr_pass.py

@@ -7,6 +7,7 @@
 from pythonbpf.type_deducer import ctypes_to_ir, is_ctypes
 from .call_registry import CallHandlerRegistry
 from .ir_ops import deref_to_depth, access_struct_field
+from .operators import apply_binop, UNARY_OPS, BOOL_OPS
 from .type_normalization import (
     convert_to_bool,
     handle_comparator,
@@ -22,12 +23,22 @@
 # ============================================================================
 
 
-def _handle_name_expr(expr: ast.Name, local_sym_tab: Dict, builder: ir.IRBuilder):
+def _handle_name_expr(
+    expr: ast.Name, compilation_context, local_sym_tab: Dict, builder: ir.IRBuilder
+):
     """Handle ast.Name expressions."""
     if expr.id in local_sym_tab:
-        var = local_sym_tab[expr.id].var
-        val = builder.load(var)
-        return val, local_sym_tab[expr.id].ir_type
+        sym = local_sym_tab[expr.id]
+        # A local that shadows a @bpfglobal is unreadable above its binding,
+        # exactly as in Python; for every other local this is a no-op.
+        sym.check_bound_at(expr.id, expr.lineno)
+        val = builder.load(sym.var)
+        return val, sym.ir_type
+    elif expr.id in compilation_context.bpf_globals:
+        # A @bpfglobal
+        sym = compilation_context.bpf_globals[expr.id]
+        val = builder.load(sym.var)
+        return val, sym.ir_type
     else:
         # Check if it's a vmlinux enum/constant
         vmlinux_result = VmlinuxHandlerRegistry.handle_name(expr.id)
@@ -78,7 +89,9 @@ def _handle_attribute_expr(
             var_ptr, var_type, var_metadata = local_sym_tab[var_name]
             logger.info(f"Loading attribute {attr_name} from variable {var_name}")
             logger.info(
-                f"Variable type: {var_type}, Variable ptr: {var_ptr}, Variable Metadata: {var_metadata}"
+                f"Variable type: {var_type}, Variable ptr: {
+                    var_ptr
+                }, Variable Metadata: {var_metadata}"
             )
             if (
                 hasattr(var_metadata, "__module__")
@@ -96,7 +109,9 @@ def _handle_attribute_expr(
 
             elif isinstance(var_metadata, Field):
                 logger.error(
-                    f"Cannot access field '{attr_name}' on already-loaded field value '{var_name}'"
+                    f"Cannot access field '{attr_name}' on already-loaded field value '{
+                        var_name
+                    }'"
                 )
                 return None
 
@@ -175,6 +190,9 @@ def get_operand_value(func, compilation_context, operand, builder, local_sym_tab
             else:
                 val = deref_to_depth(func, builder, var, depth)
             return val
+        elif operand.id in compilation_context.bpf_globals:
+            # A @bpfglobal: plain load off the global symbol.
+            return builder.load(compilation_context.bpf_globals[operand.id].var)
         else:
             # Check if it's a vmlinux enum/constant
             vmlinux_result = VmlinuxHandlerRegistry.handle_name(operand.id)
@@ -223,25 +241,7 @@ def _handle_binary_op_impl(func, compilation_context, rval, builder, local_sym_t
         right = builder.sext(right, ir.IntType(64))
 
     # Map AST operation nodes to LLVM IR builder methods
-    op_map = {
-        ast.Add: builder.add,
-        ast.Sub: builder.sub,
-        ast.Mult: builder.mul,
-        ast.Div: builder.sdiv,
-        ast.Mod: builder.srem,
-        ast.LShift: builder.shl,
-        ast.RShift: builder.lshr,
-        ast.BitOr: builder.or_,
-        ast.BitXor: builder.xor,
-        ast.BitAnd: builder.and_,
-        ast.FloorDiv: builder.udiv,
-    }
-
-    if type(op) in op_map:
-        result = op_map[type(op)](left, right)
-        return result
-    else:
-        raise SyntaxError("Unsupported binary operation")
+    return apply_binop(builder, op, left, right)
 
 
 def _handle_binary_op(
@@ -304,7 +304,9 @@ def _handle_ctypes_call(
         # Get the IR type from the value itself
         actual_ir_type = value.type
         logger.info(
-            f"Converting vmlinux field {val_type.name} (IR type: {actual_ir_type}) to {call_type}"
+            f"Converting vmlinux field {val_type.name} (IR type: {actual_ir_type}) to {
+                call_type
+            }"
         )
     else:
         actual_ir_type = val_type
@@ -317,7 +319,9 @@ def _handle_ctypes_call(
             if actual_ir_type.width < expected_type.width:
                 value = builder.sext(value, expected_type)
                 logger.info(
-                    f"Sign-extended from i{actual_ir_type.width} to i{expected_type.width}"
+                    f"Sign-extended from i{actual_ir_type.width} to i{
+                        expected_type.width
+                    }"
                 )
             elif actual_ir_type.width > expected_type.width:
                 value = builder.trunc(value, expected_type)
@@ -329,7 +333,9 @@ def _handle_ctypes_call(
                 pass
         else:
             raise ValueError(
-                f"Type mismatch: expected {expected_type}, got {actual_ir_type} (original type: {val_type})"
+                f"Type mismatch: expected {expected_type}, got {
+                    actual_ir_type
+                } (original type: {val_type})"
             )
 
     return value, expected_type
@@ -373,7 +379,7 @@ def _handle_unary_op(
     local_sym_tab,
 ):
     """Handle ast.UnaryOp expressions."""
-    if not isinstance(expr.op, ast.Not) and not isinstance(expr.op, ast.USub):
+    if not isinstance(expr.op, UNARY_OPS):
         logger.error("Only 'not' and '-' unary operators are supported")
         return None
 
@@ -516,6 +522,9 @@ def _handle_boolean_op(
 ):
     """Handle `and` and `or` boolean operations."""
 
+    if not isinstance(expr.op, BOOL_OPS):
+        logger.error(f"Unsupported boolean operator: {type(expr.op).__name__}")
+        return None
     if isinstance(expr.op, ast.And):
         return _handle_and_op(func, builder, expr, local_sym_tab, compilation_context)
     elif isinstance(expr.op, ast.Or):
@@ -662,7 +671,7 @@ def eval_expr(
 
     logger.info(f"Evaluating expression: {ast.dump(expr)}")
     if isinstance(expr, ast.Name):
-        return _handle_name_expr(expr, local_sym_tab, builder)
+        return _handle_name_expr(expr, compilation_context, local_sym_tab, builder)
     elif isinstance(expr, ast.Constant):
         return _handle_constant_expr(compilation_context, builder, expr)
     elif isinstance(expr, ast.Call):