[Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.

[Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Thanks to Jesper Lundgren.

(cherry picked from commit fc63c938b522e147ea728b75f385728bf4a8fc35)

When `strftime()` returns empty result (for example, for "%p" on some
locales or when LuaJIT is built with musl's `strftime()` and format
string is invalid), the `os.date()` endless retries to format result
and reallocates buffer for resulting string. This leads to OOM.

This patch limits amount of retries by 4.

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

Part of tarantool/tarantool#8069
---

PR: https://github.com/tarantool/tarantool/pull/8237
Issues:
* https://github.com/LuaJIT/LuaJIT/issues/463
* https://github.com/tarantool/tarantool/issues/8069
Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci

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

diff --git a/src/lib_os.c b/src/lib_os.c
index 9e78d49a..ffbc3fdc 100644
--- a/src/lib_os.c
+++ b/src/lib_os.c
@@ -205,12 +205,12 @@ LJLIB_CF(os_date)
     setboolfield(L, "isdst", stm->tm_isdst);
   } else if (*s) {
     SBuf *sb = &G(L)->tmpbuf;
-    MSize sz = 0;
+    MSize sz = 0, retry = 4;
     const char *q;
     for (q = s; *q; q++)
       sz += (*q == '%') ? 30 : 1;  /* Overflow doesn't matter. */
     setsbufL(sb, L);
-    for (;;) {
+    while (retry--) {  /* Limit growth for invalid format or empty result. */
       char *buf = lj_buf_need(sb, sz);
       size_t len = strftime(buf, sbufsz(sb), s, stm);
       if (len) {
diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua
new file mode 100644
index 00000000..cce78b6e
--- /dev/null
+++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua
@@ -0,0 +1,19 @@
+local tap = require('tap')
+
+-- See also https://github.com/LuaJIT/LuaJIT/issues/463.
+local test = tap.test('lj-463-os-date-oom')
+test:plan(1)
+
+-- The ru_RU.utf8 locale is chosen as one that will be set on a
+-- developer's PC with high possibility. It may be unavailable at
+-- CI, so don't check the status and result of function calls.
+-- If it's unavailable `os.setlocale()` does nothing.
+-- Before the patch, the call to `os.date('%p')` on non-standard
+-- locale may lead to OOM.
+
+os.setlocale('ru_RU.utf8')
+os.date('%p')
+
+test:ok(true, 'os.date() finished without OOM')
+
+os.exit(test:check() and 0 or 1)
-- 
2.34.1

Re: [Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.

[Maxim Kokryashkin via Tarantool-patches]

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


Hi, Sergey!
Thanks for the patch!
LGTM, except for a few nits below.
> 
>>From: Mike Pall <mike>
>>
>>Thanks to Jesper Lundgren.
>>
>>(cherry picked from commit fc63c938b522e147ea728b75f385728bf4a8fc35)
>>
>>When `strftime()` returns empty result (for example, for "%p" on some
>>locales or when LuaJIT is built with musl's `strftime()` and format
>>string is invalid), the `os.date()` endless retries to format result
>Typo: s/endless/endlessly/
>Typo: s/format result/format the result/
>>and reallocates buffer for resulting string. This leads to OOM.
>Typo: s/buffer for resulting/the buffer for the resulting/
>>
>>This patch limits amount of retries by 4.
>Typo: s/amount/the amount/
>Side note: Why is it exactly 4?
>>
>>Sergey Kaplun:
>>* added the description and the test for the problem
>>
>>Part of tarantool/tarantool#8069
>>---
>>
>>PR:  https://github.com/tarantool/tarantool/pull/8237
>>Issues:
>>*  https://github.com/LuaJIT/LuaJIT/issues/463
>>*  https://github.com/tarantool/tarantool/issues/8069
>>Branch:  https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci
>>
>> src/lib_os.c | 4 ++--
>> .../lj-463-os-date-oom.test.lua | 19 +++++++++++++++++++
>> 2 files changed, 21 insertions(+), 2 deletions(-)
>> create mode 100644 test/tarantool-tests/lj-463-os-date-oom.test.lua
>>
>>diff --git a/src/lib_os.c b/src/lib_os.c
>>index 9e78d49a..ffbc3fdc 100644
>>--- a/src/lib_os.c
>>+++ b/src/lib_os.c
>>@@ -205,12 +205,12 @@ LJLIB_CF(os_date)
>>     setboolfield(L, "isdst", stm->tm_isdst);
>>   } else if (*s) {
>>     SBuf *sb = &G(L)->tmpbuf;
>>- MSize sz = 0;
>>+ MSize sz = 0, retry = 4;
>>     const char *q;
>>     for (q = s; *q; q++)
>>       sz += (*q == '%') ? 30 : 1; /* Overflow doesn't matter. */
>>     setsbufL(sb, L);
>>- for (;;) {
>>+ while (retry--) { /* Limit growth for invalid format or empty result. */
>>       char *buf = lj_buf_need(sb, sz);
>>       size_t len = strftime(buf, sbufsz(sb), s, stm);
>>       if (len) {
>>diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua
>>new file mode 100644
>>index 00000000..cce78b6e
>>--- /dev/null
>>+++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua
>>@@ -0,0 +1,19 @@
>>+local tap = require('tap')
>>+
>>+-- See also  https://github.com/LuaJIT/LuaJIT/issues/463 .
>>+local test = tap.test('lj-463-os-date-oom')
>>+test:plan(1)
>>+
>>+-- The ru_RU.utf8 locale is chosen as one that will be set on a
>>+-- developer's PC with high possibility. It may be unavailable at
>Typo: s/with high possibility/with a high probability/
>>+-- CI, so don't check the status and result of function calls.
>>+-- If it's unavailable `os.setlocale()` does nothing.
>>+-- Before the patch, the call to `os.date('%p')` on non-standard
>>+-- locale may lead to OOM.
>>+
>>+os.setlocale('ru_RU.utf8')
>>+os.date('%p')
>>+
>>+test:ok(true, 'os.date() finished without OOM')
>>+
>>+os.exit(test:check() and 0 or 1)
>>--
>>2.34.1
>--
>Best regards,
>Maxim Kokryashkin
> 

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

Re: [Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim!

Thanks for the review!

Fixed your comments, branch is force-pushed.

On 02.02.23, Maxim Kokryashkin wrote:
> 
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a few nits below.
> > 
> >>From: Mike Pall <mike>
> >>
> >>Thanks to Jesper Lundgren.
> >>
> >>(cherry picked from commit fc63c938b522e147ea728b75f385728bf4a8fc35)
> >>
> >>When `strftime()` returns empty result (for example, for "%p" on some
> >>locales or when LuaJIT is built with musl's `strftime()` and format
> >>string is invalid), the `os.date()` endless retries to format result
> >Typo: s/endless/endlessly/
> >Typo: s/format result/format the result/

Fixed, thanks!

> >>and reallocates buffer for resulting string. This leads to OOM.
> >Typo: s/buffer for resulting/the buffer for the resulting/

Fixed, thanks!

> >>
> >>This patch limits amount of retries by 4.
> >Typo: s/amount/the amount/

Fixed, thanks!

> >Side note: Why is it exactly 4?

That's what she said:)

> >>
> >>Sergey Kaplun:
> >>* added the description and the test for the problem
> >>
> >>Part of tarantool/tarantool#8069
> >>---
> >>
> >>PR:  https://github.com/tarantool/tarantool/pull/8237
> >>Issues:
> >>*  https://github.com/LuaJIT/LuaJIT/issues/463
> >>*  https://github.com/tarantool/tarantool/issues/8069
> >>Branch:  https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci
> >>
> >> src/lib_os.c | 4 ++--
> >> .../lj-463-os-date-oom.test.lua | 19 +++++++++++++++++++
> >> 2 files changed, 21 insertions(+), 2 deletions(-)
> >> create mode 100644 test/tarantool-tests/lj-463-os-date-oom.test.lua
> >>
> >>diff --git a/src/lib_os.c b/src/lib_os.c
> >>index 9e78d49a..ffbc3fdc 100644
> >>--- a/src/lib_os.c
> >>+++ b/src/lib_os.c
> >>@@ -205,12 +205,12 @@ LJLIB_CF(os_date)
> >>     setboolfield(L, "isdst", stm->tm_isdst);
> >>   } else if (*s) {
> >>     SBuf *sb = &G(L)->tmpbuf;
> >>- MSize sz = 0;
> >>+ MSize sz = 0, retry = 4;
> >>     const char *q;
> >>     for (q = s; *q; q++)
> >>       sz += (*q == '%') ? 30 : 1; /* Overflow doesn't matter. */
> >>     setsbufL(sb, L);
> >>- for (;;) {
> >>+ while (retry--) { /* Limit growth for invalid format or empty result. */
> >>       char *buf = lj_buf_need(sb, sz);
> >>       size_t len = strftime(buf, sbufsz(sb), s, stm);
> >>       if (len) {
> >>diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua
> >>new file mode 100644
> >>index 00000000..cce78b6e
> >>--- /dev/null
> >>+++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua
> >>@@ -0,0 +1,19 @@
> >>+local tap = require('tap')
> >>+
> >>+-- See also  https://github.com/LuaJIT/LuaJIT/issues/463 .
> >>+local test = tap.test('lj-463-os-date-oom')
> >>+test:plan(1)
> >>+
> >>+-- The ru_RU.utf8 locale is chosen as one that will be set on a
> >>+-- developer's PC with high possibility. It may be unavailable at
> >Typo: s/with high possibility/with a high probability/

Fixed.

> >>+-- CI, so don't check the status and result of function calls.
> >>+-- If it's unavailable `os.setlocale()` does nothing.
> >>+-- Before the patch, the call to `os.date('%p')` on non-standard
> >>+-- locale may lead to OOM.
> >>+
> >>+os.setlocale('ru_RU.utf8')
> >>+os.date('%p')
> >>+
> >>+test:ok(true, 'os.date() finished without OOM')
> >>+
> >>+os.exit(test:check() and 0 or 1)
> >>--
> >>2.34.1
> >--
> >Best regards,
> >Maxim Kokryashkin
> > 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.

[Maxim Kokryashkin via Tarantool-patches]

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


Hi!
Thanks for the fixes!
LGTM.
--
Best regards,
Maxim Kokryashkin
 
 
> 
>>Hi, Maxim!
>>
>>Thanks for the review!
>>
>>Fixed your comments, branch is force-pushed.
>>
>>On 02.02.23, Maxim Kokryashkin wrote:
>>>
>>> Hi, Sergey!
>>> Thanks for the patch!
>>> LGTM, except for a few nits below.
>>> > 
>>> >>From: Mike Pall <mike>
>>> >>
>>> >>Thanks to Jesper Lundgren.
>>> >>
>>> >>(cherry picked from commit fc63c938b522e147ea728b75f385728bf4a8fc35)
>>> >>
>>> >>When `strftime()` returns empty result (for example, for "%p" on some
>>> >>locales or when LuaJIT is built with musl's `strftime()` and format
>>> >>string is invalid), the `os.date()` endless retries to format result
>>> >Typo: s/endless/endlessly/
>>> >Typo: s/format result/format the result/
>>
>>Fixed, thanks!
>>
>>> >>and reallocates buffer for resulting string. This leads to OOM.
>>> >Typo: s/buffer for resulting/the buffer for the resulting/
>>
>>Fixed, thanks!
>>
>>> >>
>>> >>This patch limits amount of retries by 4.
>>> >Typo: s/amount/the amount/
>>
>>Fixed, thanks!
>>
>>> >Side note: Why is it exactly 4?
>>
>>That's what she said:)
>>
>>> >>
>>> >>Sergey Kaplun:
>>> >>* added the description and the test for the problem
>>> >>
>>> >>Part of tarantool/tarantool#8069
>>> >>---
>>> >>
>>> >>PR:  https://github.com/tarantool/tarantool/pull/8237
>>> >>Issues:
>>> >>*  https://github.com/LuaJIT/LuaJIT/issues/463
>>> >>*  https://github.com/tarantool/tarantool/issues/8069
>>> >>Branch:  https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci
>>> >>
>>> >> src/lib_os.c | 4 ++--
>>> >> .../lj-463-os-date-oom.test.lua | 19 +++++++++++++++++++
>>> >> 2 files changed, 21 insertions(+), 2 deletions(-)
>>> >> create mode 100644 test/tarantool-tests/lj-463-os-date-oom.test.lua
>>> >>
>>> >>diff --git a/src/lib_os.c b/src/lib_os.c
>>> >>index 9e78d49a..ffbc3fdc 100644
>>> >>--- a/src/lib_os.c
>>> >>+++ b/src/lib_os.c
>>> >>@@ -205,12 +205,12 @@ LJLIB_CF(os_date)
>>> >>     setboolfield(L, "isdst", stm->tm_isdst);
>>> >>   } else if (*s) {
>>> >>     SBuf *sb = &G(L)->tmpbuf;
>>> >>- MSize sz = 0;
>>> >>+ MSize sz = 0, retry = 4;
>>> >>     const char *q;
>>> >>     for (q = s; *q; q++)
>>> >>       sz += (*q == '%') ? 30 : 1; /* Overflow doesn't matter. */
>>> >>     setsbufL(sb, L);
>>> >>- for (;;) {
>>> >>+ while (retry--) { /* Limit growth for invalid format or empty result. */
>>> >>       char *buf = lj_buf_need(sb, sz);
>>> >>       size_t len = strftime(buf, sbufsz(sb), s, stm);
>>> >>       if (len) {
>>> >>diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua
>>> >>new file mode 100644
>>> >>index 00000000..cce78b6e
>>> >>--- /dev/null
>>> >>+++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua
>>> >>@@ -0,0 +1,19 @@
>>> >>+local tap = require('tap')
>>> >>+
>>> >>+-- See also  https://github.com/LuaJIT/LuaJIT/issues/463 .
>>> >>+local test = tap.test('lj-463-os-date-oom')
>>> >>+test:plan(1)
>>> >>+
>>> >>+-- The ru_RU.utf8 locale is chosen as one that will be set on a
>>> >>+-- developer's PC with high possibility. It may be unavailable at
>>> >Typo: s/with high possibility/with a high probability/
>>
>>Fixed.
>>
>>> >>+-- CI, so don't check the status and result of function calls.
>>> >>+-- If it's unavailable `os.setlocale()` does nothing.
>>> >>+-- Before the patch, the call to `os.date('%p')` on non-standard
>>> >>+-- locale may lead to OOM.
>>> >>+
>>> >>+os.setlocale('ru_RU.utf8')
>>> >>+os.date('%p')
>>> >>+
>>> >>+test:ok(true, 'os.date() finished without OOM')
>>> >>+
>>> >>+os.exit(test:check() and 0 or 1)
>>> >>--
>>> >>2.34.1
>>> >--
>>> >Best regards,
>>> >Maxim Kokryashkin
>>> > 
>>
>>--
>>Best regards,
>>Sergey Kaplun
> 

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

Re: [Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.

[sergos via Tarantool-patches]

Hi!

Thanks for the patch!

I can propose to update test with something like

  for i in io.popen('locale -a', 'r'):read('*a'):gmatch("([^\n]*)\n?") do
      os.setlocale(i)
      os.date(%p)
  end

to run the test across all available options. The reason is I got:

  tarantool> os.setlocale('ru_RU.utf8')
  ---
  - null

on my system, so I don’t by the ‘high probability’. 

Regards,
Sergos


> On 1 Feb 2023, at 10:46, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> From: Mike Pall <mike>
> 
> Thanks to Jesper Lundgren.
> 
> (cherry picked from commit fc63c938b522e147ea728b75f385728bf4a8fc35)
> 
> When `strftime()` returns empty result (for example, for "%p" on some
> locales or when LuaJIT is built with musl's `strftime()` and format
> string is invalid), the `os.date()` endless retries to format result
> and reallocates buffer for resulting string. This leads to OOM.
> 
> This patch limits amount of retries by 4.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#8069
> ---
> 
> PR: https://github.com/tarantool/tarantool/pull/8237
> Issues:
> * https://github.com/LuaJIT/LuaJIT/issues/463
> * https://github.com/tarantool/tarantool/issues/8069
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci
> 
> src/lib_os.c                                  |  4 ++--
> .../lj-463-os-date-oom.test.lua               | 19 +++++++++++++++++++
> 2 files changed, 21 insertions(+), 2 deletions(-)
> create mode 100644 test/tarantool-tests/lj-463-os-date-oom.test.lua
> 
> diff --git a/src/lib_os.c b/src/lib_os.c
> index 9e78d49a..ffbc3fdc 100644
> --- a/src/lib_os.c
> +++ b/src/lib_os.c
> @@ -205,12 +205,12 @@ LJLIB_CF(os_date)
>     setboolfield(L, "isdst", stm->tm_isdst);
>   } else if (*s) {
>     SBuf *sb = &G(L)->tmpbuf;
> -    MSize sz = 0;
> +    MSize sz = 0, retry = 4;
>     const char *q;
>     for (q = s; *q; q++)
>       sz += (*q == '%') ? 30 : 1;  /* Overflow doesn't matter. */
>     setsbufL(sb, L);
> -    for (;;) {
> +    while (retry--) {  /* Limit growth for invalid format or empty result. */
>       char *buf = lj_buf_need(sb, sz);
>       size_t len = strftime(buf, sbufsz(sb), s, stm);
>       if (len) {
> diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua
> new file mode 100644
> index 00000000..cce78b6e
> --- /dev/null
> +++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua
> @@ -0,0 +1,19 @@
> +local tap = require('tap')
> +
> +-- See also https://github.com/LuaJIT/LuaJIT/issues/463.
> +local test = tap.test('lj-463-os-date-oom')
> +test:plan(1)
> +
> +-- The ru_RU.utf8 locale is chosen as one that will be set on a
> +-- developer's PC with high possibility. It may be unavailable at
> +-- CI, so don't check the status and result of function calls.
> +-- If it's unavailable `os.setlocale()` does nothing.
> +-- Before the patch, the call to `os.date('%p')` on non-standard
> +-- locale may lead to OOM.
> +
> +os.setlocale('ru_RU.utf8')
> +os.date('%p')
> +
> +test:ok(true, 'os.date() finished without OOM')
> +
> +os.exit(test:check() and 0 or 1)
> -- 
> 2.34.1
> 

Re: [Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.

[Sergey Kaplun via Tarantool-patches]

Hi, Sergos!

Thanks for the review!

Changed the test as you propose:

===================================================================
diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua
index a39ab6d1..0861ce14 100644
--- a/test/tarantool-tests/lj-463-os-date-oom.test.lua
+++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua
@@ -4,15 +4,13 @@ local tap = require('tap')
 local test = tap.test('lj-463-os-date-oom')
 test:plan(1)
 
--- The ru_RU.utf8 locale is chosen as one that will be set on a
--- developer's PC with a high possibility. It may be unavailable
--- at CI, so don't check the status and result of function calls.
--- If it's unavailable `os.setlocale()` does nothing.
+-- Try all available locales to check the behaviour.
 -- Before the patch, the call to `os.date('%p')` on non-standard
--- locale may lead to OOM.
-
-os.setlocale('ru_RU.utf8')
-os.date('%p')
+-- locale (ru_RU.utf8, sv_SE.utf8, etc.) may lead to OOM.
+for locale in io.popen('locale -a'):read('*a'):gmatch('([^\n]*)\n?') do
+    os.setlocale(locale)
+    os.date('%p')
+end
 
 test:ok(true, 'os.date() finished without OOM')
===================================================================

Branch is force-pushed.

On 03.02.23, sergos wrote:
> Hi!
> 
> Thanks for the patch!
> 
> I can propose to update test with something like
> 
>   for i in io.popen('locale -a', 'r'):read('*a'):gmatch("([^\n]*)\n?") do
>       os.setlocale(i)
>       os.date(%p)
>   end
> 
> to run the test across all available options. The reason is I got:
> 
>   tarantool> os.setlocale('ru_RU.utf8')
>   ---
>   - null
> 
> on my system, so I don’t by the ‘high probability’. 

Lucky you:).

> 
> Regards,
> Sergos
> 
> 
> > On 1 Feb 2023, at 10:46, Sergey Kaplun <skaplun@tarantool.org> wrote:
> > 
> > From: Mike Pall <mike>
> > 
> > Thanks to Jesper Lundgren.
> > 
> > (cherry picked from commit fc63c938b522e147ea728b75f385728bf4a8fc35)
> > 
> > When `strftime()` returns empty result (for example, for "%p" on some
> > locales or when LuaJIT is built with musl's `strftime()` and format
> > string is invalid), the `os.date()` endless retries to format result
> > and reallocates buffer for resulting string. This leads to OOM.
> > 
> > This patch limits amount of retries by 4.
> > 
> > Sergey Kaplun:
> > * added the description and the test for the problem
> > 
> > Part of tarantool/tarantool#8069
> > ---
> > 
> > PR: https://github.com/tarantool/tarantool/pull/8237
> > Issues:
> > * https://github.com/LuaJIT/LuaJIT/issues/463
> > * https://github.com/tarantool/tarantool/issues/8069
> > Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci
> > 
> > src/lib_os.c                                  |  4 ++--
> > .../lj-463-os-date-oom.test.lua               | 19 +++++++++++++++++++
> > 2 files changed, 21 insertions(+), 2 deletions(-)
> > create mode 100644 test/tarantool-tests/lj-463-os-date-oom.test.lua
> > 
> > diff --git a/src/lib_os.c b/src/lib_os.c
> > index 9e78d49a..ffbc3fdc 100644
> > --- a/src/lib_os.c
> > +++ b/src/lib_os.c

<snipped>

> > diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua
> > new file mode 100644
> > index 00000000..cce78b6e
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua
> > @@ -0,0 +1,19 @@
> > +local tap = require('tap')
> > +
> > +-- See also https://github.com/LuaJIT/LuaJIT/issues/463.
> > +local test = tap.test('lj-463-os-date-oom')
> > +test:plan(1)
> > +
> > +-- The ru_RU.utf8 locale is chosen as one that will be set on a
> > +-- developer's PC with high possibility. It may be unavailable at
> > +-- CI, so don't check the status and result of function calls.
> > +-- If it's unavailable `os.setlocale()` does nothing.
> > +-- Before the patch, the call to `os.date('%p')` on non-standard
> > +-- locale may lead to OOM.
> > +
> > +os.setlocale('ru_RU.utf8')
> > +os.date('%p')
> > +
> > +test:ok(true, 'os.date() finished without OOM')
> > +
> > +os.exit(test:check() and 0 or 1)
> > -- 
> > 2.34.1
> > 
> 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.

[sergos via Tarantool-patches]

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

Thanks for update!

LGTM.

Sergos

> On 3 Feb 2023, at 11:32, Sergey Kaplun <skaplun@tarantool.org> wrote:
> 
> Hi, Sergos!
> 
> Thanks for the review!
> 
> Changed the test as you propose:
> 
> ===================================================================
> diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua
> index a39ab6d1..0861ce14 100644
> --- a/test/tarantool-tests/lj-463-os-date-oom.test.lua
> +++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua
> @@ -4,15 +4,13 @@ local tap = require('tap')
> local test = tap.test('lj-463-os-date-oom')
> test:plan(1)
> 
> --- The ru_RU.utf8 locale is chosen as one that will be set on a
> --- developer's PC with a high possibility. It may be unavailable
> --- at CI, so don't check the status and result of function calls.
> --- If it's unavailable `os.setlocale()` does nothing.
> +-- Try all available locales to check the behaviour.
> -- Before the patch, the call to `os.date('%p')` on non-standard
> --- locale may lead to OOM.
> -
> -os.setlocale('ru_RU.utf8')
> -os.date('%p')
> +-- locale (ru_RU.utf8, sv_SE.utf8, etc.) may lead to OOM.
> +for locale in io.popen('locale -a'):read('*a'):gmatch('([^\n]*)\n?') do
> +    os.setlocale(locale)
> +    os.date('%p')
> +end
> 
> test:ok(true, 'os.date() finished without OOM')
> ===================================================================
> 
> Branch is force-pushed.
> 
> On 03.02.23, sergos wrote:
>> Hi!
>> 
>> Thanks for the patch!
>> 
>> I can propose to update test with something like
>> 
>>  for i in io.popen('locale -a', 'r'):read('*a'):gmatch("([^\n]*)\n?") do
>>      os.setlocale(i)
>>      os.date(%p)
>>  end
>> 
>> to run the test across all available options. The reason is I got:
>> 
>>  tarantool> os.setlocale('ru_RU.utf8')
>>  ---
>>  - null
>> 
>> on my system, so I don’t by the ‘high probability’. 
> 
> Lucky you:).
> 
>> 
>> Regards,
>> Sergos
>> 
>> 
>>> On 1 Feb 2023, at 10:46, Sergey Kaplun <skaplun@tarantool.org> wrote:
>>> 
>>> From: Mike Pall <mike>
>>> 
>>> Thanks to Jesper Lundgren.
>>> 
>>> (cherry picked from commit fc63c938b522e147ea728b75f385728bf4a8fc35)
>>> 
>>> When `strftime()` returns empty result (for example, for "%p" on some
>>> locales or when LuaJIT is built with musl's `strftime()` and format
>>> string is invalid), the `os.date()` endless retries to format result
>>> and reallocates buffer for resulting string. This leads to OOM.
>>> 
>>> This patch limits amount of retries by 4.
>>> 
>>> Sergey Kaplun:
>>> * added the description and the test for the problem
>>> 
>>> Part of tarantool/tarantool#8069
>>> ---
>>> 
>>> PR: https://github.com/tarantool/tarantool/pull/8237
>>> Issues:
>>> * https://github.com/LuaJIT/LuaJIT/issues/463
>>> * https://github.com/tarantool/tarantool/issues/8069
>>> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci
>>> 
>>> src/lib_os.c                                  |  4 ++--
>>> .../lj-463-os-date-oom.test.lua               | 19 +++++++++++++++++++
>>> 2 files changed, 21 insertions(+), 2 deletions(-)
>>> create mode 100644 test/tarantool-tests/lj-463-os-date-oom.test.lua
>>> 
>>> diff --git a/src/lib_os.c b/src/lib_os.c
>>> index 9e78d49a..ffbc3fdc 100644
>>> --- a/src/lib_os.c
>>> +++ b/src/lib_os.c
> 
> <snipped>
> 
>>> diff --git a/test/tarantool-tests/lj-463-os-date-oom.test.lua b/test/tarantool-tests/lj-463-os-date-oom.test.lua
>>> new file mode 100644
>>> index 00000000..cce78b6e
>>> --- /dev/null
>>> +++ b/test/tarantool-tests/lj-463-os-date-oom.test.lua
>>> @@ -0,0 +1,19 @@
>>> +local tap = require('tap')
>>> +
>>> +-- See also https://github.com/LuaJIT/LuaJIT/issues/463.
>>> +local test = tap.test('lj-463-os-date-oom')
>>> +test:plan(1)
>>> +
>>> +-- The ru_RU.utf8 locale is chosen as one that will be set on a
>>> +-- developer's PC with high possibility. It may be unavailable at
>>> +-- CI, so don't check the status and result of function calls.
>>> +-- If it's unavailable `os.setlocale()` does nothing.
>>> +-- Before the patch, the call to `os.date('%p')` on non-standard
>>> +-- locale may lead to OOM.
>>> +
>>> +os.setlocale('ru_RU.utf8')
>>> +os.date('%p')
>>> +
>>> +test:ok(true, 'os.date() finished without OOM')
>>> +
>>> +os.exit(test:check() and 0 or 1)
>>> -- 
>>> 2.34.1
>>> 
>> 
> 
> -- 
> Best regards,
> Sergey Kaplun


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

Re: [Tarantool-patches] [PATCH luajit] Fix os.date() for wider libc strftime() compatibility.

[Igor Munkin via Tarantool-patches]

Sergey,

I've checked the patch into all long-term branches in tarantool/luajit
and bumped a new version in master, 2.10 and 1.10.

On 01.02.23, Sergey Kaplun via Tarantool-patches wrote:
> From: Mike Pall <mike>
> 
> Thanks to Jesper Lundgren.
> 
> (cherry picked from commit fc63c938b522e147ea728b75f385728bf4a8fc35)
> 
> When `strftime()` returns empty result (for example, for "%p" on some
> locales or when LuaJIT is built with musl's `strftime()` and format
> string is invalid), the `os.date()` endless retries to format result
> and reallocates buffer for resulting string. This leads to OOM.
> 
> This patch limits amount of retries by 4.
> 
> Sergey Kaplun:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#8069
> ---
> 
> PR: https://github.com/tarantool/tarantool/pull/8237
> Issues:
> * https://github.com/LuaJIT/LuaJIT/issues/463
> * https://github.com/tarantool/tarantool/issues/8069
> Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-463-os-date-oom-full-ci
> 
>  src/lib_os.c                                  |  4 ++--
>  .../lj-463-os-date-oom.test.lua               | 19 +++++++++++++++++++
>  2 files changed, 21 insertions(+), 2 deletions(-)
>  create mode 100644 test/tarantool-tests/lj-463-os-date-oom.test.lua
> 

<snipped>

> -- 
> 2.34.1
> 

-- 
Best regards,
IM