[Tarantool-patches] [PATCH luajit 0/2] Fix LuaJIT tests for old libc versions

[Tarantool-patches] [PATCH luajit 0/2] Fix LuaJIT tests for old libc versions

[Sergey Kaplun via Tarantool-patches]

This patch set consists of 2 patches:
The first allows the use of tags with number values in the LuaJIT-tests
suite runner <test.lua>. This is required for the second patch -- it
skips the `strtod parsing` test in <tonumber_scan.lua> for libc versions
older than 2.19, due to the bug [1] in it.

Branch:
https://github.com/tarantool/luajit/tree/skaplun/fix-luajit-tests-centos7
PR in the Tarantool with tests enabled:
https://github.com/tarantool/tarantool/pull/10824

Only sysprof-related tests are failing, this will be fixed in the
separate patch-set.

[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=16151

Sergey Kaplun (2):
  test: support number value of tag in LuaJIT-tests
  test: fix LuaJIT-tests for old libc version

 test/CMakeLists.txt                          |  1 +
 test/LuaJIT-tests/CMakeLists.txt             |  9 +++++
 test/LuaJIT-tests/lib/base/tonumber_scan.lua |  4 ++-
 test/LuaJIT-tests/test.lua                   |  7 +++-
 test/cmake/GetLibCVersion.cmake              | 35 ++++++++++++++++++++
 5 files changed, 54 insertions(+), 2 deletions(-)
 create mode 100644 test/cmake/GetLibCVersion.cmake

-- 
2.47.0

[Tarantool-patches] [PATCH luajit 1/2] test: support number value of tag in LuaJIT-tests

[Sergey Kaplun via Tarantool-patches]

The LuaJIT-tests suite lacks the ability to specify tags with custom
values. This patch adds the ability to specify number tags in the format
`+tag=number`. It is useful for version specification of system
libraries (like libc).
---
 test/LuaJIT-tests/test.lua | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/test/LuaJIT-tests/test.lua b/test/LuaJIT-tests/test.lua
index ab178331..2a535605 100644
--- a/test/LuaJIT-tests/test.lua
+++ b/test/LuaJIT-tests/test.lua
@@ -161,7 +161,12 @@ local function parse_args(t)
         error(arg .." does not expect an argument")
       end
     elseif arg:find"^[-+]" then
-      opts.tags[arg:sub(2)] = (arg:sub(1, 1) == "+")
+      local tagval = arg:find"="
+      if tagval then
+        opts.tags[arg:sub(2, tagval - 1)] = tonumber(arg:sub(tagval + 1))
+      else
+        opts.tags[arg:sub(2)] = (arg:sub(1, 1) == "+")
+      end
     elseif arg:find"^%d+$" then
       if not opts.numbers_to_run then
         opts.numbers_to_run = {}
-- 
2.47.0

[Tarantool-patches] [PATCH luajit 2/2] test: fix LuaJIT-tests for old libc version

[Sergey Kaplun via Tarantool-patches]

The `strtod parsing` subtest in the <lib/base/tonumber_scan.lua> checks
the results yielded by the `strtod()` via FFI call. In versions before
2.19 it returns an incorrect result for "0x1p-2075" [1]. This patch
skips this test for a smaller version of the libc installed.

[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=16151
---
 test/CMakeLists.txt                          |  1 +
 test/LuaJIT-tests/CMakeLists.txt             |  9 +++++
 test/LuaJIT-tests/lib/base/tonumber_scan.lua |  4 ++-
 test/cmake/GetLibCVersion.cmake              | 35 ++++++++++++++++++++
 4 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 test/cmake/GetLibCVersion.cmake

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 0db2dd8b..be02d2e4 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -73,6 +73,7 @@ separate_arguments(LUAJIT_TEST_COMMAND)
 
 set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
 include(AddTestLib)
+include(GetLibCVersion)
 include(LibRealPath)
 
 # CTEST_FLAGS is used by CMake targets in test suites.
diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt
index 019762e0..d0771e14 100644
--- a/test/LuaJIT-tests/CMakeLists.txt
+++ b/test/LuaJIT-tests/CMakeLists.txt
@@ -62,6 +62,15 @@ if(CMAKE_C_FLAGS MATCHES "-march=skylake-avx512")
   list(APPEND LUAJIT_TEST_TAGS_EXTRA +avx512)
 endif()
 
+if(NOT CMAKE_SYSTEM_NAME STREQUAL "Darwin")
+  GetLibCVersion(LIBC_VERSION)
+  # XXX: <tonumber_scan.lua> uses `strtod()`, old versions of
+  # which have the bug [1] for "0x1p-2075" parsing. Add the skip
+  # check for it.
+  # [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=16151
+  list(APPEND LUAJIT_TEST_TAGS_EXTRA +libc=${LIBC_VERSION})
+endif()
+
 set(TEST_SUITE_NAME "LuaJIT-tests")
 
 # XXX: The call produces both test and target <LuaJIT-tests-deps>
diff --git a/test/LuaJIT-tests/lib/base/tonumber_scan.lua b/test/LuaJIT-tests/lib/base/tonumber_scan.lua
index e2dcd4d0..ac7d68a4 100644
--- a/test/LuaJIT-tests/lib/base/tonumber_scan.lua
+++ b/test/LuaJIT-tests/lib/base/tonumber_scan.lua
@@ -186,7 +186,9 @@ do --- tonumber parsing
   test_conv(tonumber)
 end
 
-do --- strtod parsing
+-- Skip for the old libc version with the bug in the `strtod()`.
+-- See also https://sourceware.org/bugzilla/show_bug.cgi?id=16151.
+do --- strtod parsing -libc<2.19
   test_conv(function(s)
     local d = ffi.C.strtod(s, e)
     return (e[0][0] == 0 and #s ~= 0) and d or nil
diff --git a/test/cmake/GetLibCVersion.cmake b/test/cmake/GetLibCVersion.cmake
new file mode 100644
index 00000000..fd05fa6d
--- /dev/null
+++ b/test/cmake/GetLibCVersion.cmake
@@ -0,0 +1,35 @@
+# Get the libc version installed in the system.
+# XXX: uses `LibRealPath`, so unsupported for OSX.
+macro(GetLibCVersion output)
+  LibRealPath(libcpath "libc.so.6")
+
+  # Get the version from the library name.
+  if(libcpath MATCHES "libc-([0-9]+\.[0-9]+)\.so")
+    set(${output} ${CMAKE_MATCH_1})
+  else()
+    # Try to directly run the library and parse the version from
+    # the output message.
+    execute_process(
+      COMMAND ${libcpath}
+      OUTPUT_VARIABLE LIB_C_INFO
+      ERROR_VARIABLE ERROR_MSG
+      OUTPUT_STRIP_TRAILING_WHITESPACE
+      RESULT_VARIABLE RES
+    )
+
+    if(NOT RES EQUAL 0)
+      message(FATAL_ERROR "Executing '${libcpath}' has failed: '${ERROR_MSG}'")
+    endif()
+
+    if(LIB_C_INFO MATCHES "^.*stable release version ([0-9]+\.[0-9]+)")
+      set(${output} ${CMAKE_MATCH_1})
+    else()
+      message(FATAL_ERROR "Can't determine libc version")
+    endif()
+
+    unset(RES)
+    unset(ERROR_MSG)
+    unset(LIB_C_INFO)
+  endif()
+  unset(CMAKE_MATCH_1)
+endmacro()
-- 
2.47.0

Re: [Tarantool-patches] [PATCH luajit 1/2] test: support number value of tag in LuaJIT-tests

[Sergey Bronnikov via Tarantool-patches]

Hi, Sergey,

thanks for the patch!
LGTM with minor comment below.

On 16:42 Thu 05 Dec , Sergey Kaplun wrote:
>The LuaJIT-tests suite lacks the ability to specify tags with custom
>values. This patch adds the ability to specify number tags in the format
>`+tag=number`. It is useful for version specification of system
>libraries (like libc).

Usually, for such janitorial patches we add a note like
"Required by the following patch".

>---
> test/LuaJIT-tests/test.lua | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/test/LuaJIT-tests/test.lua b/test/LuaJIT-tests/test.lua
>index ab178331..2a535605 100644
>--- a/test/LuaJIT-tests/test.lua
>+++ b/test/LuaJIT-tests/test.lua
>@@ -161,7 +161,12 @@ local function parse_args(t)
>         error(arg .." does not expect an argument")
>       end
>     elseif arg:find"^[-+]" then
>-      opts.tags[arg:sub(2)] = (arg:sub(1, 1) == "+")
>+      local tagval = arg:find"="
>+      if tagval then
>+        opts.tags[arg:sub(2, tagval - 1)] = tonumber(arg:sub(tagval + 1))

I would also check a type here:

--- a/test/LuaJIT-tests/test.lua
+++ b/test/LuaJIT-tests/test.lua
@@ -162,7 +162,7 @@ local function parse_args(t)
        end
      elseif arg:find"^[-+]" then
        local tagval = arg:find"="
-      if tagval then
+      if tagval and type(tagval) == 'number' then
          opts.tags[arg:sub(2, tagval - 1)] = tonumber(arg:sub(tagval + 1))
        else
          opts.tags[arg:sub(2)] = (arg:sub(1, 1) == "+")

>+      else
>+        opts.tags[arg:sub(2)] = (arg:sub(1, 1) == "+")
>+      end
>     elseif arg:find"^%d+$" then
>       if not opts.numbers_to_run then
>         opts.numbers_to_run = {}
>-- 
>2.47.0
>

-- 
sergeyb@

Re: [Tarantool-patches] [PATCH luajit 1/2] test: support number value of tag in LuaJIT-tests

[Sergey Kaplun via Tarantool-patches]

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

On 06.12.24, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch!
> LGTM with minor comment below.
> 
> On 16:42 Thu 05 Dec , Sergey Kaplun wrote:
> >The LuaJIT-tests suite lacks the ability to specify tags with custom
> >values. This patch adds the ability to specify number tags in the format
> >`+tag=number`. It is useful for version specification of system
> >libraries (like libc).
> 
> Usually, for such janitorial patches we add a note like
> "Required by the following patch".

Added the following line:
| Required for the next patch.

> 
> >---
> > test/LuaJIT-tests/test.lua | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> >diff --git a/test/LuaJIT-tests/test.lua b/test/LuaJIT-tests/test.lua
> >index ab178331..2a535605 100644
> >--- a/test/LuaJIT-tests/test.lua
> >+++ b/test/LuaJIT-tests/test.lua
> >@@ -161,7 +161,12 @@ local function parse_args(t)
> >         error(arg .." does not expect an argument")
> >       end
> >     elseif arg:find"^[-+]" then
> >-      opts.tags[arg:sub(2)] = (arg:sub(1, 1) == "+")
> >+      local tagval = arg:find"="
> >+      if tagval then
> >+        opts.tags[arg:sub(2, tagval - 1)] = tonumber(arg:sub(tagval + 1))
> 
> I would also check a type here:
> 
> --- a/test/LuaJIT-tests/test.lua
> +++ b/test/LuaJIT-tests/test.lua
> @@ -162,7 +162,7 @@ local function parse_args(t)
>         end
>       elseif arg:find"^[-+]" then
>         local tagval = arg:find"="
> -      if tagval then
> +      if tagval and type(tagval) == 'number' then
>           opts.tags[arg:sub(2, tagval - 1)] = tonumber(arg:sub(tagval + 1))
>         else
>           opts.tags[arg:sub(2)] = (arg:sub(1, 1) == "+")

This check is excess, see the `string.find()` documentation [1]:
| If it finds a match, then `find` returns the indices of `s` where this
| occurrence starts and ends; otherwise, it returns `nil`.

We have only two options here: a number or `nil`, so this `if` check is
just enough.

> 
> >+      else
> >+        opts.tags[arg:sub(2)] = (arg:sub(1, 1) == "+")
> >+      end
> >     elseif arg:find"^%d+$" then
> >       if not opts.numbers_to_run then
> >         opts.numbers_to_run = {}
> >-- 
> >2.47.0
> >
> 
> -- 
> sergeyb@

[1]: https://www.lua.org/manual/5.1/manual.html#pdf-string.find

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH luajit 2/2] test: fix LuaJIT-tests for old libc version

[Sergey Bronnikov via Tarantool-patches]

Hi, Sergey,

thanks for the patch!


On 16:42 Thu 05 Dec , Sergey Kaplun wrote:
>The `strtod parsing` subtest in the <lib/base/tonumber_scan.lua> checks
>the results yielded by the `strtod()` via FFI call. In versions before

s/versions/GLibc versions/

>2.19 it returns an incorrect result for "0x1p-2075" [1]. This patch
>skips this test for a smaller version of the libc installed.
>
>[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=16151
>---
> test/CMakeLists.txt                          |  1 +
> test/LuaJIT-tests/CMakeLists.txt             |  9 +++++
> test/LuaJIT-tests/lib/base/tonumber_scan.lua |  4 ++-
> test/cmake/GetLibCVersion.cmake              | 35 ++++++++++++++++++++
> 4 files changed, 48 insertions(+), 1 deletion(-)
> create mode 100644 test/cmake/GetLibCVersion.cmake
>
>diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
>index 0db2dd8b..be02d2e4 100644
>--- a/test/CMakeLists.txt
>+++ b/test/CMakeLists.txt
>@@ -73,6 +73,7 @@ separate_arguments(LUAJIT_TEST_COMMAND)
>
> set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
> include(AddTestLib)
>+include(GetLibCVersion)
> include(LibRealPath)
>
> # CTEST_FLAGS is used by CMake targets in test suites.
>diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt
>index 019762e0..d0771e14 100644
>--- a/test/LuaJIT-tests/CMakeLists.txt
>+++ b/test/LuaJIT-tests/CMakeLists.txt
>@@ -62,6 +62,15 @@ if(CMAKE_C_FLAGS MATCHES "-march=skylake-avx512")
>   list(APPEND LUAJIT_TEST_TAGS_EXTRA +avx512)
> endif()
>
>+if(NOT CMAKE_SYSTEM_NAME STREQUAL "Darwin")
>+  GetLibCVersion(LIBC_VERSION)
>+  # XXX: <tonumber_scan.lua> uses `strtod()`, old versions of
>+  # which have the bug [1] for "0x1p-2075" parsing. Add the skip
>+  # check for it.
>+  # [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=16151
>+  list(APPEND LUAJIT_TEST_TAGS_EXTRA +libc=${LIBC_VERSION})
>+endif()
>+
> set(TEST_SUITE_NAME "LuaJIT-tests")
>
> # XXX: The call produces both test and target <LuaJIT-tests-deps>
>diff --git a/test/LuaJIT-tests/lib/base/tonumber_scan.lua b/test/LuaJIT-tests/lib/base/tonumber_scan.lua
>index e2dcd4d0..ac7d68a4 100644
>--- a/test/LuaJIT-tests/lib/base/tonumber_scan.lua
>+++ b/test/LuaJIT-tests/lib/base/tonumber_scan.lua
>@@ -186,7 +186,9 @@ do --- tonumber parsing
>   test_conv(tonumber)
> end
>
>-do --- strtod parsing
>+-- Skip for the old libc version with the bug in the `strtod()`.
>+-- See also https://sourceware.org/bugzilla/show_bug.cgi?id=16151.
>+do --- strtod parsing -libc<2.19
>   test_conv(function(s)
>     local d = ffi.C.strtod(s, e)
>     return (e[0][0] == 0 and #s ~= 0) and d or nil
>diff --git a/test/cmake/GetLibCVersion.cmake b/test/cmake/GetLibCVersion.cmake
>new file mode 100644
>index 00000000..fd05fa6d
>--- /dev/null
>+++ b/test/cmake/GetLibCVersion.cmake
>@@ -0,0 +1,35 @@
>+# Get the libc version installed in the system.
>+# XXX: uses `LibRealPath`, so unsupported for OSX.
>+macro(GetLibCVersion output)
>+  LibRealPath(libcpath "libc.so.6")

I don't get why shared lib filename is hardcoded.
Version (6) can be changed and our test will fail.

We can use gcc:

[1] ~/sources/MRG/tarantool/third_party/luajit $ gcc -Wl,--trace main.c  | grep libc
/usr/lib/gcc/x86_64-linux-gnu/11/../../../x86_64-linux-gnu/libc.so
/lib/x86_64-linux-gnu/libc.so.6
/usr/lib/x86_64-linux-gnu/libc_nonshared.a
/usr/lib/x86_64-linux-gnu/libc_nonshared.a

or ldd:

[1] ~/sources/MRG/tarantool/third_party/luajit $ ldd build/src/luajit
         linux-vdso.so.1 (0x00007ffd161fd000)
         libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f272b6ac000)
         libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f272b68c000)
         libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f272b400000)
         /lib64/ld-linux-x86-64.so.2 (0x00007f272b88a000)

>+
>+  # Get the version from the library name.
>+  if(libcpath MATCHES "libc-([0-9]+\.[0-9]+)\.so")
>+    set(${output} ${CMAKE_MATCH_1})
>+  else()
>+    # Try to directly run the library and parse the version from
>+    # the output message.
>+    execute_process(
>+      COMMAND ${libcpath}
>+      OUTPUT_VARIABLE LIB_C_INFO
>+      ERROR_VARIABLE ERROR_MSG
>+      OUTPUT_STRIP_TRAILING_WHITESPACE
>+      RESULT_VARIABLE RES
>+    )
>+
>+    if(NOT RES EQUAL 0)
>+      message(FATAL_ERROR "Executing '${libcpath}' has failed: '${ERROR_MSG}'")
>+    endif()
>+
>+    if(LIB_C_INFO MATCHES "^.*stable release version ([0-9]+\.[0-9]+)")
>+      set(${output} ${CMAKE_MATCH_1})
>+    else()
>+      message(FATAL_ERROR "Can't determine libc version")
>+    endif()
>+
>+    unset(RES)
>+    unset(ERROR_MSG)
>+    unset(LIB_C_INFO)
>+  endif()
>+  unset(CMAKE_MATCH_1)
>+endmacro()
>-- 
>2.47.0
>

-- 
sergeyb@

Re: [Tarantool-patches] [PATCH luajit 1/2] test: support number value of tag in LuaJIT-tests

[Sergey Bronnikov via Tarantool-patches]

Hi, Sergey,

thanks! LGTM

On 14:17 Fri 06 Dec , Sergey Kaplun via Tarantool-patches wrote:

Re: [Tarantool-patches] [PATCH luajit 1/2] test: support number value of tag in LuaJIT-tests

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM

Re: [Tarantool-patches] [PATCH luajit 2/2] test: fix LuaJIT-tests for old libc version

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM