Re: [Tarantool-patches] [PATCH v2 luajit 00/26] More tests from LuaJIT-tests, part 1

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

Hi, Sergey!
Thanks for the review!

On 01.02.24, Sergey Bronnikov wrote:
> Hi, Sergey
> 
> there is a problem with running under ASAN:
> 
> 
> cmake -S . -B build -DLUAJIT_USE_ASAN=ON
> 
> <snipped>
> 
> cmake --build build/ -t LuaJIT-tests
> 
> <snipped>
> 
> [100%] Linking C executable luajit
> [100%] Built target luajit_static
> Running LuaJIT-tests
> ==320737==ASan runtime does not come first in initial library list; you 
> should either link runtime to your application or manually preload it 
> with LD_PRELOAD.
> gmake[3]: *** 
> [test/LuaJIT-tests/CMakeFiles/LuaJIT-tests.dir/build.make:74: 
> LuaJIT-tests] Error 1
> gmake[2]: *** [CMakeFiles/Makefile2:1193: 
> test/LuaJIT-tests/CMakeFiles/LuaJIT-tests.dir/all] Error 2
> gmake[1]: *** [CMakeFiles/Makefile2:1200: 
> test/LuaJIT-tests/CMakeFiles/LuaJIT-tests.dir/rule] Error 2
> gmake: *** [Makefile:455: LuaJIT-tests] Error 2
> 
> cannot reproduce on tarantool/master

Good catch!
I've been tricked by the CMake again...

The new patch is the following:

===================================================================
diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt
index 96981e32..e91e3d4a 100644
--- a/test/LuaJIT-tests/CMakeLists.txt
+++ b/test/LuaJIT-tests/CMakeLists.txt
@@ -24,6 +24,39 @@ else()
   list(APPEND LUAJIT_TESTS_ENV LD_LIBRARY_PATH="${LD_LIBRARY_PATH}")
 endif()
 
