Re: [Tarantool-patches] [PATCH luajit] Fix limit check in narrow_conv_backprop().

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey!
Thanks for the review!

On 17.09.24, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for tha patch! LGTM with a minor comment below.
> 
> On 26.08.2024 15:37, Sergey Kaplun wrote:
> > From: Mike Pall <mike>

<snipped>

> > +-- XXX: Test fails only under ASAN.
> > +-- XXX: The original reproducer was found by fuzzer:
> > +--https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=70779.
> > +-- It creates a long side trace with a huge amount of ADD IRs,
> > +-- which are recursively used in the `narrow_conv_backprop()` many
> > +-- enough times to catch the stack-buffer-overflow. I can't
> > +-- simplify the reproducer any more (or write it from scratch), so
> > +-- I leave it in that state.
> > +
> 
> I would remove a sentence "I can't simplify the reproducer ..."
> 
> from the test, because it is useful information on review and
> 
> I think it is useless for those who will use the test.

I do not agree -- it is kind of worning to prevent changing the test.
Also, we have something similar for gh-4199-gc64-fuse, for example.

> 
> Feel free to ignore and leave as is.

So, ignoring for now.

> 
> 
> s/any more/anymore/

I decided to drop any, so the comment is the following now:
===================================================================
diff --git a/test/tarantool-tests/lj-1262-fix-limit-narrow-conv-backprop.test.lua b/test/tarantool-tests/lj-1262-fix-limit-narrow-conv-backprop.test.lua
index 6bb4025d..c225fa14 100644
--- a/test/tarantool-tests/lj-1262-fix-limit-narrow-conv-backprop.test.lua
+++ b/test/tarantool-tests/lj-1262-fix-limit-narrow-conv-backprop.test.lua
@@ -15,9 +15,9 @@ test:plan(1)
 -- https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=70779.
 -- It creates a long side trace with a huge amount of ADD IRs,
 -- which are recursively used in the `narrow_conv_backprop()` many
--- enough times to catch the stack-buffer-overflow. I can't
--- simplify the reproducer any more (or write it from scratch), so
--- I leave it in that state.
+-- enough times to catch the stack-buffer-overflow. I can't more
+-- simplify the reproducer any (or write it from scratch), so I
+-- leave it in that state.
 
 local DEFAULT_NUMBER = 1
 local tonumber = tonumber
===================================================================

<snipped>

-- 
Best regards,
Sergey Kaplun