Skip to content

Vendor bitn v0.6.1 from the release artifact - #15

Merged
derek-miller merged 1 commit into
mainfrom
vendor-bitn-v0.6.1
Aug 8, 2026
Merged

Vendor bitn v0.6.1 from the release artifact#15
derek-miller merged 1 commit into
mainfrom
vendor-bitn-v0.6.1

Conversation

@derek-miller

Copy link
Copy Markdown
Contributor

Vendors bitn v0.6.1, taken from the GitHub release asset rather than a local make build.

sha256  684ba2c5ab905f5aaceff9fdd4512eed5c327926ca13730dec3c34760cbd9dae

The bump carries a real correctness fix, not just a version string. _compat.lua selected its native-operator implementation by testing whether a & b parses. LuaJIT rolling releases from 2026 accept that syntax but return a signed 32-bit result from the bit library, so the branch was taken on a runtime whose values need the to_unsigned() normalisation that branch skips. arshift gave plainly wrong answers on such a host, not merely sign-extended ones: arshift(0x80000000, 1) returned 0x40000000 instead of 0xC0000000.

v0.6.1 tests the value rather than the runtime, so it stays correct for whatever grows the syntax next.

Control4 controllers run LuaJIT (verified on a dev controller: jit is present, 2.1.1700206165), so this is not hypothetical for us. That particular build predates the syntax change, which is the only reason we had no exposure yet.

Full test suite passes against the bumped vendor.

Picks up the _compat.lua implementation-selection fix: the native-operator
branch was chosen by testing whether `a & b` parses, which LuaJIT rolling
releases from 2026 accept while returning signed 32-bit values from the bit
library. That skipped the to_unsigned() normalisation those hosts need and
made arshift return wrong values, not just differently-signed ones.

Artifact taken from the v0.6.1 release, sha256
684ba2c5ab905f5aaceff9fdd4512eed5c327926ca13730dec3c34760cbd9dae.

Full test suite passes.

@svc-finitelabs svc-finitelabs Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Provenance is exact, the correctness claim reproduces, and the change is a verified no-op on every runtime that is not affected.

Provenance, verified in two hops

  1. vendor/bitn.lua at 79fb2382 is byte-identical to the published bitn.lua asset on finitelabs/lua-bitn release v0.6.1 (asset id 506709615, 108,625 bytes, cmp clean). Its sha256 is 684ba2c5ab905f5aaceff9fdd4512eed5c327926ca13730dec3c34760cbd9dae, matching the body.
  2. The asset is not taken on faith either: the bitn._compat block embedded in it is line-for-line identical to src/bitn/_compat.lua at tag v0.6.1 (ee6406c), ignoring blank lines and the amalgamation scaffolding at the block boundary. The release is non-draft, non-prerelease, published 19:48:21Z, and its release job is green on that commit.

So the artifact traces back to tagged source, not just to "some file someone uploaded".

The correctness claim reproduces

I could not get the runtime itself: local LuaJIT is 2.1.1783773675, a 2026 rolling build that still does not parse a & b (load returns nil, so ok and result is false and it drops to the bit library). So I emulated the described host instead, by overriding load so the six probe chunks (a & b, a | b, a ~ b, ~a, a << n, a >> n) return bit-library-backed closures, then requiring the vendored copy. That reproduces "the syntax is accepted and the results are signed 32-bit" without needing the build.

emulated 2026-rolling host impl_name() raw_arshift(0x80000000, 1) run_tests.sh
main (v0.6.0) native operators (Lua 5.3+) 0x40000000 159/174
79fb2382 (v0.6.1) bit library correct 174/174

0x40000000 is exactly the number in the body. All 15 failures on main are zigzag32 / zigzag64 / sint32 / sint64, which is the expected blast radius: pb.zigzag_encode32 and pb.zigzag_encode64 call bit32.raw_arshift / bit64.raw_arshift, and on the native branch raw_arshift is _compat.arshift, whose is_negative = a >= 0x80000000 test can never be true once the value is already negative.

Non-regression on the runtimes I can actually reach: on real LuaJIT 2.1.1783773675 and on Lua 5.5.0, main and 79fb2382 select the same implementation and return identical arshift and raw_arshift values, 174/174 both. The change is inert where it should be inert.

