[Tarantool-patches] [PATCH luajit] Limit number of string format elements to compile.

[Tarantool-patches] [PATCH luajit] Limit number of string format elements to compile.

[Sergey Kaplun via Tarantool-patches]

From: Mike Pall <mike>

Reported by pwnhacker0x18.

(cherry picked from commit 4fc48c50fe3f3f5a9680bada5c0c0d0d7eb345a3)

When compiling `string.format()` with a huge sequence of elements, it is
possible that too many KGC IRs underflow the IR buffer. This patch
limits the maximum number of `string.format()` elements to be compiled
to 100.

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

Part of tarantool/tarantool#10199
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1203-limit-format-elements
Related issues:
* https://github.com/tarantool/tarantool/issues/10199
* https://github.com/LuaJIT/LuaJIT/issues/1203

 src/lj_ffrecord.c                             |  2 ++
 .../lj-1203-limit-format-elements.test.lua    | 28 +++++++++++++++++++
 2 files changed, 30 insertions(+)
 create mode 100644 test/tarantool-tests/lj-1203-limit-format-elements.test.lua

diff --git a/src/lj_ffrecord.c b/src/lj_ffrecord.c
index d5fc081e..3b82d044 100644
--- a/src/lj_ffrecord.c
+++ b/src/lj_ffrecord.c
@@ -962,6 +962,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
   TRef hdr, tr;
   FormatState fs;
   SFormat sf;
+  int nfmt = 0;
   /* Specialize to the format string. */
   emitir(IRTG(IR_EQ, IRT_STR), trfmt, lj_ir_kstr(J, fmt));
   tr = hdr = recff_bufhdr(J);
@@ -1031,6 +1032,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
       recff_nyiu(J, rd);
       return;
     }
+    if (++nfmt > 100) lj_trace_err(J, LJ_TRERR_TRACEOV);
   }
   J->base[0] = emitir(IRT(IR_BUFSTR, IRT_STR), tr, hdr);
 }
diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
new file mode 100644
index 00000000..f17d4e37
--- /dev/null
+++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
@@ -0,0 +1,28 @@
+local tap = require('tap')
+
+-- Test file to demonstrate LuaJIT incorrect recording of
+-- `string.format()` function with huge amount of elements.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1173.
+
+local test = tap.test('lj-1203-limit-format-elements'):skipcond({
+  ['Test requires JIT enabled'] = not jit.status(),
+})
+
+test:plan(2)
+
+jit.opt.start('hotloop=1')
+
+-- XXX: Use a huge amount of format elements to process, which
+-- creates a lot of string constants.
+local NELEMENTS = 25000
+local fmt = ('%'):rep(NELEMENTS * 2)
+local expected = ('%'):rep(NELEMENTS)
+local result
+for _ = 1, 4 do
+  result = fmt:format()
+end
+
+test:ok(true, 'no IR buffer underflow')
+test:is(result, expected, 'correct result')
+
+test:done(true)
-- 
2.46.0

Re: [Tarantool-patches] [PATCH luajit] Limit number of string format elements to compile.

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey,

thanks for the patch!

See my comments below.

Sergey

