Skip to content

Commit 2b77c40

Browse files
r41k0uclaude
andcommitted
Core: Bind declared globals in local_sym_tab instead of a per-function set
A 'global x' statement is a fact about one function's scope, but it was tracked as current_func_globals on the compilation-wide context -- per-function state living at the wrong level, which had to be set before each body and reset after, and which every read and write site then had to consult in a specific order relative to local_sym_tab (the source of the parameter collision fixed in f4ec12d). The declaration now does what it means: it binds the name in local_sym_tab to the @bpfglobal's storage, as a LocalSymbol whose var is the GlobalVariable and whose declared_global flag records why. A GlobalVariable is a pointer to storage exactly as an alloca is, so every existing path that loads from or stores through a symbol's var works unchanged. That deletes the dedicated global-store branch in handle_variable_assignment, the global branch in handle_aug_assign, the allocation-pass skip, and the reset -- one table, one lookup, no order to get wrong. Reads of an undeclared global (legal in Python) still fall back to bpf_globals after local_sym_tab. Python's rules fall out naturally: a parameter is already in the table when the declarations are processed, so 'global ctx' on a parameter is detected by the same membership check. LocalSymbol.__iter__ deliberately still yields three fields, since several sites tuple-unpack a symbol. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BSDVsZH5NtoASyxB8FCtGU
1 parent f4ec12d commit 2b77c40

5 files changed

Lines changed: 39 additions & 75 deletions

File tree

pythonbpf/allocation_pass.py

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,24 +50,20 @@ def handle_assign_allocation(compilation_context, builder, stmt, local_sym_tab):
5050

5151
var_name = target.id
5252

53-
# Writes to @bpfglobal variables use the global symbol, not a stack
54-
# slot. Requires Python's own `global` declaration; without it an
55-
# assignment to a global's name would silently create a local that
56-
# shadows it, which is exactly the bug class we refuse to compile.
57-
if var_name in compilation_context.current_func_globals:
58-
logger.debug(f"'{var_name}' is a declared global, no allocation needed")
53+
# Already bound in this scope: a parameter, an earlier assignment, or a
54+
# `global` declaration (whose slot is the GlobalVariable). No slot needed.
55+
if var_name in local_sym_tab:
56+
logger.debug(f"'{var_name}' already bound, no allocation needed")
5957
continue
58+
59+
# Not declared `global`, yet named like one: in real Python this would
60+
# create a shadowing local. Refuse rather than guess which was meant.
6061
if var_name in compilation_context.bpf_globals:
6162
raise SyntaxError(
6263
f"assignment to '{var_name}' shadows the BPF global of the same "
6364
f"name — add 'global {var_name}' to write to it"
6465
)
6566

