Skip to content

Commit

Permalink
Fix bit op coercion in DUALNUM builds.
Browse files Browse the repository at this point in the history
Thanks to Sergey Kaplun.

(cherry picked from commit f5fd222)

The `lj_carith_check64()` function coerces the given number value to the
32-bit wide value. In this case, the 64-bit-wide operands will lose
upper bits.

This patch removes the excess coercion for the DUALNUM mode and drops
the corresponding skipcond introduced for the test in the previous
commit.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#10199
  • Loading branch information
Mike Pall authored and Buristan committed Oct 2, 2024
1 parent 003dbdd commit 2022c5a
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 6 deletions.
4 changes: 1 addition & 3 deletions src/lj_carith.c
Original file line number Diff line number Diff line change
Expand Up @@ -349,9 +349,7 @@ uint64_t lj_carith_check64(lua_State *L, int narg, CTypeID *id)
if (LJ_LIKELY(tvisint(o))) {
return (uint32_t)intV(o);
} else {
int32_t i = lj_num2bit(numV(o));
if (LJ_DUALNUM) setintV(o, i);
return (uint32_t)i;
return (uint32_t)lj_num2bit(numV(o));
}
}

Expand Down
3 changes: 0 additions & 3 deletions test/tarantool-tests/lj-1079-fix-64-bitshift-folds.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ end
test:test('rol', test_64bitness, bitop_rotation, bit.rol)
test:test('ror', test_64bitness, bitop_rotation, bit.ror)
test:test('rshift signed', test_64bitness, bitop_rshift_signed)
-- XXX: This skipcond will be dropped in the next commit.
local IS_DUALNUM = tostring(tonumber('-0')) ~= tostring(-0)
test:skipcond({['Has inconsistent behaviour for DUALNUM mode'] = IS_DUALNUM})
test:test('rshift huge', test_64bitness, bitop_rshift_huge)

test:done(true)
19 changes: 19 additions & 0 deletions test/tarantool-tests/lj-1273-dualnum-bit-coercion.test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
local tap = require('tap')

-- Test file to demonstrate LuaJIT misbehaviour on operating
-- for 64-bit operands in built-in `bit` library in DUALNUM mode.
-- See also, https://github.com/LuaJIT/LuaJIT/issues/1273.

local test = tap.test('lj-1273-dualnum-bit-coercion')
test:plan(1)

local bit = require('bit')

-- The cdata value for 2 ^ 33.
local EXPECTED = 2 ^ 33 + 0LL
-- Same value is used as mask for `bit.band()`.
local MASK = EXPECTED

test:is(bit.band(2 ^ 33, MASK), EXPECTED, 'correct bit.band result')

test:done(true)

0 comments on commit 2022c5a

Please sign in to comment.