Skip to content

Fix binary encoding of int:24 and others#2230

Merged
bettio merged 1 commit intoatomvm:release-0.7from
pguyot:w13/fix-little-24
Mar 27, 2026
Merged

Fix binary encoding of int:24 and others#2230
bettio merged 1 commit intoatomvm:release-0.7from
pguyot:w13/fix-little-24

Conversation

@pguyot
Copy link
Copy Markdown
Collaborator

@pguyot pguyot commented Mar 24, 2026

Needed by forthcoming JIT Xtensa assembler

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

@petermm
Copy link
Copy Markdown
Contributor

petermm commented Mar 24, 2026

https://ampcode.com/threads/T-019d1eb5-272c-707a-88db-9ba033e3e15d

Given byte-multiple-only widths, the non-byte-multiple bug (issue #1) is eliminated. The remaining real concerns are:

  1. The unaligned little-endian fallback is untested — when offset isn't byte-aligned (e.g. <<1:1, Val:24/little, 0:7>>), the byte-swap path (lines ~80–109) runs but no test covers it.
  2. Signed right-shift is implementation-defined — value >>= 8 on negative values (lines 75, 89). Should use uint64_t for bit manipulation.
  3. 0x01LL << 63 is UB on signed types (line 101). Use 0x01ULL.
  4. ^= instead of |= (line 107) — fragile if destination isn't zero-initialized.
  5. Widths > 64 bits — zero-fills rather than sign-extends for the aligned path; big-endian silently truncates.

Issues 2 and 3 are the most actionable — they're one-line fixes to avoid undefined/implementation-defined behavior.

So maybe 2 & 3?

@pguyot pguyot force-pushed the w13/fix-little-24 branch 2 times, most recently from df324c0 to 75dee35 Compare March 25, 2026 04:45
Signed-off-by: Paul Guyot <pguyot@kallisys.net>
@pguyot pguyot force-pushed the w13/fix-little-24 branch from 75dee35 to 5b948db Compare March 25, 2026 21:09
@bettio bettio merged commit 04fbf46 into atomvm:release-0.7 Mar 27, 2026
201 of 203 checks passed
@pguyot pguyot deleted the w13/fix-little-24 branch March 27, 2026 17:49
bettio added a commit that referenced this pull request Mar 30, 2026
Merge fixes, features, and optimizations from release-0.7, including:
- Add signature-driven code loader (#2229)
- Add UART support on generic_unix via POSIX termios NIFs (#2243)
- Add erlang node/1 BIF (#2225)
- Add erts_internal:cmp_term/2 NIF and fix map type ordering (#2226)
- Add short option to float_to_binary/list (#2240)
- Add locale-independent float parsing and fix overflow (#2246)
- JIT: add RISC-V 64-bit backend (#2231)
- JIT: add DWARF debug information support (#1910)
- JIT x86: instruction encoding optimizations (#2234)
- JIT: remove redundant AND when untagging integers (#2235)
- Fix bug in bs_match get_tail handling (#2242)
- Fix binary encoding of int:24 and others (#2230)
- Fix cancel_timer/1 spec and documentation (#2244)
- Resurrect opcodes emitted with no_bs_create_bin (#2245)
- Fix flaky tests related to GitHub DNS Resolver (#2232)
- Add git guide (#2106)
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.

3 participants