Control4 exposure reads correctly. Controllers were measured at LuaJIT 2.1.1700206165 with a & b not parsing, so they take Implementation 2 with to_unsigned applied and are correct today. Worth remembering that this only holds because of the version: a Control4 OS update carrying a newer LuaJIT would have put them straight onto the broken path.

This PR obsoletes the CI pin, which is also why no check here exercises it

.github/workflows/build.yml:78-82 substitutes luajit-openresty for the luajit-2.1 matrix entry. It was added in c728105 (Jul 20) with this reason:

The rolling luajit-2.1 HEAD built by leafo/gh-actions-lua began miscompiling the signed arithmetic-shift edge cases in zigzag encoding (crypto/bthome pass on the same build; only protobuf's zigzag hits it, and it passes on a pinned local luajit-2.1).

That was not a miscompile. It is this bug, and every piece of the supporting evidence fits:

  • "only protobuf's zigzag hits it": protobuf is the consumer that calls raw_arshift. lua-crypto's two bit32.mask callers decompose with % 256, which is sign-agnostic, so they survive the same wrong branch.
  • "it passes on a pinned local luajit-2.1": an older build does not parse a & b, so it never takes the native branch.
  • the named symptom, arshift of INT_MIN in zigzag, is precisely the 15 failures above.

Two consequences. First, the green matrix on this PR is not evidence that the fix works, because no leg runs a runtime that takes the broken path. That is why I built the emulation rather than pointing at the checks. Second, the pin can come off in a follow-up, and doing so is what would give the repo real coverage against a regression of this class. Evidence that it is safe: lua-bitn's own workflow leaves luaVersion: ${{ matrix.lua-version }} unpinned, and at ee6406c (v0.6.1) its Lua luajit-2.1 leg is green. Not a blocker for this PR, and I have not tested whether the rolling build has unrelated problems.

Three sibling repos still carry the identical pre-fix blob

The blob this PR replaces, 92c9cbe, is byte-identical across four repos. After this merges:

  • lua-crypto still on 92c9cbe, tracked by FL-6
  • lua-bthome-ble still on 92c9cbe, untracked
  • lua-noiseprotocol still on 92c9cbe, untracked

FL-6 is scoped in its summary to lua-crypto, so it under-describes the situation. Happy to widen it to the remaining three, or to file the CI pin removal separately, whichever you prefer. Not doing either unprompted since vendor/ is shared-library surface.

Two hunks the body does not mention

The body describes the probe fix, which is the one that matters, but the 43 changed lines are three things:

  1. the probe fix, if fn becomes if fn and fn(0xFFFFFFFF, 0xFFFFFFFF) == 0xFFFFFFFF
  2. the pcall(require, "bit") probe now uses fresh bit_ok / bit_module locals instead of reusing ok / result from the native probe. Readability only: those locals were unconditionally reassigned before, so behaviour is unchanged.
  3. bit64.selftest() hoists test.expected into a local so type() narrowing applies, for the typecheck gate. Behaviour identical.

Both extras are upstream, both are inert. Noting it only so the line count does not read as a surprise.

One vendored comment is known-wrong, deliberately

The new comment says "anything with 32-bit semantics answers -1". That is false for a Lua 5.4 built with LUA_32BITS=1: there the 0xFFFFFFFF literal wraps on both sides of the comparison, so it is true and the native path is selected on a 32-bit host. This was raised on lua-bitn#14 and left as a wording note on purpose, because the obvious tightening makes things worse: > 0 correctly rejects that host but drops it to the pure Lua path, which dies on its own wrapped constants (0x100000000 is 0, then attempt to perform 'n%0'), scoring 0/3 instead of 1/3. Flagging it here so nobody "fixes" it in this repo. Irrelevant to Control4, which is 64-bit.

What I did not check

  • No run on a genuine rolling LuaJIT that parses a & b. The table above is an emulation, corroborated by the fact that its failure signature matches what c728105 independently observed in real CI.
  • Did not run make check locally. Relied on the Check job, green at 79fb2382, along with the other 7.
  • Did not audit the rest of the 3,300-line amalgamation. Byte-identity with the tagged release is the argument there, not reading it.

@derek-miller
derek-miller merged commit a4f8c26 into main Aug 8, 2026
8 checks passed
@derek-miller
derek-miller deleted the vendor-bitn-v0.6.1 branch August 8, 2026 23:10
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