66-
# Skip if already allocated
67-
if var_name in local_sym_tab:
68-
logger.debug(f"Variable {var_name} already allocated, skipping")
69-
continue
70-
7167
# Determine type and allocate based on rval
7268
if isinstance(rval, ast.Call):
7369
_allocate_for_call(

pythonbpf/assign_pass.py

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -113,33 +113,6 @@ def handle_variable_assignment(
113113
):
114114
"""Handle single named variable assignment."""
115115

116-
# A name declared with `global` writes the @bpfglobal symbol directly:
117-
# the plain `store i64 %v, ptr @counter` form of the C reference.
118-
if (
119-
var_name not in local_sym_tab
120-
and var_name in compilation_context.current_func_globals
121-
):
122-
sym = compilation_context.bpf_globals[var_name]
123-
val_result = eval_expr(func, compilation_context, builder, rval, local_sym_tab)
124-
if val_result is None:
125-
logger.error(f"Failed to evaluate value for global {var_name}")
126-
return False
127-
val, val_type = val_result
128-
if isinstance(val_type, ir.IntType) and isinstance(sym.ir_type, ir.IntType):
129-
# Same implicit widening/truncation rules as local assignments
130-
if val_type.width < sym.ir_type.width:
131-
val = builder.sext(val, sym.ir_type)
132-
elif val_type.width > sym.ir_type.width:
133-
val = builder.trunc(val, sym.ir_type)
134-
elif val_type != sym.ir_type:
135-
logger.error(
136-
f"Type mismatch for global {var_name}: {val_type} vs {sym.ir_type}"
137-
)
138-
return False
139-
builder.store(val, sym.var)
140-
logger.info(f"Stored to BPF global {var_name}")
141-
return True
142-
143116
if var_name not in local_sym_tab:
144117
logger.error(f"Variable {var_name} not declared.")
145118
return False

pythonbpf/context.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,6 @@ def __init__(self, module: ir.Module):
6969
self.map_sym_tab: dict[str, "MapSymbol"] = {}
7070
self.bpf_globals: dict[str, "BpfGlobalSymbol"] = {}
7171

72-
# Names a `global` statement declared writable in the function whose
73-
# body is currently being emitted; managed by process_func_body.
74-
self.current_func_globals: set[str] = set()
75-
7672
# Helper management
7773
self.scratch_pool = ScratchPoolManager()
7874

@@ -86,4 +82,3 @@ def reset(self):
8682
"""Reset state between functions if necessary, though new context per compile is preferred."""
8783
self.scratch_pool.reset()
8884
self.current_func = None
89-
self.current_func_globals = set()

pythonbpf/functions/functions_pass.py

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -205,20 +205,15 @@ def handle_aug_assign(func, compilation_context, builder, stmt, local_sym_tab):
205205
"""
206206
if isinstance(stmt.target, ast.Name):
207207
name = stmt.target.id
208-
# Same resolution order as reads: local, then declared global. The two
209-
# cannot both hold a name (a parameter may not be declared global, and
210-
# an undeclared write to a global's name is refused), so this order is
211-
# about giving the same answer as _handle_name_expr, not precedence.
208+
# One table: a declared global is a local_sym_tab entry whose slot is
209+
# the GlobalVariable, so it needs no separate branch.
212210
if name in local_sym_tab:
213211
slot = local_sym_tab[name].var
214212
slot_type = local_sym_tab[name].ir_type
215213
if slot is None:
216214
raise SyntaxError(
217215
f"cannot assign to '{name}': it is the context parameter"
218216
)
219-
elif name in compilation_context.current_func_globals:
220-
sym = compilation_context.bpf_globals[name]
221-
slot, slot_type = sym.var, sym.ir_type
222217
elif name in compilation_context.bpf_globals:
223218
raise SyntaxError(
224219
f"augmented assignment to '{name}' shadows the BPF global of "
@@ -414,28 +409,6 @@ def process_func_body(
414409

415410
local_sym_tab = {}
416411

417-
# Collect `global x` declarations. Python scoping rules apply: a declared
418-
# name may be written anywhere in this function and always means the
419-
# @bpfglobal, never a local. Undeclared writes to a global name are
420-
# rejected in the allocation pass rather than silently shadowing.
421-
declared_globals: set[str] = set()
422-
param_names = {arg.arg for arg in func_node.args.args}
423-
for node in ast.walk(func_node):
424-
if isinstance(node, ast.Global):
425-
for gname in node.names:
426-
if gname in param_names:
427-
# Python's own rule and wording. Without it the read path
428-
# (local first) and the write path would resolve the same
429-
# name to different storage.
430-
raise SyntaxError(f"name '{gname}' is parameter and global")
431-
if gname not in compilation_context.bpf_globals:
432-
raise SyntaxError(
433-
f"'global {gname}' in '{func_node.name}': no @bpfglobal "
434-
f"named '{gname}' is declared"
435-
)
436-
declared_globals.add(gname)
437-
compilation_context.current_func_globals = declared_globals
438-
439412
# Add the context parameter (first function argument) to the local symbol table
440413
if func_node.args.args and len(func_node.args.args) > 0:
441414
context_arg = func_node.args.args[0]
@@ -474,6 +447,26 @@ def process_func_body(
474447
local_sym_tab[context_name] = context_type
475448
logger.info(f"Added argument '{context_name}' to local symbol table")
476449

450+
# A `global x` statement binds x in this function's scope to the
451+
# @bpfglobal's storage. It goes into local_sym_tab like any other name,
452+
# flagged, so every read and write resolves it through the one table with
453+
# no separate lookup order to get wrong. Python's rules apply: a parameter
454+
# cannot be declared global, and the name must be a @bpfglobal.
455+
for node in ast.walk(func_node):
456+
if isinstance(node, ast.Global):
457+
for gname in node.names:
458+
if gname in local_sym_tab:
459+
raise SyntaxError(f"name '{gname}' is parameter and global")
460+
if gname not in compilation_context.bpf_globals:
461+
raise SyntaxError(
462+
f"'global {gname}' in '{func_node.name}': no @bpfglobal "
463+
f"named '{gname}' is declared"
464+
)
465+
sym = compilation_context.bpf_globals[gname]
466+
local_sym_tab[gname] = LocalSymbol(
467+
sym.var, sym.ir_type, None, declared_global=True
468+
)
469+
477470
# pre-allocate dynamic variables
478471
local_sym_tab = allocate_mem(
479472
compilation_context,
@@ -500,8 +493,6 @@ def process_func_body(
500493
if not did_return:
501494
builder.ret(ir.Constant(ir.IntType(64), 0))
502495

503-
compilation_context.current_func_globals = set()
504-
505496

506497
def process_bpf_chunk(func_node, compilation_context, return_type):
507498
"""Process a single BPF chunk (function) and emit corresponding LLVM IR."""

pythonbpf/local_symbol.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,20 @@
55

66
@dataclass
77
class LocalSymbol:
8-
var: ir.AllocaInstr
8+
"""One name visible in a BPF function's scope.
9+
10+
`var` is the storage the name resolves to: an alloca for locals, the
11+
GlobalVariable for a name brought in by a `global` statement, or None for
12+
the context parameter (which arrives as func.args[0], not a slot).
13+
"""
14+
15+
var: ir.AllocaInstr | ir.GlobalVariable | None
916
ir_type: ir.Type
1017
metadata: Any = None
18+
declared_global: bool = False
1119

1220
def __iter__(self):
21+
# Three fields on purpose: several call sites tuple-unpack a symbol.
1322
yield self.var
1423
yield self.ir_type
1524
yield self.metadata

0 commit comments

Comments
 (0)