Re: [Tarantool-patches] [PATCH luajit v2 4/5] build: introduce LUAJIT_USE_ASAN option

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Sergey,

Thanks for your review! See my answers below.

On 24.07.23, Sergey Bronnikov wrote:
> Thanks for the patch! See comments below.
> 
> 
> On 7/21/23 11:12, Igor Munkin wrote:

<snipped>

> > +# ASan enabled.
> > +option(LUAJIT_USE_ASAN "Build LuaJIT with AddressSanitizer" OFF)
> > +if(LUAJIT_USE_ASAN)
> > +  if(NOT LUAJIT_USE_SYSMALLOC)
> > +    message(WARNING
> > +      "Unfortunately, internal LuaJIT memory allocator is not instrumented yet,"
> > +      " so to find any memory errors it's better to build LuaJIT with system"
> > +      " provided memory allocator (i.e. run CMake configuration phase with"
> > +      " -DLUAJIT_USE_SYSMALLOC=ON)."
> > +    )
> > +  endif()
> > +  # Use all recomendations described in AddressSanitize docs:
> 
> typo: recomendations -> recommendations

Fixed, thanks!

> 

<snipped>

> 
> With applied patch tests below failed:

My bad: I've made inaccurate rebase on the bleeding master and forgotten
to update tests added by Max and Sergey. See the iterative diff:

================================================================================

diff --git a/test/tarantool-tests/lj-1024-varg-maxslot.test.lua b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
index e33ddba2..81fb1eea 100644
--- a/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
+++ b/test/tarantool-tests/lj-1024-varg-maxslot.test.lua
@@ -60,4 +60,4 @@ end
 
 test:ok(true, 'BC_VARG recording with VARG slots defined on trace')
 
-os.exit(test:check() and 0 or 1)
+test:done(true)
diff --git a/test/tarantool-tests/lj-128-fix-union-init.test.lua b/test/tarantool-tests/lj-128-fix-union-init.test.lua
index 93cbf3af..b36758fc 100644
--- a/test/tarantool-tests/lj-128-fix-union-init.test.lua
+++ b/test/tarantool-tests/lj-128-fix-union-init.test.lua
@@ -55,4 +55,4 @@ for i = 1, NITERATIONS do
   test:ok(union_type(i).u == i, 'first member init only')
 end
 
-os.exit(test:check() and 0 or 1)
+test:done(true)

================================================================================

Thanks for manual testing this, since it was covered by another bug in
CI (see the explanation in the next patch).

> 

<snipped>

> 

-- 
Best regards,
IM