~bigbes/tarantool

tarantool-protobuf

569bc3c9cbe68a88b744f3dd789421817fc6eeb0 — Eugene Blikh 2 months ago 89e781b
fix(wire): cap the Lua-number varint fast path at 2^51 (x86_64 corruption)

encode_varint had a fast path for Lua numbers in [2^28, 2^53) that emitted
each byte via bit.band(n, 0x7f) / math.floor(n / 128). bit.band routes
through LuaJIT's number->int32 conversion, which on x86_64 uses the
magic-number trick (add 2^52 + 2^51, read the low bits). That is exact only
while n + 2^52 + 2^51 < 2^53, i.e. n < 2^51; above it the addition rounds to
an even double and silently drops low bits, corrupting the varint.

arm64 LuaJIT uses an exact FP->int instruction, so the bug was invisible on
Apple-Silicon dev machines and only surfaced on x86_64 (a 64-bit lease ID in
tarantool-etcd round-tripped 3041234677171912 -> 3041234677171940 over gRPC,
breaking lease lookups). Cap the fast path at 2^51; values in [2^51, 2^53)
now fall through to the exact uint64 cdata loop.

Adds test/wire_varint_test.lua pinning the round-trip at the boundaries.
2 files changed, 63 insertions(+), 6 deletions(-)

M runtime/pb/wire.lua
A test/wire_varint_test.lua
M runtime/pb/wire.lua => runtime/pb/wire.lua +14 -6
@@ 131,17 131,25 @@ encode_varint_slow = function(n)
                bit.bor(bit.band(bit.rshift(n, 14), 0x7f), 0x80),
                bit.rshift(n, 21))
        end
        -- 2^28 <= n < 2^53: still exact in double, but past uint32.
        -- Emit one byte via Lua-number bit op, then recurse on the
        -- residue (which is now < n/128 and may now fit a fast path).
        if n < 2^53 then
        -- 2^28 <= n < 2^51: emit one byte via a Lua-number bit op, then
        -- recurse on the residue (now < n/128, may fit a fast path).
        --
        -- The bound is 2^51, not 2^53: `bit.band(n, 0x7f)` routes through
        -- LuaJIT's number->int32 conversion, which on x64 uses the
        -- magic-number trick (add 2^52 + 2^51, read the low bits). That is
        -- exact only while n + 2^52 + 2^51 < 2^53, i.e. n < 2^51; above it
        -- the addition rounds to an even double and silently drops low
        -- bits, corrupting the varint. (arm64 uses an exact FP->int op, so
        -- this only ever bit on x86_64.) Values in [2^51, 2^53) fall
        -- through to the exact uint64 cdata loop below.
        if n < 2^51 then
            return string.char(bit.bor(bit.band(n, 0x7f), 0x80))
                .. encode_varint_slow(math.floor(n / 128))
        end
    end
    -- Fallback: uint64 cdata path. Reached by cdata inputs, negative
    -- Lua numbers (sign-extended to 10-byte varint), and the (rare)
    -- case of a Lua number larger than 2^53.
    -- Lua numbers (sign-extended to 10-byte varint), and Lua numbers in
    -- [2^51, 2^53) that the fast path above deliberately skips.
    n = to_uint64(n)
    local out = {}
    local i = 1

A test/wire_varint_test.lua => test/wire_varint_test.lua +49 -0
@@ 0,0 1,49 @@
-- Regression coverage for varint encoding of large Lua-number inputs.
--
-- encode_varint had a fast path for Lua numbers in [2^28, 2^53) that emitted
-- bytes via `bit.band(n, 0x7f)`. On x86_64 LuaJIT's number->int32 conversion
-- uses the magic-number trick (add 2^52 + 2^51, read the low bits), which is
-- exact only for n < 2^51; above that it rounds and silently dropped low bits,
-- corrupting the varint. arm64 uses an exact FP->int op, so the bug was
-- invisible on Apple-Silicon dev machines and only surfaced on x86_64 CI
-- (a 64-bit lease ID round-tripped to a different value over gRPC).
local t = require('luatest')
local ffi = require('ffi')
local wire = require('pb.wire')

local g = t.group('wire.varint')

-- Round-trip a value through encode_varint -> decode_varint and assert the
-- decoded uint64 equals the input. Inputs are given as Lua numbers; the bug
-- only manifested for the Lua-number encode path, not for cdata inputs.
local function assert_roundtrip(n)
    local enc = wire.encode_varint(n)
    local dec = wire.decode_varint(enc, 1)
    t.assert_equals(dec, ffi.cast('uint64_t', n),
        string.format('varint round-trip for %.0f', n))
end

g.test_large_lua_number_roundtrip = function()
    -- The exact value that corrupted on x86_64 (a representative lease ID),
    -- plus the boundaries around the old 2^53 / new 2^51 fast-path cutoff.
    assert_roundtrip(3041234677171912)   -- corrupted to ...171940 on x64 pre-fix
    assert_roundtrip(2 ^ 51)             -- first value past the safe fast path
    assert_roundtrip(2 ^ 51 + 12345)
    assert_roundtrip(2 ^ 52)
    assert_roundtrip(2 ^ 53 - 1)         -- largest exact double integer
end

g.test_fast_path_boundaries_still_exact = function()
    for _, n in ipairs({0, 1, 127, 128, 16383, 16384, 2 ^ 21, 2 ^ 28,
                        2 ^ 28 + 1, 2 ^ 51 - 1}) do
        assert_roundtrip(n)
    end
end

g.test_cdata_inputs_unchanged = function()
    for _, n in ipairs({ffi.cast('uint64_t', 3041234677171912ULL),
                        ffi.cast('uint64_t', 0xFFFFFFFFFFFFFFFFULL)}) do
        local dec = wire.decode_varint(wire.encode_varint(n), 1)
        t.assert_equals(dec, n)
    end
end