endian: read and write little-endian fields byte-wise (unaligned access fault) - #129
Conversation
The six little-endian accessors cast a uint8_t* at an arbitrary offset to
uint16_t*/uint32_t*/uint64_t* and dereference it:
const uint32_t *bytes = (const uint32_t *)(data + *pos);
return *bytes;
A pstop frame begins with two uint8_t fields, so every multi-byte field in
it lands on an unaligned offset: stamp at 2, received_stamp at 10, id at 18,
receiver_id at 22, and so on. None of the 32- or 64-bit fields is naturally
aligned, and the u16 checksum at 46 is only 2-aligned.
On x86 and Xtensa the hardware absorbs this, which is why it has not
surfaced. On a target that traps unaligned access it is fatal. Decoding a
frame on Cortex-M3 faults immediately:
E: ***** USAGE FAULT *****
E: Unaligned memory access
E: Faulting instruction address (r15/pc): 0x0000435c
It is also undefined behaviour everywhere, independently of the hardware:
the cast violates both the alignment requirement and strict aliasing, so a
compiler is entitled to assume the pointer is aligned and emit a wide load
regardless of target.
Rewrites all six to build and split values byte by byte, exactly as the
big-endian half already does -- those were always correct, which is why only
the _le variants are touched here. No behavioural change on a
little-endian host: the encoded bytes are identical, verified against
golden frames captured from the previous implementation.
Verified on x86-64 and on Cortex-M3 under QEMU, where encode/decode now
round-trips instead of faulting.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
Rewrites the little-endian read_uint*_le and write_uint*_le helpers to assemble and disassemble values byte by byte instead of casting the buffer to a wider pointer and dereferencing it. This removes reliance on host endianness and unaligned-access behaviour, so the LE codecs produce correct results on big-endian and alignment-strict platforms.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 45 functions depend on the 13 functions this change touches.
Health — this change adds coupling hotspots:
- new:
pstop_message_decode()— 7 callers, 6 callees - new:
pstop_message_encode()— 7 callers, 6 callees - new:
send_msg()— 3 callers, 5 callees - new:
sess_bond_step()— 1 callers, 8 callees - new:
main()— 0 callers, 31 callees - new:
comparator_task()— 0 callers, 23 callees - new:
main()— 0 callers, 15 callees - new:
comparator_task()— 0 callers, 12 callees - …and 3 more — each is listed as a finding
Verification — 45 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 29 function(s) in the blast radius were not formally verified this run
· 11 more finding(s) on lines outside this diff (see the check run).
|
@johnpolymath is this valid/relevant here? I want to be extra careful as this is safety related, so there are serious concerns for any changes. |
johnandromeda
left a comment
johnandromeda
left a comment
There was a problem hiding this comment.
Looks good! I'll merge it in. Thanks!
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
Rewrites the little-endian read/write helpers (read_uint16_le, read_uint32_le, read_uint64_le, write_uint16_le, write_uint32_le, write_uint64_le) to assemble and emit values byte-by-byte with explicit shifts instead of casting the buffer to a wider integer pointer. This avoids unaligned access and type-punning UB, and makes the little-endian encoding correct regardless of host byte order.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 45 functions depend on the 13 functions this change touches.
Health — this change adds coupling hotspots:
- new:
pstop_message_decode()— 7 callers, 6 callees - new:
pstop_message_encode()— 7 callers, 6 callees - new:
send_msg()— 3 callers, 5 callees - new:
sess_bond_step()— 1 callers, 8 callees - new:
main()— 0 callers, 34 callees - new:
comparator_task()— 0 callers, 25 callees - new:
main()— 0 callers, 15 callees - new:
comparator_task()— 0 callers, 12 callees - …and 3 more — each is listed as a finding
Verification — 45 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 29 function(s) in the blast radius were not formally verified this run
· 11 more finding(s) on lines outside this diff (see the check run).
4 participants
The six little-endian accessors in
endian.ccast auint8_t*at an arbitrary offset touint16_t*/uint32_t*/uint64_t*and dereference it:A pstop frame begins with two
uint8_tfields, so every multi-byte field lands on an unaligned offset:stampat 2,received_stampat 10,idat 18,receiver_idat 22, and so on. Not one of the 32- or 64-bit fields is naturally aligned; the u16 checksum at 46 is only 2-aligned.Impact
On x86 and Xtensa the hardware absorbs the unaligned access, which is why this has not surfaced — it works fine on the ESP32 targets. On a target that traps it, decoding a frame faults immediately. Cortex-M3 under QEMU:
It is also undefined behaviour independently of the hardware: the cast violates both the alignment requirement and strict aliasing, so a compiler is entitled to assume the pointer is aligned and emit a wide load on any target.
The change
Rewrites all six to build and split values byte by byte — exactly as the big-endian half already does. Those were always correct, which is why only the
_levariants are touched.No behavioural change on a little-endian host. The encoded bytes are identical, which I verified against golden frames captured from the previous implementation rather than regenerated from the new one.
Testing
native_sim)The x86 run includes golden-frame tests that assert encoded output against byte arrays captured from this library before the change, so a silent shift in the wire format would have failed them.
Context
Found while adding
qemu_cortex_m3coverage to a Zephyr application built onpstop_c, so that its unit suites can run on macOS —native_simis Linux-only. Independent of #121 (that touches onlyzephyr/, this onlypstop_c/); the two apply cleanly together.🤖 Generated with Claude Code