Skip to content

Reject word sizes struct cannot express - #364

Open
zardus wants to merge 2 commits into
masterfrom
feature/fix-word-width
Open

Reject word sizes struct cannot express#364
zardus wants to merge 2 commits into
masterfrom
feature/fix-word-width

Conversation

@zardus

@zardus zardus commented Aug 9, 2026

Copy link
Copy Markdown
Member

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Problem

Arch.struct_fmt() returns the format character Z for a three-byte word, and struct has no such character, so every caller that uses the string it hands back raises. It fires on the 14 of pypcode's 187 sleigh languages whose word is 24 bits — the PIC, dsPIC, HCS-12 and extended AVR families — and on any explicit struct_fmt(size=3):

ArchPcode('HCS-12:BE:24:default')  bits=24 bytes=3
    .struct_fmt()                          -> '>Z'
    struct.calcsize('>Z')                  -> error: bad char in struct format
    struct.unpack('>Z', b'\x01\x02\x03')   -> error: bad char in struct format

The exception surfaces at the caller's struct call, naming a format string the caller never wrote, rather than at the architecture that produced it.

Root cause

archinfo/arch.py maps size 3 to a character struct does not define:

elif size == 3:
    fmt_size = "Z"  # special case for 24-bit architectures like AVR8

struct has integer format characters for 1, 2, 4 and 8 bytes and nothing for 3, so no character can be correct here. The else branch of the same chain already raises for 5, 6, 7 and 16; three bytes was the one invalid size that returned a value instead of raising.

Fix

Delete the size == 3 branch so a three-byte word falls into the existing else:

ArchPcode('HCS-12:BE:24:default')  bits=24 bytes=3
    .struct_fmt()                          -> ValueError: Invalid size: struct has no format character for a 3-byte integer

The message names the size rather than claiming the argument must be a power of two, which the old text asserted and which this function never required. A caller that has to read a 24-bit word assembles it from bytes; struct_fmt cannot do that for them, and the call is where saying so belongs. The docstring records the constraint so the next reader does not put a character back.

Testing

tests/test_struct_fmt.py asserts that struct.calcsize(arch.struct_fmt(size=n)) equals n for every size struct can express, signed and unsigned, and that 0, 3, 5, 6, 7 and 16 raise. tests/test_pcode.py::test_struct_fmt_24bit pins the p-code case. Both fail on master, where the 24-bit call returns '>Z' and calcsize rejects it.

angr/cle#721 is the other half of this. Validation: #364 (comment)

sync: angr/cle#721

session: sharpen

@zardus

zardus commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Validation record for head 02d70fa9910ff5aa521fc186be057f96b5827c88 against baseline bf85c7e47bb469878c564480e28c677abbe35acc.

  • Reproducer: python -c "import archinfo, struct; print(archinfo.ArchPcode('HCS-12:BE:24:default').struct_fmt())" — prints >Z on baseline, and struct.calcsize('>Z') then raises bad char in struct format; raises ValueError on head
  • Regression: python -m pytest tests/test_pcode.py::TestArchPcode::test_struct_fmt_24bit tests/test_struct_fmt.py — both fail on baseline (struct_fmt() returns a format character instead of raising), pass on head
  • Full suite: python -m pytest tests/ — 20 passed
  • Lint/type: pylint and pyright scored per changed file against the merge base, the way the hosted jobs score them — archinfo/arch.py 9.70 -> 9.70, tests/test_pcode.py 10.00 -> 10.00, tests/test_struct_fmt.py new file 10.00; pyright badness 0.2227 -> 0.2222, 0.0 -> 0.0, 0.0 -> 0.0
  • Pre-commit: pre-commit run --all-files — every hook passes, no rewrites
  • Workspace gate: archinfo and cle suites, hooks and workspace checks under Python 3.12 — exit 0 twice on this exact state

