Fix register state dump keyerror for register_state_dump test - #91
Merged
Conversation
has_field() consulted hasattr() on the ctypes class while get_field_type() indexed the parsed members dict. For any member ctypes exposes but the vmlinux parser never registered, has_field() said True and get_field_type() then died with a raw KeyError. Both now consult the parsed members dict, and a lookup that misses raises a ValueError that distinguishes "no such field" from "field exists in vmlinux.py but the parser does not support it yet". get_field_index() additionally derives the index from the ctypes _fields_ list (the C declaration order) instead of the insertion order of the parsed members dict, so it no longer depends on the two happening to coincide. No change to generated IR: every vmlinux/xdp test program compiles to a byte-identical .ll. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Carries the chain of member indices from the enclosing struct down to a field, so a field that does not sit at the top level of the struct can still describe where it lives. Defaults to None, which keeps meaning "this field is a plain top-level member". Pure data, nothing reads it yet, generated IR is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
On this kernel `struct pt_regs` wraps `cs` and `ss` in anonymous unions. ctypes lifts those names onto the parent class, exactly as C does, but the parser only walked the top-level `_fields_`, so `cs` and `ss` were unknown to the compiler and `ctx.cs` blew up. Register the members of an anonymous member as fields of the parent, one level deep, taking the ABSOLUTE offset ctypes reports on the parent (no arithmetic) and recording the access path (anonymous member index, member index). The anonymous member itself stays registered as before. Deliberately conservative: only scalar ctypes members are flattened. Nested vmlinux structs (`fred_cs`, `fred_ss`) and bitfields are skipped with a warning rather than half-registered, and a flattened member never overwrites an existing top-level field of the same name. Overlapping members are kept as distinct fields, since `cs` and `csx` both legitimately live at offset 136. Flattened fields are not yet emitted: IR generation and DWARF generation skip them, and critically they do not consume a top-level field index, so existing CO-RE names are untouched. Every vmlinux/xdp test program still compiles to a byte-identical .ll. Entirely inert for any struct without `_anonymous_`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A field lifted out of an anonymous member needs an access string that walks into that member, so `struct pt_regs.cs` is `llvm.pt_regs:0:136$0:17:0` (member 0 of anonymous member 17) rather than the flat `$0:<index>` form. This is byte-for-byte what clang emits for the equivalent C, verified against `__builtin_preserve_access_index(ctx->cs)` on a struct with the same shape, and the indices and offsets agree with `bpftool btf dump file /sys/kernel/btf/vmlinux`. Only fields carrying an access path take the new path; every other field keeps the existing flat and indexed/array forms unchanged. The only .ll diff across all vmlinux/xdp test programs is four brand new globals for cs, csx, ss and ssx. No pre-existing relocation string changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…p_md Reading a context field narrower than a register was hardcoded to struct_xdp_md's i32 fields in three places that had to agree with each other: the destination alloca in allocation_pass, the zext in load_ctx_field, and the store in assign_pass. struct pt_regs' cs and ss are 2 bytes, so a second special case would have been needed. Replace all three with the general rule: a context field is loaded at its natural width and zero-extended to i64, so its destination is i64. allocation_pass gains the same context discriminator load_ctx_field uses (a context argument has no alloca of its own), so non-context field reads, which go through load_struct_field and bpf_probe_read_kernel, keep their natural width and are untouched. struct_xdp_md's generated IR is byte-identical, as is every other vmlinux/xdp test program's; the only diff in the whole corpus remains the four new globals from the previous commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…solve
The previous commits emit `llvm.pt_regs:0:136$0:17:0` for `ctx.cs`, which llc
happily turns into a CO-RE relocation, but the local BTF it is resolved
against described member 17 as a NAMED, EMPTY composite:
'_0' type_id=4 bits_offset=1088
[4] STRUCT '(anon)' size=8 vlen=0
libbpf walks a CO-RE access string by member index into the local type and
then matches by field NAME in the target type, so both halves of that break
it: index 0 into a vlen=0 composite has nothing to land on, and a local
member called `_0` cannot be paired with the kernel's anonymous one. The
object would build cleanly and then fail to load.
Emit anonymous members the way a C compiler does: unnamed, with a real
DW_TAG_union_type (or structure, for an anonymous struct) carrying their
members in declaration order. The emitted BTF now reads
'(anon)' type_id=4 bits_offset=1088
[4] UNION '(anon)' size=8 vlen=3
'cs' ... 'csx' ... 'fred_cs' ...
which is the same shape clang produces for the equivalent C and the same
shape `bpftool btf dump file /sys/kernel/btf/vmlinux` reports.
Gated on the struct declaring `_anonymous_`, and it bails back to the old
opaque member for anything it cannot describe faithfully. Across every
vmlinux/xdp test program the set of emitted globals, every relocation string
and every instruction are unchanged; the only diffs are the new DWARF nodes
and the DWARF node renumbering they cause, in the two pt_regs programs.
This is the one part of the fix that cannot be verified without loading a
program, so it is kept as its own commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…calar Style only, no behaviour change and no IR change. Keeps `ruff check --select SIM` clean on the files this branch touches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.