On 26.08.2024 13:25, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by pwnhacker0x18.
>
> (cherry picked from commit 4fc48c50fe3f3f5a9680bada5c0c0d0d7eb345a3)
>
> When compiling `string.format()` with a huge sequence of elements, it is
> possible that too many KGC IRs underflow the IR buffer. This patch
> limits the maximum number of `string.format()` elements to be compiled
> to 100.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#10199
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1203-limit-format-elements
> Related issues:
> *https://github.com/tarantool/tarantool/issues/10199
> *https://github.com/LuaJIT/LuaJIT/issues/1203
>
>   src/lj_ffrecord.c                             |  2 ++
>   .../lj-1203-limit-format-elements.test.lua    | 28 +++++++++++++++++++
>   2 files changed, 30 insertions(+)
>   create mode 100644 test/tarantool-tests/lj-1203-limit-format-elements.test.lua
>
> diff --git a/src/lj_ffrecord.c b/src/lj_ffrecord.c
> index d5fc081e..3b82d044 100644
> --- a/src/lj_ffrecord.c
> +++ b/src/lj_ffrecord.c
> @@ -962,6 +962,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
>     TRef hdr, tr;
>     FormatState fs;
>     SFormat sf;
> +  int nfmt = 0;
>     /* Specialize to the format string. */
>     emitir(IRTG(IR_EQ, IRT_STR), trfmt, lj_ir_kstr(J, fmt));
>     tr = hdr = recff_bufhdr(J);
> @@ -1031,6 +1032,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
>         recff_nyiu(J, rd);
>         return;
>       }
> +    if (++nfmt > 100) lj_trace_err(J, LJ_TRERR_TRACEOV);
>     }
>     J->base[0] = emitir(IRT(IR_BUFSTR, IRT_STR), tr, hdr);
>   }
> diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> new file mode 100644
> index 00000000..f17d4e37
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> @@ -0,0 +1,28 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate LuaJIT incorrect recording of
> +-- `string.format()` function with huge amount of elements.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1173.
Seems a correct link is https://github.com/LuaJIT/LuaJIT/issues/1203
> +
> +local test = tap.test('lj-1203-limit-format-elements'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +test:plan(2)
> +
> +jit.opt.start('hotloop=1')
> +
> +-- XXX: Use a huge amount of format elements to process, which
> +-- creates a lot of string constants.
> +local NELEMENTS = 25000

Why 25000? It is reproduced with 10000 as well.


> +local fmt = ('%'):rep(NELEMENTS * 2)
> +local expected = ('%'):rep(NELEMENTS)
> +local result
> +for _ = 1, 4 do
> +  result = fmt:format()
> +end
> +
> +test:ok(true, 'no IR buffer underflow')
Why do you need this check? Why the following check it not enough?
> +test:is(result, expected, 'correct result')
> +
> +test:done(true)

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

Re: [Tarantool-patches] [PATCH luajit] Limit number of string format elements to compile.

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the review!
See my answers below.

On 04.09.24, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch!
> 
> See my comments below.
> 
> Sergey
> 
> On 26.08.2024 13:25, Sergey Kaplun wrote:
> > From: Mike Pall <mike>
> >
> > Reported by pwnhacker0x18.
> >
> > (cherry picked from commit 4fc48c50fe3f3f5a9680bada5c0c0d0d7eb345a3)
> >
> > When compiling `string.format()` with a huge sequence of elements, it is
> > possible that too many KGC IRs underflow the IR buffer. This patch
> > limits the maximum number of `string.format()` elements to be compiled
> > to 100.
> >
> > Sergey Kaplun:
> > * added the description and the test for the problem
> >
> > Part of tarantool/tarantool#10199
> > ---
> >
> > Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1203-limit-format-elements
> > Related issues:
> > *https://github.com/tarantool/tarantool/issues/10199
> > *https://github.com/LuaJIT/LuaJIT/issues/1203
> >
> >   src/lj_ffrecord.c                             |  2 ++
> >   .../lj-1203-limit-format-elements.test.lua    | 28 +++++++++++++++++++
> >   2 files changed, 30 insertions(+)
> >   create mode 100644 test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> >
> > diff --git a/src/lj_ffrecord.c b/src/lj_ffrecord.c
> > index d5fc081e..3b82d044 100644
> > --- a/src/lj_ffrecord.c
> > +++ b/src/lj_ffrecord.c
> > @@ -962,6 +962,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
> >     TRef hdr, tr;
> >     FormatState fs;
> >     SFormat sf;
> > +  int nfmt = 0;
> >     /* Specialize to the format string. */
> >     emitir(IRTG(IR_EQ, IRT_STR), trfmt, lj_ir_kstr(J, fmt));
> >     tr = hdr = recff_bufhdr(J);
> > @@ -1031,6 +1032,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
> >         recff_nyiu(J, rd);
> >         return;
> >       }
> > +    if (++nfmt > 100) lj_trace_err(J, LJ_TRERR_TRACEOV);
> >     }
> >     J->base[0] = emitir(IRT(IR_BUFSTR, IRT_STR), tr, hdr);
> >   }
> > diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> > new file mode 100644
> > index 00000000..f17d4e37
> > --- /dev/null
> > +++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> > @@ -0,0 +1,28 @@
> > +local tap = require('tap')
> > +
> > +-- Test file to demonstrate LuaJIT incorrect recording of
> > +-- `string.format()` function with huge amount of elements.
> > +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1173.
> Seems a correct link is https://github.com/LuaJIT/LuaJIT/issues/1203

Fixed, thanks!
Branch is force-pushed.

===================================================================
diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
index f17d4e37..bf250000 100644
--- a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
+++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
@@ -2,7 +2,7 @@ local tap = require('tap')
 
 -- Test file to demonstrate LuaJIT incorrect recording of
 -- `string.format()` function with huge amount of elements.
