Skip to content

fix: probe the string.pack dialect before binding the byte helper fast path - #22

Closed
svc-finitelabs[bot] wants to merge 3 commits into
mainfrom
agent/FL-20-pack-dialect-probe
Closed

svc-finitelabs[bot] wants to merge 3 commits into
mainfrom
agent/FL-20-pack-dialect-probe

Conversation

@svc-finitelabs

Copy link
Copy Markdown
Contributor

Problem

bit32 and bit64's eight byte helpers took a fast path whenever
rawget(string, "pack") / rawget(string, "unpack") were truthy, then called them
with the Lua 5.3 contract. Control4's DriverWorks supplies both names from lpack,
a different API: pack(fmt, ...) has no size-suffixed codes, and
unpack(s, fmt, pos) is argument-swapped and returns position before value.

Measured on the dev controller (LuaJIT 2.1, _VERSION Lua 5.1, both functions
present):

call result
string.pack(">I4", n) raises, 0/16 sampled values succeeded
string.unpack(">I4", "\1\2\3\4", 1) raises
string.unpack(">I4", <BE bytes of 0>, 1) returns 1
string.unpack(">I4", <BE bytes of 42>, 1) returns 1
string.unpack(">I4", <BE bytes of 65535>, 1) returns 1

The decoders do not reliably raise. lpack reads the payload of a 5.3-shaped call
as its format string, and that parser stops at a NUL byte or when the data runs out
instead of erroring. Sweeping the leading payload byte over 0-255 on the controller,
22 values return a position rather than raising, and 0x00 is one of them. Since
(string_unpack(...)) keeps only the first return value, and lpack returns position
first, the helper hands back a small integer. Every big-endian u32 below 2^24
decodes to 1.

Encoders are uniformly loud; only the decode side is silent.

Fix

_compat probes the contract rather than presence, by value, across all four format
strings this library uses plus a non-1 offset, and exports
_compat.string_pack / _compat.string_unpack as nil unless every check answers
exactly. bit32/bit64 bind from those instead of from string directly, so a
failed probe falls through to the manual byte math that was always sitting below it.

This follows the backend selection already in _compat: probe behaviour, not version.

Test mode

run_tests.sh runs every module a second time with test/lpack_shim.lua installed
before the require, an lpack-shaped pair modelled on the controller measurements
above. The pass asserts _compat.string_pack came out nil before running the
selftest, so a green result cannot come from an interpreter that never had
string.pack. On 5.3+ the shim overwrites a genuine string.pack, so the fallback
gets exercised on hosts that would otherwise always take the fast path.

Needs no CI change: the matrix already runs make test-all on 5.1, 5.2, 5.3, 5.4,
LuaJIT 2.0 and 2.1, so the new mode runs on all six.

Verification

  • Hardware. The probe body, run verbatim on the dev controller, returns false.
    The fallback then round-trips 0, 1, 42, 1000, 65535, 16909060, 1789071852 and
    4294967295 with 0 mismatches, and be_bytes_to_u32 of 0 / 42 / 65535 returns
    0 / 42 / 65535 instead of 1 / 1 / 1.
  • Discrimination. The new lpack pass was run against unfixed src/ at
    330c855: bit32 and bit64 fail on both LuaJIT and 5.5, bit16 stays green (it has
    no byte helpers). So the test fails for the reason it claims.
  • Probe is not always-false. _compat.string_pack is non-nil on stock Lua 5.5
    and nil on LuaJIT 2.1, Lua 5.1, and on all three under the shim. The fast path is
    still taken where it is genuinely available.
  • Shim fidelity. Against the controller's 0-255 sweep, the shim reproduces every
    case this library can reach: pack raises, the 0x01 payload raises, and 0 / 42 /
    1000 / 65535 all return 1. It accepts 14 of 256 leading bytes against the
    controller's 22; the sets differ both ways on codes the library never emits, which
    is documented in the file.
  • Suites. 6/6 modules on Lua 5.5, LuaJIT 2.1 and real Lua 5.1, covering all three
    bit backends (native operators, bit library, pure Lua).
  • Amalgamation. amalg has no Lua 5.5 rock so make build could not run here.
    A hand-built package.preload bundle of the same shape loads and passes on all
    three interpreters, with the fast path bound on 5.5 and declined elsewhere. The CI
    build job runs the real thing on 5.4.
  • make format-check and make typecheck clean (luals 3.19.0, the pinned version).
    make lint will not run on this host: luacheck 1.2.0 does not load under Lua 5.5,
    and it fails identically at 330c855, so it is the box and not this change.

Not in this PR

The re-vendor chain. v0.6.3 is vendored in lua-crypto, lua-bthome-ble, lua-protobuf,
lua-noiseprotocol and control4-driver-template, and the affected helpers are reached
by the released lua-crypto v0.2.3, lua-bthome-ble v0.1.6 and lua-protobuf v0.6.8.
Tagging v0.6.4 and re-vendoring bottom-up is a separate call and is yours to make.

Refs FL-20

…t path

The eight byte helpers took their fast path whenever rawget(string, "pack")
and rawget(string, "unpack") were truthy, and then called them with the Lua
5.3 contract. Control4's DriverWorks supplies both names from lpack instead:
pack(fmt, ...) has no size-suffixed codes, and unpack(s, fmt, pos) is
argument-swapped and returns position before value. Every helper therefore
failed on a controller, in code paths where the manual fallback directly
below would have been correct.

It did not fail loudly everywhere. lpack reads a 5.3-shaped call's payload as
its format string, and that parser stops at a NUL byte or when the data runs
out rather than raising, so on a controller be_bytes_to_u32 returned 1 for 0,
42, 1000 and 65535 alike, silently. Sweeping the leading payload byte over
0-255 there, 22 values decode to a position instead of raising.

_compat now probes the contract by value across every format string the
library uses, plus a non-1 offset, and exports string_pack/string_unpack as
nil unless all of them answer exactly. bit32 and bit64 bind from those rather
than from string directly.

run_tests.sh runs every module a second time with test/lpack_shim.lua
installed, an lpack-shaped pair modelled on measurements from a controller.
That pass asserts the probe rejected it before running the selftest, so a
green result cannot come from an interpreter that never had string.pack at
all, and on 5.3+ it overwrites a genuine string.pack so the fallback is
exercised on hosts that would otherwise always take the fast path.

Refs FL-20
The header claimed the shim/controller divergence in accepted format codes
was unreachable "because this library emits I4 alone". That reasons about
the format bitn passes, but the swapped lpack signature is precisely what
puts the payload in the format slot, so the payload bytes are format codes
and the divergence is reachable. A little-endian u32 whose low byte is one
of A a c d f n p z is quiet on a controller and raises in the shim, about
3% of values; B inverts. Measured by sweeping the first payload byte over
0-255 through the shim under lua5.1 and luajit: 14 of 256 accepted against
the controller's 22.

The conclusion holds for a different reason, now stated: packs_like_lua53()
compares returned values across every format the library emits including the
offset case, so it fails closed whether the dialect raises or answers
quietly, and run_tests.sh asserts _compat.string_pack came out nil, which
neither behaviour can perturb.

Comment only, no behaviour change. Suite stays 6/6.
b3df66b replaced a wrong reachability reason with another wrong one. It said
the shim/controller divergence in accepted format codes is harmless because
packs_like_lua53() compares returned values on every format including the
offset case, so it fails closed either way. True in the abstract, but the
probe never gets there.

Counting calls at the shim boundary while bitn._compat loads, under lua5.1
and luajit, gives pack 1 and unpack 0. The probe declines at its first
pack(">I4", ...) on the 4 and returns, so the unpack parser the divergence
lives in is never entered. The short circuit is what holds, not the value
comparisons, and only counting at the boundary could tell the two apart:
both mechanisms predict a green suite.

Also records what would make the divergence live, since that is the part a
future editor needs: widening this shim's pack to accept size suffixes puts
the probe into the unpack parser.

Comment only, no behaviour change. Suite stays 6/6.
@derek-miller

Copy link
Copy Markdown
Contributor

Superseded by #23, which carries the same probe plus the lpack test pass and is approved; closing so nothing re-vendors from this branch.

@derek-miller
derek-miller deleted the agent/FL-20-pack-dialect-probe branch September 11, 2026 21:24
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.

1 participant