From 27d3daa31140b93e57e26b74e652787c3a3e96bf Mon Sep 17 00:00:00 2001 From: "svc-finitelabs[bot]" <269744575+svc-finitelabs[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:50:11 -0500 Subject: [PATCH 1/3] fix: probe the string.pack dialect before binding the byte helper fast 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 --- CLAUDE.md | 22 +++++++++ run_tests.sh | 16 +++++++ src/bitn/_compat.lua | 75 +++++++++++++++++++++++++++++ src/bitn/bit32.lua | 4 +- src/bitn/bit64.lua | 4 +- test/lpack_shim.lua | 109 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 226 insertions(+), 4 deletions(-) create mode 100644 test/lpack_shim.lua diff --git a/CLAUDE.md b/CLAUDE.md index 5fac01a..c1e927c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -185,6 +185,19 @@ expression compiles would route them into the native branch and skip `to_unsigned()`. A consequence worth knowing: a Lua 5.1 install with LuaBitOp takes the LuaJIT path and reports `_compat.is_luajit = true`. +`_compat` probes `string.pack`/`string.unpack` the same way, and exports them as +`_compat.string_pack` / `_compat.string_unpack`, nil unless they honour the Lua 5.3 +contract. The byte helpers in bit32/bit64 take their fast path from those two names +and never from `string.pack` directly. Two unrelated APIs ship under those names: +Lua 5.3+ has `pack(fmt, ...)` with size-suffixed codes and `unpack(fmt, s, pos)` +returning value then position, while Control4's DriverWorks has lpack, with no size +suffixes and an argument-swapped `unpack(s, fmt, pos)` returning position then +value. Testing presence rather than contract is not a loud failure on a controller: +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, so `>I4` decodes +of 0, 42 and 65535 all returned 1 there with nothing raised. The probe therefore +checks returned values, across every format string the library uses. + ### Raw Operations (bit32 and bit64) Both modules expose `raw_*` variants of every operation (`raw_band`, `raw_bor`, @@ -214,6 +227,15 @@ make test-matrix # across Lua versions CI additionally covers **LuaJIT 2.0**, so a green local matrix is a weaker signal than a green CI one. +Every module runs twice per interpreter. The second pass installs +`test/lpack_shim.lua`, an lpack-shaped `string.pack`/`string.unpack` modelled on a +Control4 controller, before the module is required, and asserts that +`_compat.string_pack` came out nil before running the selftest. That assertion is +what makes the pass meaningful: without it the run would go green on an +interpreter that never had `string.pack` at all. On 5.3+ the shim also overwrites a +genuine `string.pack`, so the fallback is exercised on hosts that would otherwise +always take the fast path. + ## Benchmarking Each module includes a `benchmark()` function built on `bitn.utils.benchmark`, diff --git a/run_tests.sh b/run_tests.sh index 9a6a0d6..45cbfbe 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -115,6 +115,10 @@ run_test() { echo } +# Each module runs twice: once against whatever the interpreter provides, and once +# with string.pack/string.unpack replaced by the lpack-shaped pair a Control4 +# controller ships, which the byte helpers must detect and decline to call. The +# shim is installed before the require because the dialect is probed at load. run_selftest() { local module_name="$1" local module_key="$2" @@ -125,6 +129,18 @@ run_selftest() { os.exit(1) end " + run_test "$module_name [lpack string.pack]" "$module_key" " + require('test.lpack_shim').install() + local _compat = require('bitn._compat') + if _compat.string_pack ~= nil or _compat.string_unpack ~= nil then + print('FAIL: the dialect probe bound an lpack string.pack/string.unpack') + os.exit(1) + end + local result = require('$lua_module').selftest() + if not result then + os.exit(1) + end + " } run_selftest "16-bit operations" "bit16" "bitn.bit16" diff --git a/src/bitn/_compat.lua b/src/bitn/_compat.lua index bbc655a..9974521 100644 --- a/src/bitn/_compat.lua +++ b/src/bitn/_compat.lua @@ -29,6 +29,81 @@ _compat.to_unsigned = to_unsigned -- Constants local MASK32 = 0xFFFFFFFF +-------------------------------------------------------------------------------- +-- string.pack/string.unpack dialect detection +-------------------------------------------------------------------------------- + +-- Two unrelated APIs ship under these two names. Lua 5.3+ has pack(fmt, ...) with +-- size-suffixed codes and unpack(fmt, s, pos) returning value then position. +-- Control4's DriverWorks has lpack: pack(fmt, ...) with no size suffixes, and +-- unpack(s, fmt, pos) returning position then value. Presence says nothing about +-- which one is installed. +-- +-- Guessing wrong is not a loud failure. Measured on a controller, 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 erroring, so `>I4` decodes of 0, 42 +-- and 65535 all return 1 with nothing raised. So test the contract by value, on +-- every format this module uses, and fall back unless all of them answer exactly. +local string_pack = rawget(string, "pack") +local string_unpack = rawget(string, "unpack") + +local function packs_like_lua53() + if not (string_pack and string_unpack) then + return false + end + + local ok, s = pcall(string_pack, ">I4", 0x01020304) + if not (ok and s == "\1\2\3\4") then + return false + end + ok, s = pcall(string_pack, "I4I4", 0x01020304, 0x05060708) + if not (ok and s == "\1\2\3\4\5\6\7\8") then + return false + end + ok, s = pcall(string_pack, "I4", "\1\2\3\4", 1) + if not (ok and high == 0x01020304 and pos == 5) then + return false + end + ok, high, pos = pcall(string_unpack, "I4I4", "\1\2\3\4\5\6\7\8", 1) + if not (ok and high == 0x01020304 and low == 0x05060708 and pos == 9) then + return false + end + ok, high, low, pos = pcall(string_unpack, "I4", "\0\1\2\3\4", 2) + return ok and high == 0x01020304 and pos == 6 +end + +if not packs_like_lua53() then + string_pack, string_unpack = nil, nil +end + +--- `string.pack` when it honours the Lua 5.3 contract, otherwise nil. +--- @type nil|fun(fmt: string, ...: integer): string +_compat.string_pack = string_pack + +--- `string.unpack` when it honours the Lua 5.3 contract, otherwise nil. +--- @type nil|fun(fmt: string, s: string, pos: integer): ... +_compat.string_unpack = string_unpack + -------------------------------------------------------------------------------- -- Implementation 1: Native operators (Lua 5.3+) -------------------------------------------------------------------------------- diff --git a/src/bitn/bit32.lua b/src/bitn/bit32.lua index ed7f89a..f8bbfac 100644 --- a/src/bitn/bit32.lua +++ b/src/bitn/bit32.lua @@ -223,8 +223,8 @@ end local string_char = string.char local string_byte = string.byte -local string_pack = rawget(string, "pack") -local string_unpack = rawget(string, "unpack") +local string_pack = _compat.string_pack +local string_unpack = _compat.string_unpack -- % is the whole operation on the pure Lua backend; skip the call. local fast_band = _compat.has_native_ops or _compat.has_bit_lib diff --git a/src/bitn/bit64.lua b/src/bitn/bit64.lua index cffc839..316f8c7 100644 --- a/src/bitn/bit64.lua +++ b/src/bitn/bit64.lua @@ -30,8 +30,8 @@ local bit32_rshift = bit32.rshift local impl_name = _compat.impl_name local math_floor = math.floor local string_char = string.char -local string_pack = rawget(string, "pack") -local string_unpack = rawget(string, "unpack") +local string_pack = _compat.string_pack +local string_unpack = _compat.string_unpack -- Private metatable for Int64 type identification local Int64Meta = { __name = "Int64" } diff --git a/test/lpack_shim.lua b/test/lpack_shim.lua new file mode 100644 index 0000000..e1a5ae6 --- /dev/null +++ b/test/lpack_shim.lua @@ -0,0 +1,109 @@ +--- @module "test.lpack_shim" +--- Installs an lpack-shaped `string.pack`/`string.unpack`, the pair Control4's +--- DriverWorks ships under those two names, so the suite can prove that bitn +--- rejects the dialect and takes its manual byte path instead of calling it. +--- +--- The shape and the two quiet behaviours below were measured on a controller: +--- * `pack(fmt, ...)` has no size-suffixed codes, so a digit is an unknown code +--- * `unpack(data, fmt, pos)` is argument-swapped, and returns position then value +--- * the format is read as a C string, so a NUL byte ends it +--- * a code with fewer bytes left than it needs stops the run without raising +--- +--- The last two are why a Lua 5.3 shaped call is not simply an error here: the +--- payload lands where the format is expected, so `>I4` decodes of 0, 42 and 65535 +--- all come back as the position 1, matching the controller. +--- +--- Only the integer codes are modelled. Sweeping the first payload byte over 0-255 +--- accepts 14 of them here against the controller's 22, and the two sets differ +--- both ways (`B` is accepted here and not there, `A a c d f n p z` there and not +--- here). None of those codes are reachable from this library, which emits `I4` +--- alone, so the divergence does not affect what this double is for. +local lpack_shim = {} + +local SIZE = { b = 1, B = 1, h = 2, H = 2, i = 4, I = 4, l = 8, L = 8 } + +--- @param fmt string +--- @return string codes Format up to the first NUL, which terminates it +local function codes(fmt) + local stop = string.find(fmt, "\0", 1, true) + return stop and string.sub(fmt, 1, stop - 1) or fmt +end + +--- @param fmt string Format string, lpack dialect +--- @param ... integer Values to pack +--- @return string bytes +function lpack_shim.pack(fmt, ...) + local args, taken, little, out = { ... }, 0, true, {} + local format = codes(fmt) + for i = 1, #format do + local c = string.sub(format, i, i) + if c == "<" or c == "=" then + little = true + elseif c == ">" then + little = false + elseif c ~= " " and c ~= "," then + local size = SIZE[c] + if not size then + error("lpack shim: unsupported code '" .. c .. "'") + end + taken = taken + 1 + local v = args[taken] % (2 ^ (size * 8)) + local bytes = {} + for b = 1, size do + bytes[b] = string.char(math.floor(v / 2 ^ ((b - 1) * 8)) % 256) + end + if not little then + local reversed = {} + for b = 1, size do + reversed[b] = bytes[size - b + 1] + end + bytes = reversed + end + out[#out + 1] = table.concat(bytes) + end + end + return table.concat(out) +end + +--- @param data string Binary data +--- @param fmt string Format string, lpack dialect +--- @param pos? integer Starting position (default: 1) +--- @return integer nextpos, integer ... Position first, then the values +function lpack_shim.unpack(data, fmt, pos) + pos = pos or 1 + local little, out = true, {} + local format = codes(fmt) + for i = 1, #format do + local c = string.sub(format, i, i) + if c == "<" or c == "=" then + little = true + elseif c == ">" then + little = false + elseif c ~= " " and c ~= "," then + local size = SIZE[c] + if not size then + error("lpack shim: unsupported code '" .. c .. "'") + end + if pos + size - 1 > #data then + break + end + local v = 0 + for b = 0, size - 1 do + local byte = string.byte(data, pos + b) + v = v + byte * 2 ^ (little and (b * 8) or ((size - 1 - b) * 8)) + end + out[#out + 1] = v + pos = pos + size + end + end + return pos, (unpack or table.unpack)(out) +end + +--- Replace `string.pack`/`string.unpack` with the lpack-shaped pair. Must run +--- before the module under test is required, since the dialect is probed at load. +function lpack_shim.install() + string.pack = lpack_shim.pack + string.unpack = lpack_shim.unpack +end + +return lpack_shim From b3df66bd843f6be793a9b29269b0c8ceb8a9da76 Mon Sep 17 00:00:00 2001 From: "svc-finitelabs[bot]" <269744575+svc-finitelabs[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:05:17 -0500 Subject: [PATCH 2/3] docs: correct the lpack shim's reachability note 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. --- test/lpack_shim.lua | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/lpack_shim.lua b/test/lpack_shim.lua index e1a5ae6..ccf6b16 100644 --- a/test/lpack_shim.lua +++ b/test/lpack_shim.lua @@ -16,8 +16,13 @@ --- Only the integer codes are modelled. Sweeping the first payload byte over 0-255 --- accepts 14 of them here against the controller's 22, and the two sets differ --- both ways (`B` is accepted here and not there, `A a c d f n p z` there and not ---- here). None of those codes are reachable from this library, which emits `I4` ---- alone, so the divergence does not affect what this double is for. +--- here). Those bytes are reachable: the swapped signature is what puts the payload +--- in the format slot, so a little-endian u32 whose low byte is one of the eight is +--- quiet on a controller and raises here, about 3% of values. The divergence still +--- does not affect what this double is for, because `packs_like_lua53()` compares +--- returned values on every format the library emits including the offset case, so +--- it fails closed whether the dialect raises or answers quietly, and the suite +--- asserts `_compat.string_pack` came out nil, which neither behaviour perturbs. local lpack_shim = {} local SIZE = { b = 1, B = 1, h = 2, H = 2, i = 4, I = 4, l = 8, L = 8 } From e6bee93465fefae6614570ec93c3379f69559200 Mon Sep 17 00:00:00 2001 From: "svc-finitelabs[bot]" <269744575+svc-finitelabs[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 16:09:17 -0500 Subject: [PATCH 3/3] docs: name the mechanism that actually holds the shim divergence back 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. --- test/lpack_shim.lua | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/test/lpack_shim.lua b/test/lpack_shim.lua index ccf6b16..35b8af6 100644 --- a/test/lpack_shim.lua +++ b/test/lpack_shim.lua @@ -16,13 +16,16 @@ --- Only the integer codes are modelled. Sweeping the first payload byte over 0-255 --- accepts 14 of them here against the controller's 22, and the two sets differ --- both ways (`B` is accepted here and not there, `A a c d f n p z` there and not ---- here). Those bytes are reachable: the swapped signature is what puts the payload ---- in the format slot, so a little-endian u32 whose low byte is one of the eight is ---- quiet on a controller and raises here, about 3% of values. The divergence still ---- does not affect what this double is for, because `packs_like_lua53()` compares ---- returned values on every format the library emits including the offset case, so ---- it fails closed whether the dialect raises or answers quietly, and the suite ---- asserts `_compat.string_pack` came out nil, which neither behaviour perturbs. +--- here). Those bytes are reachable in principle: the swapped signature is what puts +--- the payload in the format slot, so a little-endian u32 whose low byte is one of +--- the eight is quiet on a controller and raises here, about 3% of values. +--- +--- It does not reach them today, and the reason is a short circuit rather than the +--- value comparisons. Counting calls at this boundary while `bitn._compat` loads, +--- under both lua5.1 and luajit, gives pack 1 and unpack 0: `packs_like_lua53()` +--- declines at its first `pack(">I4", ...)` on the `4` and returns, so the unpack +--- parser the divergence lives in is never entered. Widening `pack` here to accept +--- size suffixes would put the probe into that parser and make the divergence live. local lpack_shim = {} local SIZE = { b = 1, B = 1, h = 2, H = 2, i = 4, I = 4, l = 8, L = 8 }