Scope: 14 of the 183 p-code languages pypcode 4.0.1.dev0 exposes report a three-byte word — HCS-12, HCS12, HCS-12X, PIC-18, PIC-24E/F/H, dsPIC30F, dsPIC33C/E/F and the three extended AVR languages. All returned Z on baseline, regardless of input. It surfaced in a corpus sweep running CFGFast, out of Clemory.unpack_word (angr/cle#721).

Caveats: test_struct_fmt_24bit lives in TestArchPcode, which skips without pypcode — an optional extra here, so the Windows and macOS uv run pytest jobs skip it. tests/test_struct_fmt.py covers the same contract on ArchAMD64 and runs everywhere.

Re-keyed 2026-08-28. The figures above were measured at 6faa1ee404c78c908317aac60868f35955b2ea02 on baseline da171ca0dd8eec1e16dd9c4f04576b2c958e2aea, which is the head the opening line named until now; the branch is at 02d70fa9910ff5aa521fc186be057f96b5827c88 on bf85c7e47bb469878c564480e28c677abbe35acc. git range-diff da171ca0dd8eec1e16dd9c4f04576b2c958e2aea..6faa1ee404c78c908317aac60868f35955b2ea02 bf85c7e47bb469878c564480e28c677abbe35acc..02d70fa9910ff5aa521fc186be057f96b5827c88 reports every commit unchanged and git diff 6faa1ee404c78c908317aac60868f35955b2ea02 02d70fa9910ff5aa521fc186be057f96b5827c88 differs only by master's own advance (4 files changed, 24 insertions(+), 7 deletions(-)). Master did touch tests/test_pcode.py between the two baselines, so the regression was re-run at this head rather than assumed: python -m pytest tests/test_pcode.py::TestArchPcode::test_struct_fmt_24bit tests/test_struct_fmt.py — 3 passed at 02d70fa; with archinfo/arch.py alone reverted to bf85c7e47bb469878c564480e28c677abbe35acc the two named tests fail, so the regression still depends on the production change at the new baseline. archinfo/arch.py, which every other figure is about, master left alone.

@angr-bot

angr-bot commented Aug 9, 2026

Copy link
Copy Markdown
Member

Corpus decompilation diffs can be found at angr/dec-snapshots@master...angr/archinfo_364

@zardus

zardus commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Arch.struct_fmt() on the three-byte word of a 24-bit p-code architecture, and on every size ArchAMD64 is asked for, before and after this change.

Before — the format string comes back with a character struct does not define, and the error surfaces at the caller's struct call:

archinfo master
archinfo: archinfo at master f92307b

ArchPcode('HCS-12:BE:24:default')  bits=24 bytes=3
    .struct_fmt()                          -> '>Z'
    struct.calcsize('>Z')                  -> error: bad char in struct format
    struct.unpack('>Z', b'\x01\x02\x03')   -> error: bad char in struct format
ArchPcode('PIC-24E:LE:24:default')  bits=24 bytes=3
    .struct_fmt()                          -> '<Z'
    struct.calcsize('<Z')                  -> error: bad char in struct format
    struct.unpack('<Z', b'\x01\x02\x03')   -> error: bad char in struct format
ArchPcode('avr8:LE:16:extended')  bits=24 bytes=3
    .struct_fmt()                          -> '<Z'
    struct.calcsize('<Z')                  -> error: bad char in struct format
    struct.unpack('<Z', b'\x01\x02\x03')   -> error: bad char in struct format

ArchAMD64().struct_fmt(size=1) -> '<B', struct.calcsize -> 1
ArchAMD64().struct_fmt(size=2) -> '<H', struct.calcsize -> 2
ArchAMD64().struct_fmt(size=3) -> '<Z', struct.calcsize -> error: bad char in struct format
ArchAMD64().struct_fmt(size=4) -> '<I', struct.calcsize -> 4
ArchAMD64().struct_fmt(size=8) -> '<Q', struct.calcsize -> 8

After — the invalid size raises where it is asked for, and every size struct can express is unchanged:

with this change
archinfo: archinfo at this branch

ArchPcode('HCS-12:BE:24:default')  bits=24 bytes=3
    .struct_fmt()                          -> ValueError: Invalid size: struct has no format character for a 3-byte integer
ArchPcode('PIC-24E:LE:24:default')  bits=24 bytes=3
    .struct_fmt()                          -> ValueError: Invalid size: struct has no format character for a 3-byte integer
ArchPcode('avr8:LE:16:extended')  bits=24 bytes=3
    .struct_fmt()                          -> ValueError: Invalid size: struct has no format character for a 3-byte integer

ArchAMD64().struct_fmt(size=1) -> '<B', struct.calcsize -> 1
ArchAMD64().struct_fmt(size=2) -> '<H', struct.calcsize -> 2
ArchAMD64().struct_fmt(size=3) -> ValueError: Invalid size: struct has no format character for a 3-byte integer
ArchAMD64().struct_fmt(size=4) -> '<I', struct.calcsize -> 4
ArchAMD64().struct_fmt(size=8) -> '<Q', struct.calcsize -> 8

struct_fmt() returned "Z" for a 3-byte word. That is not a struct format
character, so every 24-bit architecture handed its callers a format string
that raises "bad char in struct format" on use. struct has integer format
characters for 1, 2, 4 and 8 bytes only and no way to describe a 3-byte
integer, so report the size as invalid instead of inventing a character for
it; such a word has to be assembled from bytes by the caller.
@zardus
zardus force-pushed the feature/fix-word-width branch from 02d70fa to a6e6c48 Compare August 29, 2026 06:45
register_arch documents regexes as "str or SRE_Pattern", but the type check
used re._pattern_type, which CPython removed in 3.7, so passing a compiled
pattern raised AttributeError instead of registering the architecture. Use the
public re.Pattern.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zardus

zardus commented Aug 29, 2026

Copy link
Copy Markdown
Member Author

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

ci / Lint was red at 9.69/9.70 on archinfo/arch.py. The regression was arithmetic, not a new
message: pylint scores a file as 10 - 10*(5*E + W + R + C)/statements, this diff deletes the two
elif size == 3 statements, and the message set is identical at both revisions.

              base b8ffe85   head a6e6c48
score         9.70           9.69
statements    526            524
messages      12 (1E 7W 3R 1C) -- identical set, diff of the two lists is empty

Rather than move the number with a disable comment, 2b741b0 removes a real message. arch.py:843
tested isinstance(rx, re._pattern_type), and re._pattern_type has not existed since CPython 3.7
while archinfo requires >= 3.12, so the compiled-pattern argument that register_arch's own docstring
advertises raised AttributeError instead of registering the architecture. It now uses re.Pattern.
Every in-tree caller passes strings, which is why nothing caught it.

Verified against merge base b8ffe85 with the same merge-base-relative comparison the hosted jobs
run:

ok   archinfo/arch.py: 9.70 -> 9.79
ok   tests/test_pcode.py: 10.00 -> 10.00
ok   tests/test_struct_fmt.py: new file scores 10.00/10.00
ok   archinfo/arch.py: pyright badness 0.2217529 -> 0.2107482

tests/test_struct_fmt.py and tests/test_pcode.py: 12 passed. Full tests/: 27 passed, 34 subtests
passed.

session: sharpen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants