Re: [Tarantool-patches] [PATCH luajit v2 0/5] Add ASan support in LuaJIT

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

Pals, thanks for your reviews!

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

On 21.07.23, Igor Munkin wrote:
> This series implements the second attempt to enable ASan support back
> for LuaJIT repository.
> 
> The only thing that was missing the whole time was the option per se
> (and the corresponding compile flags, obviously). Anyway, when the
> option has been finally added, the dedicated CI workflow has been added
> too in scope of the second patch in the series. All other info can be
> found in the corresponding patches.
> 
> Besided, internal LuaJIT memory allocator is not instrumented yet
> unfortunately, so to find any memory faults it's better to build LuaJIT
> with system provided memory allocator (i.e. run CMake configuration
> phase with -DLUAJIT_USE_SYSMALLOC=ON). However, LUAJIT_USE_SYSMALLOC
> cannot be enabled on x64 without GC64, since realloc usually doesn't
> return addresses in the right address range. For more info, see root
> CMakeLists.txt.
> 
> Surprisingly, some hidden bugs were found while testing the
> aforementioned configuration.
> 
> 1. The assertions in memprof initialization, checking the state of the
> allocator against NULL, can fail if this allocator requires no internal
> state (e.g. glibc functions for allocating dynamic memory). In fact,
> when building LuaJIT with LUAJIT_USE_SYSMALLOC option enabled, NULL is
> given as the second parameter to <lua_newstate> and these assertions
> fail as a result. Hence, they are simply removed.
> 
> 2. Before the patch all tests in tarantool-tests suite (except
> <lj-603-err-snap-restore.test.lua>) terminate their execution via
> <os.exit> with the status depending on the test results. However, the
> second argument of <os.exit> was omitted and Lua universe was not
> properly finalized as a result. This behaviour becomes a problem, when
> LuaJIT is build with LUAJIT_USE_SYSMALLOC option and AddressSanitizer
> support enabled, since the sanitizer starts reporting false positive
> errors about the memory allocations without the corresponding memory
> releases. To resolve these errors, the second parameter to each
> <os.exit> call terminating the test has to be added. To avoid loss of
> the aforementioned parameter in future, <test:done> helper has been
> added to the TAP module. Depending on the single parameter, the new
> helper either properly finalize the test being run, or simply checks all
> the test assertions and raises an error if any of them fail. The latter
> case is added especially to handle <lj-603-err-snap-restore.test.lua>
> specifics and still check that everything works fine.
> 
> 
> Last but not least: for all ARM64 jobs in exotic builds testing pipeline
> non-GC64 configurations were disabled, since LUAJIT_ENABLE_GC64 takes no
> effect for this arch (GC64 is the only option).
> 
> Issue: https://github.com/tarantool/tarantool/issues/5878
> Branch: https://github.com/tarantool/luajit/tree/hackaton/gh-5878-enable-ASAN
> Tarantool related changes and CI can be found in #8846[1].
> 
> v1: https://lists.tarantool.org/tarantool-patches/cover.1689195028.git.imun@tarantool.org/T/#t
> 
> Changes in v2:
>   * Fixed comments as per review by Sergey B. and Sergey K
>   * Enabled LUAJIT_USE_SYSMALLOC option in sanitizers-testing.yml
>   * Removed two invalid assertions in memprof sources (found via
>     enabling LUAJIT_USE_SYSMALLOC option)
>   * Introduced test:done helper for proper test finalization (found via
>     enabling both LUAJIT_USE_ASAN and LUAJIT_USE_SYSMALLOC)
>   * Little maintenance of exotic builds workflow
> 
> Igor Munkin (5):
>   ci: clean up workflow for exotic builds
>   memprof: remove invalid assertions
>   test: introduce test:done TAP helper
>   build: introduce LUAJIT_USE_ASAN option
>   ci: introduce testing workflow with sanitizers
> 

<snipped>

> 
> [1]: https://github.com/tarantool/tarantool/pull/8846
> 
> -- 
> 2.30.2
> 

-- 
Best regards,
IM