+if(LUAJIT_USE_ASAN)
+  # When running LuaJIT-tests under ASAN, the internal ASAN check
+  # failed:
+  # AddressSanitizer: CHECK failed: asan_interceptors.cpp:356
+  # "((__interception::real___cxa_throw)) != (0)" (0x0, 0x0)
+  # This is a workaround suggested at
+  # https://github.com/google/sanitizers/issues/934.
+  macro(LibRealPath output lib)
+    execute_process(
+      COMMAND ${CMAKE_CXX_COMPILER} -print-file-name=${lib}
+      OUTPUT_VARIABLE LIB_LINK
+      OUTPUT_STRIP_TRAILING_WHITESPACE
+    )
+    # Fortunately, we are not interested in macOS here, so we can
+    # use realpath.
+    execute_process(
+      COMMAND realpath ${LIB_LINK}
+      OUTPUT_VARIABLE ${output}
+      OUTPUT_STRIP_TRAILING_WHITESPACE
+    )
+  endmacro()
+  LibRealPath(LIB_STDCPP libstdc++.so)
+  # XXX: GCC requires both. Clang requires only libstdc++.
+  if(CMAKE_C_COMPILER_ID STREQUAL "GNU")
+    LibRealPath(LIB_ASAN libasan.so)
+    # XXX: Don't use " " (separator in LD_PRELOAD) in `list()`.
+    # ";" will expand to " " as we want.
+    list(APPEND LUAJIT_TESTS_ENV LD_PRELOAD="${LIB_ASAN};${LIB_STDCPP}")
+  else()
+    list(APPEND LUAJIT_TESTS_ENV LD_PRELOAD="${LIB_STDCPP}")
+  endif()
+endif()
+
 add_custom_command(TARGET LuaJIT-tests
   COMMENT "Running LuaJIT-tests"
   COMMAND
===================================================================

> 
> 
> On 1/29/24 13:45, Sergey Kaplun wrote:
> > This patchset enables most of the LuaJIT-tests related to FFI and
> > testing of C libraries (cpptest, ctest). More tests (from the <misc>
> > directory) will be enabled in the future series.
> >
> > FFI tests from the <unportable> directory aren't enabled because of their
> > inconsistency, see also [1].
> >
> > This increases coverage up to 92.5%, see here [2].
> >
> > Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-9398-more-luajit-tests
> > Tarantool PR: https://github.com/tarantool/tarantool/pull/9603
> > Related issues:
> > * https://github.com/tarantool/tarantool/issues/9398
> > * https://github.com/tarantool/tarantool/issues/7834
> >
> > [1]: https://github.com/LuaJIT/LuaJIT/pull/415
> > [2]: https://coveralls.io/github/tarantool/luajit?branch=skaplun/gh-9398-more-luajit-tests
> >
> > Changes in the v2:
> > * Introduce the AddTestLib macro to avoid copy-pasting. It only adds a
> >    target with necessary flags without setting any variables.
> > * <cpptest.cpp> is renamed to <libcpptest.cpp> for consistency with
> >    <libctest.c>
> > * Rename some tests with uninformative naming (like `call_10i` ->
> >    `10 int args`)
> > * Add adjusments of arguments for long calls (~10 arguments) to make them
> >    comparable without changing test semantics.
> > * Fix some typos
> >
> > Sergey Kaplun (26):
> >    cmake: introduce AddTestLib macro
> >    test: prepare lauxilarily libs for LuaJIT-tests
> >    test: separate LuaJIT helpers from ffi_util.inc
> >    test: enable <ffi_arith_ptr.lua> in LuaJIT-tests
> >    test: enable <ffi_bitfield.lua> in LuaJIT-tests
> >    test: enable <ffi_call.lua> in LuaJIT-tests
> >    test: enable <ffi_callback.lua> in LuaJIT-tests
> >    test: enable <ffi_const.lua> in LuaJIT-tests
> >    test: enable <ffi_convert.lua> in LuaJIT-tests
> >    test: enable <ffi_enum.lua> in LuaJIT-tests
> >    test: enable <ffi_gcstep_recursive.lua>
> >    test: enable <ffi_jit_arith.lua> in LuaJIT-tests
> >    test: enable <ffi_jit_call.lua> in LuaJIT-tests
> >    test: enable <ffi_jit_conv.lua> in LuaJIT-tests
> >    test: enable <ffi_lex_number.lua> in LuaJIT-tests
> >    test: enable <ffi_metatype.lua> in LuaJIT-tests
> >    test: enable <ffi_new.lua> in LuaJIT-tests
> >    test: enable <ffi_parse_array.lua> in LuaJIT-tests
> >    test: enable <ffi_parse_basic.lua> in LuaJIT-tests
> >    test: enable <ffi_parse_cdef.lua> in LuaJIT-tests
> >    test: enable <ffi_parse_struct.lua> LuaJIT test
> >    test: enable <ffi_tabov.lua> LuaJIT test
> >    test: enable <lightud.lua> LuaJIT test
> >    test: enable <api_call.lua> LuaJIT test
> >    test: enable <catch_wrap.lua> LuaJIT test
> >    test: enable <catch_cpp.lua> LuaJIT test
> >
> >   test/CMakeLists.txt                           |   3 +
> >   test/LuaJIT-tests/CMakeLists.txt              |  45 ++-
> >   test/LuaJIT-tests/common/fails.lua            |   3 +
> >   test/LuaJIT-tests/common/ffi/checkfail.lua    |  14 +
> >   test/LuaJIT-tests/common/ffi/checktypes.lua   |  15 +
> >   test/LuaJIT-tests/common/ffi_util.inc         |  23 --
> >   test/LuaJIT-tests/{misc => lang}/api_call.lua |  32 +-
> >   .../{sysdep => lang}/catch_cpp.lua            |  36 +-
> >   .../{misc => lang}/catch_wrap.lua             |  29 +-
> >   test/LuaJIT-tests/lang/index                  |   4 +
> >   test/LuaJIT-tests/lang/lightud.lua            |  89 +++++
> >   test/LuaJIT-tests/lib/ffi/ffi_arith_ptr.lua   |  13 +-
> >   test/LuaJIT-tests/lib/ffi/ffi_bitfield.lua    |   4 +-
> >   test/LuaJIT-tests/lib/ffi/ffi_call.lua        | 185 +++++-----
> >   test/LuaJIT-tests/lib/ffi/ffi_callback.lua    |  45 +--
> >   test/LuaJIT-tests/lib/ffi/ffi_const.lua       |   4 +-
> >   test/LuaJIT-tests/lib/ffi/ffi_convert.lua     |  35 +-
> >   test/LuaJIT-tests/lib/ffi/ffi_enum.lua        |  11 +-
> >   .../lib/ffi/ffi_gcstep_recursive.lua          |  27 +-
> >   test/LuaJIT-tests/lib/ffi/ffi_jit_arith.lua   |  22 +-
> >   test/LuaJIT-tests/lib/ffi/ffi_jit_call.lua    |  70 ++--
> >   test/LuaJIT-tests/lib/ffi/ffi_jit_conv.lua    |  70 ++--
> >   test/LuaJIT-tests/lib/ffi/ffi_lex_number.lua  |  72 ++--
> >   test/LuaJIT-tests/lib/ffi/ffi_metatype.lua    |  16 +-
> >   test/LuaJIT-tests/lib/ffi/ffi_new.lua         |  50 +--
> >   test/LuaJIT-tests/lib/ffi/ffi_parse_array.lua | 106 +++---
> >   test/LuaJIT-tests/lib/ffi/ffi_parse_basic.lua | 251 ++++++-------
> >   test/LuaJIT-tests/lib/ffi/ffi_parse_cdef.lua  | 141 ++++----
> >   .../LuaJIT-tests/lib/ffi/ffi_parse_struct.lua | 335 +++++++++---------
> >   test/LuaJIT-tests/lib/ffi/ffi_tabov.lua       |  12 -
> >   test/LuaJIT-tests/lib/ffi/index               |  18 +
> >   test/LuaJIT-tests/misc/lightud.lua            |  88 -----
> >   test/LuaJIT-tests/src/CMakeLists.txt          |  17 +
> >   .../src/{cpptest.cpp => libcpptest.cpp}       |   4 +-
> >   test/LuaJIT-tests/src/{ctest.c => libctest.c} |   4 +-
> >   test/LuaJIT-tests/test.lua                    |   7 +-
> >   .../PUC-Rio-Lua-5.1-tests/libs/CMakeLists.txt |  25 +-
> >   test/cmake/AddTestLib.cmake                   |  28 ++
> >   test/tarantool-tests/CMakeLists.txt           |  27 +-
> >   test/tarantool-tests/ffi-tabov.test.lua       |  27 ++
> >   40 files changed, 1086 insertions(+), 921 deletions(-)
> >   create mode 100644 test/LuaJIT-tests/common/fails.lua
> >   create mode 100644 test/LuaJIT-tests/common/ffi/checkfail.lua
> >   create mode 100644 test/LuaJIT-tests/common/ffi/checktypes.lua
> >   rename test/LuaJIT-tests/{misc => lang}/api_call.lua (79%)
> >   rename test/LuaJIT-tests/{sysdep => lang}/catch_cpp.lua (75%)
> >   rename test/LuaJIT-tests/{misc => lang}/catch_wrap.lua (71%)
> >   create mode 100644 test/LuaJIT-tests/lang/lightud.lua
> >   delete mode 100644 test/LuaJIT-tests/lib/ffi/ffi_tabov.lua
> >   delete mode 100644 test/LuaJIT-tests/misc/lightud.lua
> >   create mode 100644 test/LuaJIT-tests/src/CMakeLists.txt
> >   rename test/LuaJIT-tests/src/{cpptest.cpp => libcpptest.cpp} (96%)
> >   rename test/LuaJIT-tests/src/{ctest.c => libctest.c} (99%)
> >   create mode 100644 test/cmake/AddTestLib.cmake
> >   create mode 100644 test/tarantool-tests/ffi-tabov.test.lua
> >

-- 
Best regards,
Sergey Kaplun