--- See also: https://github.com/LuaJIT/LuaJIT/issues/1173.
+-- See also: https://github.com/LuaJIT/LuaJIT/issues/1203.
 
 local test = tap.test('lj-1203-limit-format-elements'):skipcond({
   ['Test requires JIT enabled'] = not jit.status(),
===================================================================

> > +
> > +local test = tap.test('lj-1203-limit-format-elements'):skipcond({
> > +  ['Test requires JIT enabled'] = not jit.status(),
> > +})
> > +
> > +test:plan(2)
> > +
> > +jit.opt.start('hotloop=1')
> > +
> > +-- XXX: Use a huge amount of format elements to process, which
> > +-- creates a lot of string constants.
> > +local NELEMENTS = 25000
> 
> Why 25000? It is reproduced with 10000 as well.

It is flaky-reproducible with less amount inside our test suite (at
least on my laptop), so I prefer to keep this number of elements.

> 
> 
> > +local fmt = ('%'):rep(NELEMENTS * 2)
> > +local expected = ('%'):rep(NELEMENTS)
> > +local result
> > +for _ = 1, 4 do
> > +  result = fmt:format()
> > +end
> > +
> > +test:ok(true, 'no IR buffer underflow')
> Why do you need this check? Why the following check it not enough?

We usually check both (no assertion and correctness) where it is easily
done.

> > +test:is(result, expected, 'correct result')
> > +
> > +test:done(true)

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] Limit number of string format elements to compile.

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch and the fixes!
LGTM

Re: [Tarantool-patches] [PATCH luajit] Limit number of string format elements to compile.

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey!

Thanks for the fixes! LGTM

see my comments below

On 04.09.2024 18:02, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> See my answers below.
>
> On 04.09.24, Sergey Bronnikov wrote:
>> Hi, Sergey,
>>
>> thanks for the patch!
>>
>> See my comments below.
>>
>> Sergey
>>
>> On 26.08.2024 13:25, Sergey Kaplun wrote:
>>> From: Mike Pall <mike>
>>>
>>> Reported by pwnhacker0x18.
>>>
>>> (cherry picked from commit 4fc48c50fe3f3f5a9680bada5c0c0d0d7eb345a3)
>>>
>>> When compiling `string.format()` with a huge sequence of elements, it is
>>> possible that too many KGC IRs underflow the IR buffer. This patch
>>> limits the maximum number of `string.format()` elements to be compiled
>>> to 100.
>>>
>>> Sergey Kaplun:
>>> * added the description and the test for the problem
>>>
>>> Part of tarantool/tarantool#10199
>>> ---
>>>
>>> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1203-limit-format-elements
>>> Related issues:
>>> *https://github.com/tarantool/tarantool/issues/10199
>>> *https://github.com/LuaJIT/LuaJIT/issues/1203
>>>
>>>    src/lj_ffrecord.c                             |  2 ++
>>>    .../lj-1203-limit-format-elements.test.lua    | 28 +++++++++++++++++++
>>>    2 files changed, 30 insertions(+)
>>>    create mode 100644 test/tarantool-tests/lj-1203-limit-format-elements.test.lua
>>>
>>> diff --git a/src/lj_ffrecord.c b/src/lj_ffrecord.c
>>> index d5fc081e..3b82d044 100644
>>> --- a/src/lj_ffrecord.c
>>> +++ b/src/lj_ffrecord.c
>>> @@ -962,6 +962,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
>>>      TRef hdr, tr;
>>>      FormatState fs;
>>>      SFormat sf;
>>> +  int nfmt = 0;
>>>      /* Specialize to the format string. */
>>>      emitir(IRTG(IR_EQ, IRT_STR), trfmt, lj_ir_kstr(J, fmt));
>>>      tr = hdr = recff_bufhdr(J);
>>> @@ -1031,6 +1032,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
>>>          recff_nyiu(J, rd);
>>>          return;
>>>        }
>>> +    if (++nfmt > 100) lj_trace_err(J, LJ_TRERR_TRACEOV);
>>>      }
>>>      J->base[0] = emitir(IRT(IR_BUFSTR, IRT_STR), tr, hdr);
>>>    }
>>> diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
>>> new file mode 100644
>>> index 00000000..f17d4e37
>>> --- /dev/null
>>> +++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
>>> @@ -0,0 +1,28 @@
>>> +local tap = require('tap')
>>> +
>>> +-- Test file to demonstrate LuaJIT incorrect recording of
>>> +-- `string.format()` function with huge amount of elements.
>>> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1173.
>> Seems a correct link ishttps://github.com/LuaJIT/LuaJIT/issues/1203
> Fixed, thanks!
> Branch is force-pushed.
>
> ===================================================================
> diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> index f17d4e37..bf250000 100644
> --- a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> +++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> @@ -2,7 +2,7 @@ local tap = require('tap')
>   
>   -- Test file to demonstrate LuaJIT incorrect recording of
>   -- `string.format()` function with huge amount of elements.
> --- See also:https://github.com/LuaJIT/LuaJIT/issues/1173.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1203.
>   
>   local test = tap.test('lj-1203-limit-format-elements'):skipcond({
>     ['Test requires JIT enabled'] = not jit.status(),
> ===================================================================
Thanks!
>>> +
>>> +local test = tap.test('lj-1203-limit-format-elements'):skipcond({
>>> +  ['Test requires JIT enabled'] = not jit.status(),
>>> +})
>>> +
>>> +test:plan(2)
>>> +
>>> +jit.opt.start('hotloop=1')
>>> +
>>> +-- XXX: Use a huge amount of format elements to process, which
>>> +-- creates a lot of string constants.
>>> +local NELEMENTS = 25000
>> Why 25000? It is reproduced with 10000 as well.
> It is flaky-reproducible with less amount inside our test suite (at
> least on my laptop), so I prefer to keep this number of elements.
>
Okay
>>
>>> +local fmt = ('%'):rep(NELEMENTS * 2)
>>> +local expected = ('%'):rep(NELEMENTS)
>>> +local result
>>> +for _ = 1, 4 do
>>> +  result = fmt:format()
>>> +end
>>> +
>>> +test:ok(true, 'no IR buffer underflow')
>> Why do you need this check? Why the following check it not enough?
> We usually check both (no assertion and correctness) where it is easily
> done.

Looks like the first one is excessive and I would remove it.

But I'll not insist.

>
>>> +test:is(result, expected, 'correct result')
>>> +
>>> +test:done(true)

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

Re: [Tarantool-patches] [PATCH luajit] Limit number of string format elements to compile.

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Fixed your comments and force-pushed the branch.

On 05.09.24, Sergey Bronnikov wrote:
> Hi, Sergey!
> 
> Thanks for the fixes! LGTM
> 
> see my comments below
> 
> On 04.09.2024 18:02, Sergey Kaplun wrote:
> > Hi, Sergey!
> > Thanks for the review!
> > See my answers below.
> >
> > On 04.09.24, Sergey Bronnikov wrote:
> >> Hi, Sergey,
> >>
> >> thanks for the patch!
> >>
> >> See my comments below.
> >>
> >> Sergey
> >>
> >> On 26.08.2024 13:25, Sergey Kaplun wrote:
> >>> From: Mike Pall <mike>
> >>>
> >>> Reported by pwnhacker0x18.
> >>>
> >>> (cherry picked from commit 4fc48c50fe3f3f5a9680bada5c0c0d0d7eb345a3)
> >>>
> >>> When compiling `string.format()` with a huge sequence of elements, it is
> >>> possible that too many KGC IRs underflow the IR buffer. This patch
> >>> limits the maximum number of `string.format()` elements to be compiled
> >>> to 100.
> >>>
> >>> Sergey Kaplun:
> >>> * added the description and the test for the problem
> >>>
> >>> Part of tarantool/tarantool#10199
> >>> ---
> >>>
> >>> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1203-limit-format-elements
> >>> Related issues:
> >>> *https://github.com/tarantool/tarantool/issues/10199
> >>> *https://github.com/LuaJIT/LuaJIT/issues/1203
> >>>
> >>>    src/lj_ffrecord.c                             |  2 ++
> >>>    .../lj-1203-limit-format-elements.test.lua    | 28 +++++++++++++++++++
> >>>    2 files changed, 30 insertions(+)
> >>>    create mode 100644 test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> >>>
> >>> diff --git a/src/lj_ffrecord.c b/src/lj_ffrecord.c
> >>> index d5fc081e..3b82d044 100644
> >>> --- a/src/lj_ffrecord.c
> >>> +++ b/src/lj_ffrecord.c
> >>> @@ -962,6 +962,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
> >>>      TRef hdr, tr;
> >>>      FormatState fs;
> >>>      SFormat sf;
> >>> +  int nfmt = 0;
> >>>      /* Specialize to the format string. */
> >>>      emitir(IRTG(IR_EQ, IRT_STR), trfmt, lj_ir_kstr(J, fmt));
> >>>      tr = hdr = recff_bufhdr(J);
> >>> @@ -1031,6 +1032,7 @@ static void LJ_FASTCALL recff_string_format(jit_State *J, RecordFFData *rd)
> >>>          recff_nyiu(J, rd);
> >>>          return;
> >>>        }
> >>> +    if (++nfmt > 100) lj_trace_err(J, LJ_TRERR_TRACEOV);
> >>>      }
> >>>      J->base[0] = emitir(IRT(IR_BUFSTR, IRT_STR), tr, hdr);
> >>>    }
> >>> diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> >>> new file mode 100644
> >>> index 00000000..f17d4e37
> >>> --- /dev/null
> >>> +++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> >>> @@ -0,0 +1,28 @@
> >>> +local tap = require('tap')
> >>> +
> >>> +-- Test file to demonstrate LuaJIT incorrect recording of
> >>> +-- `string.format()` function with huge amount of elements.
> >>> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1173.
> >> Seems a correct link ishttps://github.com/LuaJIT/LuaJIT/issues/1203
> > Fixed, thanks!
> > Branch is force-pushed.
> >
> > ===================================================================
> > diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> > index f17d4e37..bf250000 100644
> > --- a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> > +++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
> > @@ -2,7 +2,7 @@ local tap = require('tap')
> >   
> >   -- Test file to demonstrate LuaJIT incorrect recording of
> >   -- `string.format()` function with huge amount of elements.
> > --- See also:https://github.com/LuaJIT/LuaJIT/issues/1173.
> > +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1203.
> >   
> >   local test = tap.test('lj-1203-limit-format-elements'):skipcond({
> >     ['Test requires JIT enabled'] = not jit.status(),
> > ===================================================================
> Thanks!
> >>> +
> >>> +local test = tap.test('lj-1203-limit-format-elements'):skipcond({
> >>> +  ['Test requires JIT enabled'] = not jit.status(),
> >>> +})
> >>> +
> >>> +test:plan(2)
> >>> +
> >>> +jit.opt.start('hotloop=1')
> >>> +
> >>> +-- XXX: Use a huge amount of format elements to process, which
> >>> +-- creates a lot of string constants.
> >>> +local NELEMENTS = 25000
> >> Why 25000? It is reproduced with 10000 as well.
> > It is flaky-reproducible with less amount inside our test suite (at
> > least on my laptop), so I prefer to keep this number of elements.
> >
> Okay
> >>
> >>> +local fmt = ('%'):rep(NELEMENTS * 2)
> >>> +local expected = ('%'):rep(NELEMENTS)
> >>> +local result
> >>> +for _ = 1, 4 do
> >>> +  result = fmt:format()
> >>> +end
> >>> +
> >>> +test:ok(true, 'no IR buffer underflow')
> >> Why do you need this check? Why the following check it not enough?
> > We usually check both (no assertion and correctness) where it is easily
> > done.
> 
> Looks like the first one is excessive and I would remove it.

Fair enough.
I've removed the first check.
See the iterative patch below.
Branch is force-pushed.

===================================================================
diff --git a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
index bf250000..d143b3fa 100644
--- a/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
+++ b/test/tarantool-tests/lj-1203-limit-format-elements.test.lua
@@ -8,7 +8,7 @@ local test = tap.test('lj-1203-limit-format-elements'):skipcond({
   ['Test requires JIT enabled'] = not jit.status(),
 })
 
-test:plan(2)
+test:plan(1)
 
 jit.opt.start('hotloop=1')
 
@@ -22,7 +22,6 @@ for _ = 1, 4 do
   result = fmt:format()
 end
 
-test:ok(true, 'no IR buffer underflow')
-test:is(result, expected, 'correct result')
+test:is(result, expected, 'no IR buffer underflow and the correct result')
 
 test:done(true)
===================================================================

> 
> But I'll not insist.
> 
> >
> >>> +test:is(result, expected, 'correct result')
> >>> +
> >>> +test:done(true)

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit] Limit number of string format elements to compile.

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey,

On 05.09.2024 20:14, Sergey Kaplun wrote:
> Hi, Sergey!
> Fixed your comments and force-pushed the branch.
Thanks, LGTM

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