Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>,
	Sergey Bronnikov <estetus@gmail.com>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit] Prevent sanitizer warnings for lj_tab_new*() and table.new().
Date: Wed, 10 Jun 2026 14:03:44 +0300	[thread overview]
Message-ID: <8855aae6-92fa-4fdd-80c8-541dd347fecf@tarantool.org> (raw)
In-Reply-To: <aigBA_FxqFMura9f@root>

[-- Attachment #1: Type: text/plain, Size: 3530 bytes --]

Hi, Sergey,

thanks for the review. Fixes applied and force-pushed.

Sergey

On 6/9/26 15:03, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, after fixing a few nits below.
>
> On 09.06.26, Sergey Bronnikov wrote:
>> From: Mike Pall <mike>
>>
>> Reported by Sergey Bronnikov.
>>
>> (cherry picked from commit 8f421c81ec6aaae0bcd80e01f4353de200afbbc5)
>>
>> The Undefined Behaviour Sanitizer [1] produce a warning because
> Typo: s/produce/produces/

Fixed.


>
>> the function `lua_createtable()` takes signed integer arguments,
>> but the `lj_tab_new_ah()` was not properly validating or converting
>> these signed values before using them in unsigned arithmetic.
>>
>> The fix changes the signature of `lj_tab_new_ah()` to accept
>> uint32_t directly, and adjusts `lua_createtable()` to cast the
>> incoming signed int values to uint32_t before passing them.
>>
>> [1]:https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html
>>
>> Sergey Bronnikov:
>> * added the description and the test for the problem
>>
>> Part of tarantool/tarantool#12480
>> ---
>> Branch:https://github.com/tarantool/luajit/tree/ligurio/lj-1458-ub-lj_tab_new
>>
>> Related issues:
>> -https://github.com/tarantool/tarantool/issues/12480
>> -https://github.com/LuaJIT/LuaJIT/issues/1458
>>
>>
>>   src/lj_api.c                                  |  2 +-
>>   src/lj_tab.c                                  |  4 +--
>>   src/lj_tab.h                                  |  2 +-
>>   .../lj-1458-ub-table.new.test.lua             | 30 +++++++++++++++++++
>>   4 files changed, 34 insertions(+), 4 deletions(-)
>>   create mode 100644 test/tarantool-tests/lj-1458-ub-table.new.test.lua
>>
> <snipped>
>
>> diff --git a/test/tarantool-tests/lj-1458-ub-table.new.test.lua b/test/tarantool-tests/lj-1458-ub-table.new.test.lua
>> new file mode 100644
>> index 00000000..d0cf6ff5
>> --- /dev/null
>> +++ b/test/tarantool-tests/lj-1458-ub-table.new.test.lua
> Typo: s/table.new/table-new/
Fixed.
>
>> @@ -0,0 +1,30 @@
>> +local tap = require('tap')
>> +
>> +-- The test file to demonstrate UBSan warning for `table.new()`
>> +-- with a minimal and maximum array and hash parts values.
>> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1458.
>> +local test = tap.test('lj-1458-ub-table-new')
>> +
>> +test:plan(8)
> Lets use table_sizes * 2 here.
>
Updated.



>> +
>> +local table_new = require('table.new')
>> +
>> +local INT_MAX =  2 ^ 31 - 1
>> +local INT_MIN = -2 ^ 31
>> +
>> +local table_sizes = {
>> +  { 0, INT_MIN },
>> +  { 0, INT_MAX },
>> +  { INT_MIN, 0 },
>> +  { INT_MAX, 0 },
>> +}
>> +
>> +for _, case in ipairs(table_sizes) do
>> +  local apart, hpart = unpack(case)
>> +  local ok, err = pcall(table_new, apart, hpart)
>> +  local message = ('table.new(%d, %d)'):format(apart, hpart)
>> +test:is(ok, false, message .. ' is failed')
> Typo? s/failed/OK/


Right, fixed:

--- a/test/tarantool-tests/lj-1458-ub-table-new.test.lua
+++ b/test/tarantool-tests/lj-1458-ub-table-new.test.lua
@@ -23,7 +23,7 @@ for _, case in ipairs(table_sizes) do
    local apart, hpart = unpack(case)
    local ok, err = pcall(table_new, apart, hpart)
    local message = ('table.new(%d, %d)'):format(apart, hpart)
- test:is(ok, false, message .. ' is failed')
+ test:is(ok, false, message .. ' is ok')
test:ok(err:match('table overflow'), message .. ' correct error message')
  end

>
>> +test:ok(err:match('table overflow'), message .. ' correct error message')
>> +end
>> +
>> +test:done(true)
>> -- 
>> 2.43.0
>>

[-- Attachment #2: Type: text/html, Size: 6151 bytes --]

  reply	other threads:[~2026-06-10 11:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-09 10:55 Sergey Bronnikov via Tarantool-patches
2026-06-09 12:03 ` Sergey Kaplun via Tarantool-patches
2026-06-10 11:03   ` Sergey Bronnikov via Tarantool-patches [this message]
2026-06-10 13:42     ` Sergey Kaplun via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8855aae6-92fa-4fdd-80c8-541dd347fecf@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=estetus@gmail.com \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit] Prevent sanitizer warnings for lj_tab_new*() and table.new().' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox