Hello, Thanks for review! See my comments. Fixes are applied and the branch was force-pushed. Sergey On 7/27/26 10:39, Sergey Kaplun via Tarantool-patches wrote: > Hi, Sergey! > Thanks for the patch! > Please consider my comments below. > > On 24.07.26, Sergey Bronnikov wrote: >> From: Mike Pall >> >> Reported by Sergey Bronnikov. >> >> (cherry picked from commit c3b379bf50c8819c61daa3afd9f21d9ec5708bd5) >> > Side note: We may mention that this is a follow-up for the commit > 78f4de4d7d0c410eeaf75cec5bb178fdb7db1452 ("Avoid negation of signed > integers in C that may hold INT*_MIN."). Updated, updated commit message is below:     FFI: Prevent sanitizer warning in carith_ptr().     Reported by Sergey Bronnikov.     (cherry picked from commit c3b379bf50c8819c61daa3afd9f21d9ec5708bd5)     The Undefined Behaviour Sanitizer [1] produce a warning about     signed integer overflow in the function carith_ptr(), when idx     equals INT_MIN in the expression idx = -idx. In practice,     negating -2147483648 wraps back to itself instead of producing the     correct positive value, leading to incorrect pointer arithmetic in     carith_ptr() for MM_sub (pointer subtraction). The patch fixes     that by converting idx to unsigned (uintptr_t), computes two's     complement negation via bitwise negation + 1 (safe from overflow     in unsigned arithmetic), and casts back to ptrdiff_t. This avoids     signed overflow entirely and correctly negates even the INT_MIN     edge case.     The patch follows up the commit 78f4de4d7d0c410eeaf75cec5bb178fdb7db1452     ("Avoid negation of signed integers in C that may hold INT*_MIN.").     [1]: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html     Sergey Bronnikov:     * added the description and the test for the problem     Part of tarantool/tarantool#12480 > > Feel free to ignore. > >> The Undefined Behaviour Sanitizer [1] produce a warning about > Typo: s/produce/produces/ Fixed, thanks. >> signed integer overflow in the function carith_ptr(), when idx >> equals INT_MIN in the expression idx = -idx. In practice, >> negating -2147483648 wraps back to itself instead of producing the > It's actually -9223372036854775808 or -0x8000000000000000 for 64-bit > architecture. Let's use hexademic notation (or just the `INT_MIN`) > instead of this number. replaced the number with INT_MIN > >> correct positive value, leading to incorrect pointer arithmetic in >> carith_ptr() for MM_sub (pointer subtraction). The patch fixes >> that by converting idx to unsigned (uintptr_t), computes two's >> complement negation via bitwise negation + 1 (safe from overflow >> in unsigned arithmetic), and casts back to ptrdiff_t. This avoids >> signed overflow entirely and correctly negates even the INT_MIN >> edge case. >> >> [1]:https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html >> >> Sergey Bronnikov: >> * added the description and the test for the problem >> >> Part of tarantool/tarantool#12480 > Typo: s/12480/12880/ Fixed, thanks! > >> --- >> >> Branch:https://github.com/tarantool/luajit/tree/ligurio/lj-1459-ub-carith_ptr >> Related issues: >> -https://github.com/LuaJIT/LuaJIT/issues/1459 >> -https://github.com/tarantool/tarantool/issues/12880 >> >> src/lj_carith.c | 2 +- >> .../lj-1459-subtraction-carith.test.lua | 16 ++++++++++++++++ >> 2 files changed, 17 insertions(+), 1 deletion(-) >> create mode 100644 test/tarantool-tests/lj-1459-subtraction-carith.test.lua >> >> diff --git a/src/lj_carith.c b/src/lj_carith.c >> index eb56d552..e971bfc2 100644 >> --- a/src/lj_carith.c >> +++ b/src/lj_carith.c > > >> diff --git a/test/tarantool-tests/lj-1459-subtraction-carith.test.lua b/test/tarantool-tests/lj-1459-subtraction-carith.test.lua >> new file mode 100644 >> index 00000000..df401a56 >> --- /dev/null >> +++ b/test/tarantool-tests/lj-1459-subtraction-carith.test.lua >> @@ -0,0 +1,16 @@ >> +local tap = require('tap') >> + >> +-- The test file to demonstrate UBSan warning in carith_ptr(). > Typo: s/UBSan/the UBSan/ > Minor: s/carith_ptr()/`carith_ptr()`/ Updated: --- a/test/tarantool-tests/lj-1459-subtraction-carith.test.lua +++ b/test/tarantool-tests/lj-1459-subtraction-carith.test.lua @@ -1,6 +1,7 @@  local tap = require('tap') --- The test file to demonstrate UBSan warning in carith_ptr(). +-- The test file to demonstrate the UBSan warning in +-- `carith_ptr()`.  -- See also: https://github.com/LuaJIT/LuaJIT/issues/1459.  local test = tap.test('lj-1459-subtraction-carith') > >> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1459. >> +local test = tap.test('lj-1459-subtraction-carith') >> + >> +test:plan(2) >> + >> +local func = load('_ = nil - 0LL%0') > Minor: Let's use loadstring instead (I know that they are literally the > same function in LuaJIT, but it's semantically better to use it for > loading strings, while `load()` is used for generators). > > For Lua 5.1 they are not the same: > > | lua5.1 -e 'load"print(1)"()' > | lua5.1: (command line):1: bad argument #1 to 'load' (function expected, got string) > > The `loadstring()` has been deprecated in Lua5.2. > > > This `0LL%0` looks weird. Let's just use the resulting > `-0x8000000000000000LL` instead. > > > Also, please add the comment that `nil` as the first operand of > subtraction is required, since it is required to trigger metamethod > invocation. It successfully passes the argument check since it may be > considered as NULL ptr for other metamethods. Updated: --- a/test/tarantool-tests/lj-1459-subtraction-carith.test.lua +++ b/test/tarantool-tests/lj-1459-subtraction-carith.test.lua @@ -7,7 +7,11 @@ local test = tap.test('lj-1459-subtraction-carith') test:plan(2) -local func = loadstring('_ = nil - 0LL%0') +-- The `nil` as the first operand of subtraction is required, +-- since it is required to trigger metamethod invocation. +-- It successfully passes the argument check since it may be +-- considered as NULL ptr for other metamethods. +local func = loadstring('_ = nil - 0x8000000000000000LL')  local res, err = pcall(func) test:is(res, false, 'correct result') > >> +local res, err = pcall(func) >> + >> +test:is(res, false, 'correct result') >> +local error_msg = "attempt to perform arithmetic on 'nil' and 'int64_t'" >> +test:ok(err:match(error_msg), 'error on subtraction') >> + >> +test:done(true) >> -- >> 2.43.0 >>