parse_object_body allocates an object key, then may fail in parse_colon or
parse_value before that key is ever inserted into the object. Its cleanup defer
only walks `obj`, so a key that never got there is unreachable to it. The caller
cannot free it either -- a failed parse returns a nil Value -- so it leaks.
The same applies to the parsed element on the duplicate-key path, and to both on
the out-of-memory path.
JSON5 makes this reachable from ordinary malformed input, because an unquoted
ident is a legal key and anything other than a colon after it fails. Plain JSON
leaks it too, via a quoted key.
before, measured with a tracking allocator over 8 inputs x 2 specs:
LEAK JSON5 colon fails after unquoted key 1 alloc / 7 bytes
LEAK JSON colon fails after quoted key 1 alloc / 2 bytes
LEAK JSON5 colon fails after quoted key 1 alloc / 2 bytes
LEAK JSON value fails after key 1 alloc / 2 bytes
LEAK JSON5 value fails after key 1 alloc / 2 bytes
LEAK JSON nested value fails 2 alloc / 4 bytes
LEAK JSON5 nested value fails 2 alloc / 4 bytes
LEAK JSON deep nesting fails 3 alloc / 6 bytes
LEAK JSON5 deep nesting fails 3 alloc / 6 bytes
LEAK JSON array element fails 1 alloc / 2 bytes
LEAK JSON5 array element fails 1 alloc / 2 bytes
total leaked allocations: 17
after, same probe:
total leaked allocations: 0
The leak scales with nesting depth -- one orphaned key per enclosing object -- so
a service parsing untrusted JSON leaks a little on every malformed request.
The fix marks the key and the element as owned by the loop iteration until they
are stored, and frees them otherwise. The duplicate-key path loses its explicit
delete, which the same mechanism now covers.
Found via odinfmt, which reported a 7-byte leak in a downstream test that parses
`{ broken not json` to check that invalid input is rejected.
Regression test added to tests/core/encoding/json: it reports
`17 leaks and 0 bad frees` without this change and passes with it. The existing
11 tests pass unchanged under -define:ODIN_TEST_FAIL_ON_BAD_MEMORY=true.
The other half of the FP/SIMD addressing gap: LDR/STR B/H/S/D/Q had
unsigned-offset and (since the last commit) register-offset forms, but
no writeback. Twenty new table rows -- pre-indexed and post-indexed
across the five widths, V=1 with the same imm9 encodings the integer
forms use, base opcodes confirmed against llvm-mc. Blobs regenerated;
builders unchanged.
Pipeline tests: all twenty llvm-mc golden words, with a decode -> print
round-trip matching llvm's canonical spelling.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AFeLCDKi5kRMtHrskUaRfw
LDR/STR B/H/S/D/Q had unsigned-offset forms but nothing for
[Xn, Xm{, LSL #s}] -- ten new MEM_REG table rows (V=1, base opcodes
confirmed against llvm-mc), which also get the extended-register modes
for free now that one MEM_REG form serves both. Blobs and generated
tables regenerated; the builders are unchanged, since the new forms
share inst_ldr_r_m / inst_str_r_m signatures.
Pipeline tests: thirteen llvm-mc golden words across the five widths,
LSL and extended, with a decode -> print round-trip matching llvm's
canonical spelling.
Also: build.lua's structural check still demanded
tablegen/encoding_table.odin, which arm64/riscv/x86 renamed to
instruction_table.odin -- it now accepts either name, and --check
passes for all eleven ISAs again.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AFeLCDKi5kRMtHrskUaRfw
Register offset and extended-register offset are one instruction word --
the option field at 15:13 picks LSL / UXTW / SXTW / SXTX -- but only LSL
was reachable: no encode form used MEM_EXT, so [Xn, Wm, SXTW #s] had a
matcher, a packer, and no way to be asked for. Worse, decoding such a word
produced mode REG_OFFSET with a stray extend, which re-encoded as LSL --
a silent corruption round-trip.
The one MEM_REG form now serves both modes, the way the RM slot takes
plain and shifted registers: the matcher accepts EXT_REG_OFFSET and checks
the index width against the extend (UXTW/SXTW take Wm, UXTX/SXTX take Xm,
the byte/half extends match nothing -- and a REG_OFFSET index must now be
an X register), the OFFSET_REG packer writes option from the operand's
mode, and the decoder derives the mode from option rather than from which
form matched. MEM_EXT/OFFSET_EXT stay in their enums -- the values are
baked into the table blobs -- marked subsumed.
Also fixed while there: the decoder stored the raw S bit as the shift
amount, so LDR X0, [X1, X2, LSL #3] decoded -- and printed -- as LSL #1.
The amount is log2 of the transfer size, recovered from size(31:30) and,
for SIMD, opc<1>(23). The one thing Memory cannot represent is a byte
access with an explicit #0 (S=1, amount 0); it decodes as no amount.
New pipeline tests: ten llvm-mc golden words across the extends and
widths, decode/print round-trips matching llvm's canonical spelling, the
LSL amount, and four malformed-operand rejections.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AFeLCDKi5kRMtHrskUaRfw
Register widens to a u32: hw number in bits 0-4, class byte in bits 8-15 --
bit-identical to the old u16 layout -- and, for the new REG_SYS class only,
the 15-bit MRS/MSR field in bits 16-30. System_Register, its Operand_Kind,
and the union's sysreg member are gone; a system register is now a plain
.REGISTER operand distinguished by class, so it flows through matching,
packing, and printing like any other register.
Memory is untouched: every class legal in an address still lives entirely in
the low 16 bits of the u32, so its 16-bit register slots stay lossless and
NONE round-trips (verified: Odin bit_fields zero-extend on read and reject
overflowing constants at compile time). Operand stays 11 bytes -- the union's
largest member is still 8 -- and Instruction stays exactly 64.
op_sysreg survives as an op_reg alias so MRS/MSR call sites read as what
they are, and the sysreg constants keep their field value in their name:
NZCV is now Register(0x5A10_1000) where it was System_Register(0x5A10).
New pipeline test: every SYSREG_NAMES entry round-trips encode -> decode ->
print byte-exactly, with the expected word derived from the table value.
All 330 table + 134 pipeline checks pass; benchmarks show encode ~3% faster,
decode and print at parity.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AFeLCDKi5kRMtHrskUaRfw
A bench mode for the test binary (run with `-- bench`) that times encode,
decode, and print over a representative instruction mix at three working-set
sizes (L1 / L2 / RAM), with a correctness gate so the numbers can't come from
a mix that silently fails to match. Baseline for the sysreg/register-merge
A/B comparison.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AFeLCDKi5kRMtHrskUaRfw
An instruction carried four operands. The coprocessor transfers need
six -- `mcr p15, #0, r0, c0, c0, #0` -- and dropping the tail is not
merely printing short: an opc2 that is not zero encoded as zero, which
is a different instruction. Six operands do not fit alongside the rest
in 64 bytes, since Operand is 11 and Memory cannot go below 8, so
Instruction is 128 now and straddles two cache lines rather than one.
With room for them, the block itself turns out to have been wrong
throughout. CDP, MCR and MRC took CRd from bits 19:16, which is CRn;
CRd is 15:12. MCR and MRC read a four-bit opc1 where theirs is the
three bits at 23:21. Neither they nor CDP read opc2 at all. MCRR and
MRRC dropped CRm.
LDC and STC were worse: they took CRd from the base register's field,
read the offset with the halfword-load encoding rather than as an imm8
counting words, and their base pattern named P=0 W=0, which is the
unindexed form whose syntax is `{option}` and not what they printed.
They now have the offset, pre-indexed and post-indexed forms, with U
left out of the mask because it is the offset's sign and belongs to the
memory operand. The N bit asks the coprocessor for the long transfer
and the syntax spells that as an L, so LDCL, STCL, LDC2L and STC2L are
mnemonics of their own.
None of this is reachable from the table sweep, which decodes each
form's canonical word with every variable field zero -- for these that
means p0, which ARMv8 reserves and no assembler will take. So twelve
hand-picked words go in the pipeline test instead, checked against the
text an assembler accepts for them, including a p15 system-register
read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018UmHLRF11EoWwNWCJ7JGaA
CRC32 and its castagnoli variants are unconditional-only: the condition
field is fixed at AL. Their entries left bits 31:28 free, so every
condition decoded as a CRC32 that cannot exist.
VDUP from a lane packs the element size and the lane index into one
four-bit field -- `xxx1` is a byte lane, `xx10` a halfword, `x100` a
word -- and the two forms it had read that field as though it were an
ordinary by-scalar operand, which is a different layout entirely. The
word they named, with the field zero, is not an encoding at all. Six
forms now, two per element size, each with the size in its data type.
A coprocessor number is written `p15`. It had been printing as a plain
`#15`, which is not what an assembler takes, and it is a different kind
of thing from the CRn and CRm registers that were already spelled
`c0` -- so it gets a class of its own rather than sharing theirs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018UmHLRF11EoWwNWCJ7JGaA
The one-register-and-modified-immediate forms left the op bit and the Q
bit out of their masks. Those two bits are the whole difference between
VMOV and VMVN and between the D and Q widths, so a handful of D-form
entries answered for words that were neither -- a Q-width VMOV decoded
as a D-width VMVN. With them pinned the matrix has to be complete, and
it was not: only four of the ten cmodes had Q forms at all. Both
mnemonics now carry the full set, with cmode 1110 going to VMOV.I64
rather than VMVN when op is set, and cmode 1111 belonging to VMOV
alone.
VLDM and VSTM name their addressing mode, as LDM and STM do, so they
are VLDMIA and VLDMDB rather than one mnemonic apiece -- and the
decrement-before forms, which always write back, had no entries at all.
The broadcast structure loads are not mnemonics of their own. VLD2R and
friends are how the encoding is named, not how it is written: an
assembler writes `vld2.8 {d0[], d1[]}, [r0]`, a VLD2 with an all-lanes
list. They join VLD2, VLD3 and VLD4 as the forms they are, and the list
carries the spacing bit 5 gives it.
VFMA against bfloat16 names which half of each pair it takes on the
mnemonic, bottom or top. It had one form covering the top half under
the plain name and none for the bottom.
Every word the A32 sweep produces now round-trips through llvm-mc or is
one llvm's own disassembly cannot: 985 exact, 177 with no assemblable
text, 38 reserved, and PSB CSYNC and TSB CSYNC, which llvm does not
implement for AArch32.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018UmHLRF11EoWwNWCJ7JGaA
The last of the A32 sweep's disagreements, and they were mostly the
same shape: an operand the syntax names but no field encodes, left
printing as `#0`.
MRS names APSR or SPSR by the R bit; VMRS and VMSR name FPSCR and its
neighbours out of bits 19:16, which their masks had pinned shut so the
register could not vary at all; SETEND names LE or BE by the E bit.
Those are bare tokens in the syntax, which is what the special-register
classes already model, so the endian pair joins them as a register
class of its own. DBG read the whole eight-bit hint field where its
option is only the low four, so it printed the fixed bits above it.
RFE and SRS name their addressing mode the way LDM and STM do, so they
are four mnemonics each rather than one, and the P and U bits that pick
the mode are fixed bits of each form -- they had been left out of the
mask entirely, so every one of the eight words decoded as the DA form.
The writeback bit rides in the base register for RFE and in the
implicit SP for SRS.
VORR and VBIC against a modified immediate had no forms at all. Those
words fell through to the shift-by-immediate family that sits beside
them, and decoded as VSRA, VQSHRN and VQRSHRN with a shift of zero --
which is not a shift any of them can take.
The register field in an operand was fifteen bits, on the reasoning
that a register's raw value never passes 0x401F. Two classes do: the
coprocessor registers at 0x8000, and now the endian tokens at 0x9000.
Both were truncating silently. The field is sixteen bits, which fills
the word exactly. A memory base or index is always a GPR, so those stay
as they are.
The A32 sweep now round-trips 984 of its 1183 entries byte-exact
through llvm-mc, with nothing left that llvm and this disagree on: 159
are words llvm's own disassembly cannot assemble back, 38 are reserved
encodings, and the last two are PSB CSYNC and TSB CSYNC, which llvm
does not implement for AArch32 at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018UmHLRF11EoWwNWCJ7JGaA
The NEON one-register-and-modified-immediate forms -- VMOV, VMVN, VORR
and VBIC against a constant -- had no encoding on the value slot at
all. The routine that expands an 8-bit field by its cmode was already
written and already correct; nothing called it, so twenty-six forms
decoded their constant as zero.
Printing it back is a second problem. A modified immediate is a bit
pattern, and assemblers write it as one: in hex, or as a float when
cmode 1111 expanded it into one. Both are operand kinds of their own
now, so the VFP `vmov.f32 s0, #2.000000e+00` prints as the float it is
rather than as the decimal of its bit pattern.
Two more VMOV forms: the one that moves a pair of GPRs to a pair of S
registers encodes only the first of the pair, and the second went
unprinted rather than being named as the one after it; and the one that
moves a 32-bit lane to a GPR names the lane in bit 21, which it did not
read, so every lane printed as a bare register.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018UmHLRF11EoWwNWCJ7JGaA
