Hi, Sergey,

thanks for the review! See my comments below.

Sergey

Hi, Sergey!
Thanks for the patch!
LGTM, with minor ignorable comments below.

On 05.06.26, Sergey Bronnikov wrote:

From: Mike Pall <mike>

Reported by Sergey Bronnikov.

(cherry picked from commit 86d414f5cae062b06998ec66b0696a47d4f6a0f0)

The function `lj_cf_os_time()` calculates `tm_mon` and `tm_year`
values using `get_field()` and when the helper function returns
a negative value the resulted values may be negative as well. This
is an undefined behaviour (signed integer overflow). The patch fixes
that by adding a cast for the resulted value to returned by

Typo: s/to returned/to be returned/

`get_field()`.

I am suggesting rephrasing this paragraph a bit to make it more clear:

| The function `lj_cf_os_time()` calculates the `tm_mon` and `tm_year`
| values using `get_field()`. If this helper function returns a negative
| result, subtraction may lead to undefined behavior due to signed integer
| overflow. The patch fixes the issue by adding a cast to the value
| returned by `get_field()`.

Feel free to ignore.

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-1454-ub-os-time

Related issues:

* https://github.com/tarantool/tarantool/issues/12480
* https://github.com/LuaJIT/LuaJIT/issues/1454

 src/lib_os.c                                  |  4 ++--
 test/tarantool-tests/lj-1454-os-time.test.lua | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)
 create mode 100644 test/tarantool-tests/lj-1454-os-time.test.lua

diff --git a/src/lib_os.c b/src/lib_os.c
index ffbc3fdc..0feb0d47 100644
--- a/src/lib_os.c
+++ b/src/lib_os.c

<snipped>

diff --git a/test/tarantool-tests/lj-1454-os-time.test.lua b/test/tarantool-tests/lj-1454-os-time.test.lua
new file mode 100644
index 00000000..2a48750c
--- /dev/null
+++ b/test/tarantool-tests/lj-1454-os-time.test.lua

I prefer the <lj-1454-ub-os-time.test.lua> name (as you named the
branch). Feel free to ignore.

Updated:

$ git show --name-only 

<snipped>
src/lib_os.c
test/tarantool-tests/lj-1454-ub-os-time.test.lua

@@ -0,0 +1,19 @@
+local tap = require('tap')
+
+-- The test file to demonstrate UBSan warning for `os.time()` with
+-- a huge indices value for month and/or year.

s/a huge indices value/huge negative index values/

Updated:

--- a/test/tarantool-tests/lj-1454-ub-os-time.test.lua
+++ b/test/tarantool-tests/lj-1454-ub-os-time.test.lua
@@ -1,7 +1,7 @@
 local tap = require('tap')
 
 -- The test file to demonstrate UBSan warning for `os.time()` with
--- a huge indices value for month and/or year.
+-- huge negative index values for month and/or year.
 -- See also: https://github.com/LuaJIT/LuaJIT/issues/1454.
 local test = tap.test('lj-1454-os-time')
 

+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1454.
+local test = tap.test('lj-1454-os-time')

I prefer the lj-1454-ub-os-time name (as you named the branch). Feel
free to ignore.

Updated:

 -- See also: https://github.com/LuaJIT/LuaJIT/issues/1454.
-local test = tap.test('lj-1454-os-time')
+local test = tap.test('lj-1454-ub-os-time')
 
 test:plan(1)

+
+test:plan(1)
+
+local INT_MIN = -2 ^ 31
+
+local cur_time = os.time({
+  day = 1,
+  month = INT_MIN,
+  year = INT_MIN,
+})
+test:is(cur_time, nil, 'os.time() with INT_MIN')
+
+test:done(true)
-- 
2.43.0