Skip to content

Pack and unpack words struct cannot describe - #721

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

Pack and unpack words struct cannot describe#721
zardus wants to merge 3 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

Opening any ELF on a 24-bit architecture failed with "bad char in struct format" before analysis began: ELF TLS setup writes the DTV pointer with pack_word, and struct has integer format characters for one, two, four and eight bytes only, so nothing names a three-byte word. unpack_word covered wider words with a halving recursion; pack_word had nothing.

Both now build a word of any width struct cannot name from its bytes.

The first commit stops unpack(), pack() and the two copies in ClemoryReadOnlyView from classifying a struct.error with calcsize() inside the handler, where a format struct cannot parse raises the same error again.

Regressions load an AVR object under a 24-bit p-code language and round-trip odd-width words. That object is a fixture angr/binaries#176 adds, so the checks here stay red until it merges. angr/archinfo#364 is the other half. Validation: #721 (comment).

@zardus

zardus commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS

Validation record for head 1571db8e5359f4191e7b010cbc4082f0d6c3e19a against baseline b58ea02a446106647cdaae32bdf91b7062404cc1.

  • Reproducer: cle.Loader on binaries/tests/avr/isqrt_atmega128.o with main_opts={"arch": archinfo.ArchPcode("avr8:LE:16:extended")}, a language whose word is three bytes wide — raises struct.error: bad char in struct format from cle/memory.py on baseline, loads on head. That object is the fixture Add fixtures for cle loader cases that had no real binary binaries#176 adds; cle's CI checks out binaries master, so the hosted checks here stay red until it merges
  • Regression: python -m pytest tests/test_tls_resiliency.py — 2 passed on head. Reverting cle/memory.py to the commit before the fix and leaving everything else at head fails test_tls_24bit_arch with ValueError: Invalid size: struct has no format character for a 3-byte integer, raised by struct_fmt in archinfo and reached through elf_tls.py:82elf_tls.py:118 _drop_intmemory.py:171 pack_word; restoring the file makes it pass again
  • Regression, wider: python -m pytest tests/test_tls_resiliency.py tests/test_unpackword.py tests/test_clemory.py — 10 passed on head; test_tls_24bit_arch, test_word_sizes_struct_cannot_express, test_word_off_the_end_of_a_backer, test_clemory_malformed_format and test_clemory_read_only_view_malformed_format all fail on baseline. test_tls_24bit_arch reads the DTV pointer back out of the loaded thread rather than only asserting that the load returned
  • Full suite: python -m pytest tests/ -n auto — 207 passed, 9 skipped
  • Lint/type: pylint and pyright scored per changed file against the merge base, the way the hosted jobs score them — cle/memory.py 10.00 -> 10.00, tests/test_clemory.py 5.00 -> 6.36, tests/test_tls_resiliency.py 9.33 -> 9.68, tests/test_unpackword.py 10.00 -> 10.00; pyright badness flat or lower on all four
  • 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

The full-suite and workspace-gate lines were measured at 01a4fe8, the previous head. The only change since is that test_tls_24bit_arch loads the fixture instead of assembling its own ELF, which the focused runs above re-check; lint, type and pre-commit were re-run at this head.

Behaviour outside the reported failure, compared against baseline by running the same matrix of sizes, signedness and endness under both revisions: pack_word and unpack_word still raise KeyError at the same address for an unmapped or short access, and a failed odd-width write now leaves memory untouched instead of storing the bytes that fit. unpack_word returns the same values for the one, two, four, eight, sixteen and thirty-two byte cases tests/test_unpackword.py already covered. Sizes above eight that are not powers of two used to raise ValueError and now work, as does pack_word above eight. Two differences show up only at the widths struct cannot name: a signed value that does not fit reports OverflowError from int.to_bytes rather than the struct.error the other widths report, and a word spanning two adjacent backers is composed rather than refused, because the composed path reads through load. In-tree callers that pass a size pass 2 or 4, or an expression width from angr, and catch KeyError only.

Scope: 14 of the 183 p-code languages pypcode 4.0.1.dev0 exposes report a three-byte word, and ArchPcode gives every p-code architecture a dtv_offsets of [0], so the DTV write happens for every ELF on one of them whether or not it has TLS data. It surfaced in a corpus sweep, where every AVR object loaded under one of the three 24-bit AVR languages failed to open.

Caveats: test_tls_24bit_arch skips without pypcode, which the testing dependency group installs. The 9 skips are the pre-existing test_macho_bindinghelper.py ones. The test names avr8:LE:16:extended rather than letting the loader pick it, because opinion matching in cle/backends/elf/elf.py compares an opinion's secondary constraint against e_type where Ghidra means e_flags, so every EM_AVR ELF autodetects as the 16-bit default; the fixture's own e_flags do select the extended variant, and fixing the matching is separate work. The other struct_fmt callers in cle, elfcore note parsing and StaticWord externs, still have no way to describe a three-byte word and are left alone here; neither is reached by loading an ELF that has no core notes and no glibc imports. This fixes loading; CFGFast on a 24-bit architecture still hits unrelated defects further in, inside angr's p-code lifter.

@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/cle_721

zardus added 2 commits August 9, 2026 22:46
Clemory.unpack() and pack() catch struct.error to tell an access that ran off
the end of its backer, which is a KeyError, from an operation that genuinely
failed. They made that decision by calling struct.calcsize() on the same format
string, which raises the same error when the format is one struct cannot parse.
That exception escaped from inside the handler, so neither answer was reported
and callers saw a bare struct.error from a line that was only meant to be
measuring. ClemoryReadOnlyView.unpack() carries two more copies of that handler,
one for the cached backer and one for the general lookup.

Work out which exception to report first and raise it afterwards, outside the
handler, so a short access no longer carries the struct error as its context
either.
struct has integer format characters for 1, 2, 4 and 8 bytes only, so pack_word
had no way to write the 3-byte word of a 24-bit architecture, and unpack_word
covered wider words with a halving recursion that rejected every size that was
not a power of two. ELF TLS setup writes the DTV pointer with pack_word, so
opening any ELF on a 24-bit architecture failed with "bad char in struct format"
before analysis began.

Build a word of any width struct cannot name from its bytes instead. The write
checks that the whole word is backed first, because store() writes the bytes
that fit before it reports the overrun.
@zardus
zardus force-pushed the feature/fix-word-width branch from 18e03c7 to 01a4fe8 Compare August 9, 2026 22:48
test_tls_24bit_arch assembled its own ELF32 header with struct.pack to reach an
architecture whose word is three bytes wide, so it exercised a container shape
no toolchain emits.

Load tests/avr/isqrt_atmega128.o from angr/binaries instead. Its e_flags name
the extended-address AVR variant, which Ghidra maps to avr8:LE:16:extended, a
24-bit language; name that language explicitly because cle picks the 16-bit
default for every EM_AVR ELF today.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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