* [Tarantool-patches] [PATCH v3 luajit 1/2] cmake: run tests with Valgrind
2024-09-18 8:50 [Tarantool-patches] [PATCH v3 luajit 0/2] Enable running tests with Valgrind, add CI Valgrind testing workflow mandesero--- via Tarantool-patches
@ 2024-09-18 8:50 ` mandesero--- via Tarantool-patches
2024-11-01 14:05 ` Sergey Bronnikov via Tarantool-patches
2024-09-18 8:50 ` [Tarantool-patches] [PATCH v3 luajit 2/2] ci: add Valgrind testing workflow mandesero--- via Tarantool-patches
2024-11-01 12:51 ` [Tarantool-patches] [PATCH v3 luajit 0/2] Enable running tests with Valgrind, add CI " Sergey Bronnikov via Tarantool-patches
2 siblings, 1 reply; 10+ messages in thread
From: mandesero--- via Tarantool-patches @ 2024-09-18 8:50 UTC (permalink / raw)
To: tarantool-patches, skaplun, m.kokryashkin; +Cc: Maksim Tiushev
From: Maksim Tiushev <mandesero@gmail.com>
This patch enables running tests with Valgrind. Custom Valgrind testing
options can be configured by setting the `VALGRIND_OPTIONS` variable.
Please note that this environment variable must be set before building,
and any updates to it will require a project rebuild. By default, the
suppression file is set to `src/lj.supp`.
Also this patch disables the following tests when running with Valgrind
due to failures:
For SIGPROF:
- test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
- test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
- test/tarantool-tests/lj-726-profile-flush-close.test.lua
- test/tarantool-tests/misclib-sysprof-lapi.test.lua
For TIMEOUT:
- test/tarantool-tests/gh-7745-oom-on-trace.test.lua
- test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
---
CMakeLists.txt | 5 +++++
test/CMakeLists.txt | 16 ++++++++++++++++
test/tarantool-tests/CMakeLists.txt | 3 ++-
...7264-add-proto-trace-sysprof-default.test.lua | 1 +
.../gh-7745-oom-on-trace.test.lua | 1 +
.../lj-1034-tabov-error-frame.test.lua | 1 +
.../lj-512-profiler-hook-finalizers.test.lua | 4 +++-
.../lj-726-profile-flush-close.test.lua | 4 +++-
.../misclib-sysprof-lapi.test.lua | 1 +
9 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index aa2103b3..854b3613 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -289,6 +289,11 @@ endif()
# ASan enabled.
option(LUAJIT_USE_ASAN "Build LuaJIT with AddressSanitizer" OFF)
if(LUAJIT_USE_ASAN)
+ if(LUAJIT_USE_VALGRIND)
+ message(FATAL_ERROR
+ "AddressSanitizer and Valgrind cannot be used simultaneously."
+ )
+ endif()
if(NOT LUAJIT_USE_SYSMALLOC)
message(WARNING
"Unfortunately, internal LuaJIT memory allocator is not instrumented yet,"
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 0db2dd8b..3cc0ddbc 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -69,6 +69,22 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS
)
set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
+
+if(LUAJIT_USE_VALGRIND)
+ if (NOT LUAJIT_USE_SYSMALLOC)
+ message(WARNING
+ "LUAJIT_USE_SYSMALLOC option is mandatory for Valgrind's memcheck tool"
+ " on x64 and the only way to get useful results from it for all other"
+ " architectures.")
+ endif()
+
+ find_program(VALGRIND valgrind)
+ set(LUAJIT_TEST_VALGRIND_OPTS
+ "--suppressions=${LUAJIT_SOURCE_DIR}/lj.supp $ENV{VALGRIND_OPTIONS}")
+ set(LUAJIT_TEST_COMMAND
+ "${VALGRIND} ${LUAJIT_TEST_VALGRIND_OPTS} ${LUAJIT_TEST_COMMAND}")
+endif()
+
separate_arguments(LUAJIT_TEST_COMMAND)
set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index e3750bf3..2becebb7 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -130,7 +130,8 @@ foreach(test_path ${tests})
# LUA_CPATH and LD_LIBRARY_PATH variables and also
# dependencies list with libraries are set in scope of
# BuildTestLib macro.
- ENVIRONMENT "LUA_PATH=${LUA_PATH};LUA_CPATH=${LUA_CPATH};${LUA_TEST_ENV_MORE}"
+ ENVIRONMENT "LUA_PATH=${LUA_PATH};LUA_CPATH=${LUA_CPATH};${LUA_TEST_ENV_MORE};\
+LJ_USE_VALGRIND=${LUAJIT_USE_VALGRIND}"
LABELS ${TEST_SUITE_NAME}
DEPENDS tarantool-tests-deps
)
diff --git a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
index c1d68e3c..f700b414 100644
--- a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
+++ b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
@@ -4,6 +4,7 @@ local test = tap.test('gh-7264-add-proto-trace-sysprof-default'):skipcond({
['Sysprof is implemented for x86_64 only'] = jit.arch ~= 'x86' and
jit.arch ~= 'x64',
['Sysprof is implemented for Linux only'] = jit.os ~= 'Linux',
+ ['Disabled with Valgrind (SIGPROF)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
})
test:plan(2)
diff --git a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
index fe320251..b4ab8872 100644
--- a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
+++ b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
@@ -6,6 +6,7 @@ local test = tap.test('OOM on trace'):skipcond({
(jit.os == 'OSX'),
['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
['Test requires JIT enabled'] = not jit.status(),
+ ['Disabled with Valgrind (Timeout)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
})
test:plan(1)
diff --git a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
index 0e23fdb2..5e75973b 100644
--- a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
+++ b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
@@ -4,6 +4,7 @@ local test = tap.test('lj-1034-tabov-error-frame'):skipcond({
['Test requires JIT enabled'] = not jit.status(),
['Test requires GC64 mode enabled'] = not ffi.abi('gc64'),
['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
+ ['Disabled with Valgrind (Timeout)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
})
-- XXX: The test for the problem uses the table of GC
diff --git a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
index f02bd05f..fc2f43b7 100644
--- a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
+++ b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
@@ -1,7 +1,9 @@
local tap = require('tap')
local profile = require('jit.profile')
-local test = tap.test('lj-512-profiler-hook-finalizers')
+local test = tap.test('lj-512-profiler-hook-finalizers'):skipcond({
+ ['Disabled with Valgrind (SIGPROF)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
+})
test:plan(1)
-- Sampling interval in ms.
diff --git a/test/tarantool-tests/lj-726-profile-flush-close.test.lua b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
index 36cca43d..6d09658f 100644
--- a/test/tarantool-tests/lj-726-profile-flush-close.test.lua
+++ b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
@@ -1,6 +1,8 @@
local tap = require('tap')
-local test = tap.test('lj-726-profile-flush-close')
+local test = tap.test('lj-726-profile-flush-close'):skipcond({
+ ['Disabled with Valgrind (SIGPROF)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
+})
test:plan(1)
local TEST_FILE = 'lj-726-profile-flush-close.profile'
diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
index fdaed46a..c5e2516b 100644
--- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
@@ -3,6 +3,7 @@ local test = tap.test("misc-sysprof-lapi"):skipcond({
["Sysprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
jit.arch ~= "x64",
["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
+ ['Disabled with Valgrind (SIGPROF)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
})
test:plan(19)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Tarantool-patches] [PATCH v3 luajit 1/2] cmake: run tests with Valgrind
2024-09-18 8:50 ` [Tarantool-patches] [PATCH v3 luajit 1/2] cmake: run tests with Valgrind mandesero--- via Tarantool-patches
@ 2024-11-01 14:05 ` Sergey Bronnikov via Tarantool-patches
2024-11-11 11:03 ` mandesero--- via Tarantool-patches
0 siblings, 1 reply; 10+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-11-01 14:05 UTC (permalink / raw)
To: mandesero, tarantool-patches, skaplun, m.kokryashkin
[-- Attachment #1: Type: text/plain, Size: 8463 bytes --]
Hi, Maxim!
thanks for the patch!
On 18.09.2024 11:50, mandesero--- via Tarantool-patches wrote:
> From: Maksim Tiushev<mandesero@gmail.com>
>
> This patch enables running tests with Valgrind. Custom Valgrind testing
> options can be configured by setting the `VALGRIND_OPTIONS` variable.
> Please note that this environment variable must be set before building,
> and any updates to it will require a project rebuild. By default, the
> suppression file is set to `src/lj.supp`.
>
> Also this patch disables the following tests when running with Valgrind
> due to failures:
>
> For SIGPROF:
s/SIGPROF/sysprof/?
> - test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> - test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
> - test/tarantool-tests/lj-726-profile-flush-close.test.lua
> - test/tarantool-tests/misclib-sysprof-lapi.test.lua
>
> For TIMEOUT:
It is not clear what do you mean with "for TIMEOUT".
Whether these tests timed out due to running under Valgrind or not?
> - test/tarantool-tests/gh-7745-oom-on-trace.test.lua
> - test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
I think we need followup issues for these tests and put a link to the
issue to the
commit message. Otherwise, these tests will remain unfixed. It is
desirable to
describe details of failures under Valgrind in that issue.
> ---
> CMakeLists.txt | 5 +++++
> test/CMakeLists.txt | 16 ++++++++++++++++
> test/tarantool-tests/CMakeLists.txt | 3 ++-
> ...7264-add-proto-trace-sysprof-default.test.lua | 1 +
> .../gh-7745-oom-on-trace.test.lua | 1 +
> .../lj-1034-tabov-error-frame.test.lua | 1 +
> .../lj-512-profiler-hook-finalizers.test.lua | 4 +++-
> .../lj-726-profile-flush-close.test.lua | 4 +++-
> .../misclib-sysprof-lapi.test.lua | 1 +
> 9 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index aa2103b3..854b3613 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -289,6 +289,11 @@ endif()
> # ASan enabled.
> option(LUAJIT_USE_ASAN "Build LuaJIT with AddressSanitizer" OFF)
> if(LUAJIT_USE_ASAN)
> + if(LUAJIT_USE_VALGRIND)
> + message(FATAL_ERROR
> + "AddressSanitizer and Valgrind cannot be used simultaneously."
> + )
> + endif()
> if(NOT LUAJIT_USE_SYSMALLOC)
> message(WARNING
> "Unfortunately, internal LuaJIT memory allocator is not instrumented yet,"
> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> index 0db2dd8b..3cc0ddbc 100644
> --- a/test/CMakeLists.txt
> +++ b/test/CMakeLists.txt
> @@ -69,6 +69,22 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS
> )
>
> set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
> +
> +if(LUAJIT_USE_VALGRIND)
> + if (NOT LUAJIT_USE_SYSMALLOC)
> + message(WARNING
> + "LUAJIT_USE_SYSMALLOC option is mandatory for Valgrind's memcheck tool"
> + " on x64 and the only way to get useful results from it for all other"
> + " architectures.")
> + endif()
> +
> + find_program(VALGRIND valgrind)
> + set(LUAJIT_TEST_VALGRIND_OPTS
> + "--suppressions=${LUAJIT_SOURCE_DIR}/lj.supp $ENV{VALGRIND_OPTIONS}")
> + set(LUAJIT_TEST_COMMAND
> + "${VALGRIND} ${LUAJIT_TEST_VALGRIND_OPTS} ${LUAJIT_TEST_COMMAND}")
> +endif()
> +
> separate_arguments(LUAJIT_TEST_COMMAND)
>
> set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index e3750bf3..2becebb7 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -130,7 +130,8 @@ foreach(test_path ${tests})
> # LUA_CPATH and LD_LIBRARY_PATH variables and also
> # dependencies list with libraries are set in scope of
> # BuildTestLib macro.
> - ENVIRONMENT "LUA_PATH=${LUA_PATH};LUA_CPATH=${LUA_CPATH};${LUA_TEST_ENV_MORE}"
> + ENVIRONMENT "LUA_PATH=${LUA_PATH};LUA_CPATH=${LUA_CPATH};${LUA_TEST_ENV_MORE};\
> +LJ_USE_VALGRIND=${LUAJIT_USE_VALGRIND}"
> LABELS ${TEST_SUITE_NAME}
> DEPENDS tarantool-tests-deps
> )
> diff --git a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> index c1d68e3c..f700b414 100644
> --- a/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> +++ b/test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
> @@ -4,6 +4,7 @@ local test = tap.test('gh-7264-add-proto-trace-sysprof-default'):skipcond({
> ['Sysprof is implemented for x86_64 only'] = jit.arch ~= 'x86' and
> jit.arch ~= 'x64',
> ['Sysprof is implemented for Linux only'] = jit.os ~= 'Linux',
> + ['Disabled with Valgrind (SIGPROF)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
" (SIGPROF)" looks useless, please remove. Here and below.
I would add a number of the issue instead.
> })
>
> test:plan(2)
> diff --git a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
> index fe320251..b4ab8872 100644
> --- a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
> +++ b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
> @@ -6,6 +6,7 @@ local test = tap.test('OOM on trace'):skipcond({
> (jit.os == 'OSX'),
> ['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
> ['Test requires JIT enabled'] = not jit.status(),
> + ['Disabled with Valgrind (Timeout)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
> })
>
> test:plan(1)
> diff --git a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
> index 0e23fdb2..5e75973b 100644
> --- a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
> +++ b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
> @@ -4,6 +4,7 @@ local test = tap.test('lj-1034-tabov-error-frame'):skipcond({
> ['Test requires JIT enabled'] = not jit.status(),
> ['Test requires GC64 mode enabled'] = not ffi.abi('gc64'),
> ['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
> + ['Disabled with Valgrind (Timeout)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
> })
>
> -- XXX: The test for the problem uses the table of GC
> diff --git a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
> index f02bd05f..fc2f43b7 100644
> --- a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
> +++ b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
> @@ -1,7 +1,9 @@
> local tap = require('tap')
> local profile = require('jit.profile')
>
> -local test = tap.test('lj-512-profiler-hook-finalizers')
> +local test = tap.test('lj-512-profiler-hook-finalizers'):skipcond({
> + ['Disabled with Valgrind (SIGPROF)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
> +})
> test:plan(1)
>
> -- Sampling interval in ms.
> diff --git a/test/tarantool-tests/lj-726-profile-flush-close.test.lua b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
> index 36cca43d..6d09658f 100644
> --- a/test/tarantool-tests/lj-726-profile-flush-close.test.lua
> +++ b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
> @@ -1,6 +1,8 @@
> local tap = require('tap')
>
> -local test = tap.test('lj-726-profile-flush-close')
> +local test = tap.test('lj-726-profile-flush-close'):skipcond({
> + ['Disabled with Valgrind (SIGPROF)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
> +})
> test:plan(1)
>
> local TEST_FILE = 'lj-726-profile-flush-close.profile'
> diff --git a/test/tarantool-tests/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> index fdaed46a..c5e2516b 100644
> --- a/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-sysprof-lapi.test.lua
> @@ -3,6 +3,7 @@ local test = tap.test("misc-sysprof-lapi"):skipcond({
> ["Sysprof is implemented for x86_64 only"] = jit.arch ~= "x86" and
> jit.arch ~= "x64",
> ["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
> + ['Disabled with Valgrind (SIGPROF)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
> })
>
> test:plan(19)
[-- Attachment #2: Type: text/html, Size: 9387 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* [Tarantool-patches] [PATCH v3 luajit 1/2] cmake: run tests with Valgrind
2024-11-01 14:05 ` Sergey Bronnikov via Tarantool-patches
@ 2024-11-11 11:03 ` mandesero--- via Tarantool-patches
2024-11-14 14:04 ` Sergey Kaplun via Tarantool-patches
0 siblings, 1 reply; 10+ messages in thread
From: mandesero--- via Tarantool-patches @ 2024-11-11 11:03 UTC (permalink / raw)
To: tarantool-patches; +Cc: mandesero
From: Maksim Tiushev <mandesero@gmail.com>
This patch enables running tests with Valgrind. Custom Valgrind testing
options can be configured by setting the `VALGRIND_OPTIONS` variable.
Please note that this environment variable must be set before building,
and any updates to it will require a project rebuild. By default, the
suppression file is set to `src/lj.supp`.
Also this patch disables the following tests when running with Valgrind
due to failures:
Disabled due #10803:
- test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
- test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
- test/tarantool-tests/lj-726-profile-flush-close.test.lua
- test/tarantool-tests/misclib-sysprof-lapi.test.lua
Timed out due to running under Valgrind:
- test/tarantool-tests/gh-7745-oom-on-trace.test.lua
- test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
Part of #3705
---
CMakeLists.txt | 5 +++++
test/CMakeLists.txt | 16 ++++++++++++++++
test/tarantool-tests/CMakeLists.txt | 3 ++-
.../gh-7745-oom-on-trace.test.lua | 1 +
.../lj-1034-tabov-error-frame.test.lua | 1 +
.../lj-512-profiler-hook-finalizers.test.lua | 5 ++++-
.../lj-726-profile-flush-close.test.lua | 5 ++++-
...7264-add-proto-trace-sysprof-default.test.lua | 2 ++
.../profilers/misclib-sysprof-lapi.test.lua | 2 ++
9 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index aa2103b3..854b3613 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -289,6 +289,11 @@ endif()
# ASan enabled.
option(LUAJIT_USE_ASAN "Build LuaJIT with AddressSanitizer" OFF)
if(LUAJIT_USE_ASAN)
+ if(LUAJIT_USE_VALGRIND)
+ message(FATAL_ERROR
+ "AddressSanitizer and Valgrind cannot be used simultaneously."
+ )
+ endif()
if(NOT LUAJIT_USE_SYSMALLOC)
message(WARNING
"Unfortunately, internal LuaJIT memory allocator is not instrumented yet,"
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 0db2dd8b..3cc0ddbc 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -69,6 +69,22 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS
)
set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
+
+if(LUAJIT_USE_VALGRIND)
+ if (NOT LUAJIT_USE_SYSMALLOC)
+ message(WARNING
+ "LUAJIT_USE_SYSMALLOC option is mandatory for Valgrind's memcheck tool"
+ " on x64 and the only way to get useful results from it for all other"
+ " architectures.")
+ endif()
+
+ find_program(VALGRIND valgrind)
+ set(LUAJIT_TEST_VALGRIND_OPTS
+ "--suppressions=${LUAJIT_SOURCE_DIR}/lj.supp $ENV{VALGRIND_OPTIONS}")
+ set(LUAJIT_TEST_COMMAND
+ "${VALGRIND} ${LUAJIT_TEST_VALGRIND_OPTS} ${LUAJIT_TEST_COMMAND}")
+endif()
+
separate_arguments(LUAJIT_TEST_COMMAND)
set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 8bdb2cf3..39562f35 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -110,7 +110,8 @@ foreach(test_path ${tests})
# LUA_CPATH and LD_LIBRARY_PATH variables and also
# dependencies list with libraries are set in scope of
# BuildTestLib macro.
- ENVIRONMENT "LUA_PATH=${LUA_PATH};${LUA_TEST_ENV_MORE}"
+ ENVIRONMENT "LUA_PATH=${LUA_PATH};${LUA_TEST_ENV_MORE};\
+LJ_USE_VALGRIND=${LUAJIT_USE_VALGRIND}"
LABELS ${TEST_SUITE_NAME}
DEPENDS tarantool-tests-deps
)
diff --git a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
index fe320251..b4ab8872 100644
--- a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
+++ b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
@@ -6,6 +6,7 @@ local test = tap.test('OOM on trace'):skipcond({
(jit.os == 'OSX'),
['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
['Test requires JIT enabled'] = not jit.status(),
+ ['Disabled with Valgrind (Timeout)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
})
test:plan(1)
diff --git a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
index 0e23fdb2..5e75973b 100644
--- a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
+++ b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
@@ -4,6 +4,7 @@ local test = tap.test('lj-1034-tabov-error-frame'):skipcond({
['Test requires JIT enabled'] = not jit.status(),
['Test requires GC64 mode enabled'] = not ffi.abi('gc64'),
['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
+ ['Disabled with Valgrind (Timeout)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
})
-- XXX: The test for the problem uses the table of GC
diff --git a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
index f02bd05f..8f32b9ef 100644
--- a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
+++ b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
@@ -1,7 +1,10 @@
local tap = require('tap')
local profile = require('jit.profile')
-local test = tap.test('lj-512-profiler-hook-finalizers')
+local test = tap.test('lj-512-profiler-hook-finalizers'):skipcond({
+ -- See also https://github.com/tarantool/tarantool/issues/10803
+ ['Disabled due to #10803'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
+})
test:plan(1)
-- Sampling interval in ms.
diff --git a/test/tarantool-tests/lj-726-profile-flush-close.test.lua b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
index 36cca43d..27eb3751 100644
--- a/test/tarantool-tests/lj-726-profile-flush-close.test.lua
+++ b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
@@ -1,6 +1,9 @@
local tap = require('tap')
-local test = tap.test('lj-726-profile-flush-close')
+local test = tap.test('lj-726-profile-flush-close'):skipcond({
+ -- See also https://github.com/tarantool/tarantool/issues/10803
+ ['Disabled due to #10803'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
+})
test:plan(1)
local TEST_FILE = 'lj-726-profile-flush-close.profile'
diff --git a/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
index 176c1a15..e6c853a1 100644
--- a/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
+++ b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
@@ -6,6 +6,8 @@ local test = tap.test('gh-7264-add-proto-trace-sysprof-default'):skipcond({
['Sysprof is implemented for Linux only'] = jit.os ~= 'Linux',
-- See also https://github.com/LuaJIT/LuaJIT/issues/606.
['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
+ -- See also https://github.com/tarantool/tarantool/issues/10803
+ ['Disabled due to #10803'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
})
test:plan(2)
diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
index 26c277cd..2210d668 100644
--- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
+++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
@@ -5,6 +5,8 @@ local test = tap.test("misc-sysprof-lapi"):skipcond({
["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
-- See also https://github.com/LuaJIT/LuaJIT/issues/606.
["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
+ -- See also https://github.com/tarantool/tarantool/issues/10803
+ ['Disabled due to #10803'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
})
test:plan(19)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Tarantool-patches] [PATCH v3 luajit 1/2] cmake: run tests with Valgrind
2024-11-11 11:03 ` mandesero--- via Tarantool-patches
@ 2024-11-14 14:04 ` Sergey Kaplun via Tarantool-patches
2024-11-14 15:54 ` Sergey Kaplun via Tarantool-patches
0 siblings, 1 reply; 10+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-11-14 14:04 UTC (permalink / raw)
To: mandesero; +Cc: tarantool-patches
Hi, Maksim!
Thanks for the patch!
Please consider my comments below.
On 11.11.24, mandesero@gmail.com wrote:
> From: Maksim Tiushev <mandesero@gmail.com>
>
> This patch enables running tests with Valgrind. Custom Valgrind testing
> options can be configured by setting the `VALGRIND_OPTIONS` variable.
> Please note that this environment variable must be set before building,
> and any updates to it will require a project rebuild. By default, the
> suppression file is set to `src/lj.supp`.
There is a `VALGRIND_OPTS` variable [1] that we can set -- it makes the
usage of valgrind more flexible -- we can define any necessary flags in
the command line (not at the building stage).
>
> Also this patch disables the following tests when running with Valgrind
Typo: s/Also/Also,/
> due to failures:
>
> Disabled due #10803:
Typo: s<#10803><tarantool/tarantool#10803>
> - test/tarantool-tests/gh-7264-add-proto-trace-sysprof-default.test.lua
Nit: Commit line width is more than 72 symbols.
> - test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
> - test/tarantool-tests/lj-726-profile-flush-close.test.lua
> - test/tarantool-tests/misclib-sysprof-lapi.test.lua
>
> Timed out due to running under Valgrind:
> - test/tarantool-tests/gh-7745-oom-on-trace.test.lua
> - test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
>
> Part of #3705
Typo: s<#3705><tarantool/tarantool#3705>
> ---
> CMakeLists.txt | 5 +++++
> test/CMakeLists.txt | 16 ++++++++++++++++
> test/tarantool-tests/CMakeLists.txt | 3 ++-
> .../gh-7745-oom-on-trace.test.lua | 1 +
> .../lj-1034-tabov-error-frame.test.lua | 1 +
> .../lj-512-profiler-hook-finalizers.test.lua | 5 ++++-
> .../lj-726-profile-flush-close.test.lua | 5 ++++-
> ...7264-add-proto-trace-sysprof-default.test.lua | 2 ++
> .../profilers/misclib-sysprof-lapi.test.lua | 2 ++
> 9 files changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index aa2103b3..854b3613 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
<snipped>
> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> index 0db2dd8b..3cc0ddbc 100644
> --- a/test/CMakeLists.txt
> +++ b/test/CMakeLists.txt
> @@ -69,6 +69,22 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS
> )
>
> set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
> +
> +if(LUAJIT_USE_VALGRIND)
> + if (NOT LUAJIT_USE_SYSMALLOC)
> + message(WARNING
> + "LUAJIT_USE_SYSMALLOC option is mandatory for Valgrind's memcheck tool"
> + " on x64 and the only way to get useful results from it for all other"
> + " architectures.")
> + endif()
> +
> + find_program(VALGRIND valgrind)
> + set(LUAJIT_TEST_VALGRIND_OPTS
> + "--suppressions=${LUAJIT_SOURCE_DIR}/lj.supp $ENV{VALGRIND_OPTIONS}")
I suppose that we can drop $ENV{VALGRIND_OPTIONS}, since there is a
`VALGRIND_OPTS` variable [1] that we can set -- it makes the usage of
valgrind more flexible -- we can define any necessary flags in the
command line (not at the building stage).
Therefore, `LUAJIT_TEST_VALGRIND_OPTS` may be renamed to
`LUAJIT_TEST_VALGRIND_SUPP`.
> + set(LUAJIT_TEST_COMMAND
> + "${VALGRIND} ${LUAJIT_TEST_VALGRIND_OPTS} ${LUAJIT_TEST_COMMAND}")
> +endif()
> +
> separate_arguments(LUAJIT_TEST_COMMAND)
>
> set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 8bdb2cf3..39562f35 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -110,7 +110,8 @@ foreach(test_path ${tests})
> # LUA_CPATH and LD_LIBRARY_PATH variables and also
> # dependencies list with libraries are set in scope of
> # BuildTestLib macro.
> - ENVIRONMENT "LUA_PATH=${LUA_PATH};${LUA_TEST_ENV_MORE}"
> + ENVIRONMENT "LUA_PATH=${LUA_PATH};${LUA_TEST_ENV_MORE};\
> +LJ_USE_VALGRIND=${LUAJIT_USE_VALGRIND}"
It is better to set this variable to the `LUA_TEST_ENV_MORE` list.
Also, I suggest rename it to the `LUAJIT_TEST_USE_VALGRIND`.
> LABELS ${TEST_SUITE_NAME}
> DEPENDS tarantool-tests-deps
> )
> diff --git a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
> index fe320251..b4ab8872 100644
> --- a/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
> +++ b/test/tarantool-tests/gh-7745-oom-on-trace.test.lua
> @@ -6,6 +6,7 @@ local test = tap.test('OOM on trace'):skipcond({
> (jit.os == 'OSX'),
> ['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
> ['Test requires JIT enabled'] = not jit.status(),
> + ['Disabled with Valgrind (Timeout)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
Minor: there is no need to check the custom value.
If the variable isn't set `os.getenv()` returns `nil`.
> })
>
> test:plan(1)
> diff --git a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
> index 0e23fdb2..5e75973b 100644
> --- a/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
> +++ b/test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
> @@ -4,6 +4,7 @@ local test = tap.test('lj-1034-tabov-error-frame'):skipcond({
> ['Test requires JIT enabled'] = not jit.status(),
> ['Test requires GC64 mode enabled'] = not ffi.abi('gc64'),
> ['Disabled on MacOS due to #8652'] = jit.os == 'OSX',
> + ['Disabled with Valgrind (Timeout)'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
Minor: there is no need to check the custom value.
If the variable isn't set `os.getenv()` returns `nil`.
> })
>
> -- XXX: The test for the problem uses the table of GC
> diff --git a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
> index f02bd05f..8f32b9ef 100644
> --- a/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
> +++ b/test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
> @@ -1,7 +1,10 @@
> local tap = require('tap')
> local profile = require('jit.profile')
>
> -local test = tap.test('lj-512-profiler-hook-finalizers')
> +local test = tap.test('lj-512-profiler-hook-finalizers'):skipcond({
> + -- See also https://github.com/tarantool/tarantool/issues/10803
Nit: Missed dot at the end of the sentence.
> + ['Disabled due to #10803'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
Minor: there is no need to check the custom value.
If the variable isn't set `os.getenv()` returns `nil`.
> +})
> test:plan(1)
>
> -- Sampling interval in ms.
> diff --git a/test/tarantool-tests/lj-726-profile-flush-close.test.lua b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
> index 36cca43d..27eb3751 100644
> --- a/test/tarantool-tests/lj-726-profile-flush-close.test.lua
> +++ b/test/tarantool-tests/lj-726-profile-flush-close.test.lua
> @@ -1,6 +1,9 @@
> local tap = require('tap')
>
> -local test = tap.test('lj-726-profile-flush-close')
> +local test = tap.test('lj-726-profile-flush-close'):skipcond({
> + -- See also https://github.com/tarantool/tarantool/issues/10803
Nit: Missed dot at the end of the sentence.
> + ['Disabled due to #10803'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
Minor: there is no need to check the custom value.
If the variable isn't set `os.getenv()` returns `nil`.
> +})
> test:plan(1)
>
> local TEST_FILE = 'lj-726-profile-flush-close.profile'
> diff --git a/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
> index 176c1a15..e6c853a1 100644
> --- a/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
> +++ b/test/tarantool-tests/profilers/gh-7264-add-proto-trace-sysprof-default.test.lua
> @@ -6,6 +6,8 @@ local test = tap.test('gh-7264-add-proto-trace-sysprof-default'):skipcond({
> ['Sysprof is implemented for Linux only'] = jit.os ~= 'Linux',
> -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
> ['Disabled due to LuaJIT/LuaJIT#606'] = os.getenv('LUAJIT_TABLE_BUMP'),
> + -- See also https://github.com/tarantool/tarantool/issues/10803
Nit: Missed dot at the end of the sentence.
> + ['Disabled due to #10803'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
Minor: there is no need to check the custom value.
If the variable isn't set `os.getenv()` returns `nil`.
> })
>
> test:plan(2)
> diff --git a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> index 26c277cd..2210d668 100644
> --- a/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> +++ b/test/tarantool-tests/profilers/misclib-sysprof-lapi.test.lua
> @@ -5,6 +5,8 @@ local test = tap.test("misc-sysprof-lapi"):skipcond({
> ["Sysprof is implemented for Linux only"] = jit.os ~= "Linux",
> -- See also https://github.com/LuaJIT/LuaJIT/issues/606.
> ["Disabled due to LuaJIT/LuaJIT#606"] = os.getenv("LUAJIT_TABLE_BUMP"),
>
Nit: Missed dot at the end of the sentence.
+ -- See also https://github.com/tarantool/tarantool/issues/10803
> + ['Disabled due to #10803'] = os.getenv("LJ_USE_VALGRIND") == 'ON',
Minor: there is no need to check the custom value.
If the variable isn't set `os.getenv()` returns `nil`.
> })
>
> test:plan(19)
> --
> 2.34.1
>
[1]: https://valgrind.org/docs/manual/manual-core.html#manual-core.defopts
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Tarantool-patches] [PATCH v3 luajit 1/2] cmake: run tests with Valgrind
2024-11-14 14:04 ` Sergey Kaplun via Tarantool-patches
@ 2024-11-14 15:54 ` Sergey Kaplun via Tarantool-patches
0 siblings, 0 replies; 10+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2024-11-14 15:54 UTC (permalink / raw)
To: mandesero, tarantool-patches
On 14.11.24, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Maksim!
> Thanks for the patch!
> Please consider my comments below.
>
> On 11.11.24, mandesero@gmail.com wrote:
<snipped>
> > - test/tarantool-tests/lj-512-profiler-hook-finalizers.test.lua
> > - test/tarantool-tests/lj-726-profile-flush-close.test.lua
> > - test/tarantool-tests/misclib-sysprof-lapi.test.lua
I also get the following error locally:
| test/tarantool-tests/profilers/gh-5688-tool-cli-flag.test.lua (SIGPROF)
So, it looks like all sysprof's tests should be disabled.
> >
> > Timed out due to running under Valgrind:
> > - test/tarantool-tests/gh-7745-oom-on-trace.test.lua
> > - test/tarantool-tests/lj-1034-tabov-error-frame.test.lua
> >
Also, I've found that tests from LuaJIT-tests suite reports tons of the
assertions, but still the suite passes. This should be fixed:
I suppose that we need to set `--error-exitcode` option as well.
The option `--exit-on-first-error` may be also considered.
| ctest --verbose -L LuaJIT-tests
| ...
| 2: ==20374== Command: /home/burii/reviews/luajit/valgrind-ci/src/luajit -e dofile[[/home/burii/reviews/luajit/valgrind-ci/test/luajit-test-init.lua]] /home/burii/reviews/luajit/valgrind-ci/test/LuaJIT-tests/test.lua +slow +ffi +bit +jit
| 2: ==20374==
| 2: ==20374== Invalid read of size 4
| 2: ==20374== at 0x137059: lj_getu32 (lj_def.h:244)
Same for PUC-Rio-Lua-5.1-tests, lua-Harness-test.
Also, I see these warnings for tarantool-tests suite as well, see
| ctest --verbose -R test/tarantool-tests/fix-jit-dump-ir-conv.test.lua
for example:
| 82: ==25641== Conditional jump or move depends on uninitialised value(s)
| 82: ==25641== at 0x1BCD8F: lj_BC_ADDVN (/home/burii/reviews/luajit/valgrind-ci/src/lj_vm.S:252)
| 82: ==25641== by 0x1445F3: lj_vmevent_call (lj_vmevent.c:46)
| 82: ==25641== by 0x186EB5: trace_stop (lj_trace.c:555)
| 82: ==25641== by 0x184422: trace_state (lj_trace.c:726)
| 82: ==25641== by 0x1BE97D: lj_vm_cpcall (/home/burii/reviews/luajit/valgrind-ci/src/lj_vm.S:1266)
| 82: ==25641== by 0x183CA1: lj_trace_ins (lj_trace.c:757)
| 82: ==25641== by 0x1263C4: lj_dispatch_ins (lj_dispatch.c:430)
| 82: ==25641== by 0x1C0D69: lj_vm_inshook (/home/burii/reviews/luajit/valgrind-ci/src/lj_vm.S:2649)
| 82: ==25641== by 0x11FCCA: lua_pcall (lj_api.c:1173)
| 82: ==25641== by 0x112A84: docall (luajit.c:133)
| 82: ==25641== by 0x112620: handle_script (luajit.c:304)
| 82: ==25641== by 0x111B9C: pmain (luajit.c:604)
Also, I confused with the Command content,
It looks like the valgrind with its options is missed.
Also, it would be nice to run all tarantool-c-tests under valgrind.
Also, what do you think about enabling option `--track-fds` for the CI
(maybe in the separate patch set).
--
Best regards,
Sergey Kaplun
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Tarantool-patches] [PATCH v3 luajit 2/2] ci: add Valgrind testing workflow
2024-09-18 8:50 [Tarantool-patches] [PATCH v3 luajit 0/2] Enable running tests with Valgrind, add CI Valgrind testing workflow mandesero--- via Tarantool-patches
2024-09-18 8:50 ` [Tarantool-patches] [PATCH v3 luajit 1/2] cmake: run tests with Valgrind mandesero--- via Tarantool-patches
@ 2024-09-18 8:50 ` mandesero--- via Tarantool-patches
2024-11-01 14:17 ` Sergey Bronnikov via Tarantool-patches
2024-11-01 12:51 ` [Tarantool-patches] [PATCH v3 luajit 0/2] Enable running tests with Valgrind, add CI " Sergey Bronnikov via Tarantool-patches
2 siblings, 1 reply; 10+ messages in thread
From: mandesero--- via Tarantool-patches @ 2024-09-18 8:50 UTC (permalink / raw)
To: tarantool-patches, skaplun, m.kokryashkin; +Cc: Maksim Tiushev
From: Maksim Tiushev <mandesero@gmail.com>
This patch adds CI testing with Valgrind in three scenarios:
- Full checks enabled.
- No leak checks, with memory fill set to `--malloc-fill=0x00`
and `--free-fill=0x00`.
- No leak checks, with memory fill set to `--malloc-fill=0xFF`
and `--free-fill=0xFF`.
---
.github/actions/setup-valgrind/README.md | 12 +++
.github/actions/setup-valgrind/action.yml | 12 +++
.github/workflows/valgrind-testing.yaml | 95 +++++++++++++++++++++++
3 files changed, 119 insertions(+)
create mode 100644 .github/actions/setup-valgrind/README.md
create mode 100644 .github/actions/setup-valgrind/action.yml
create mode 100644 .github/workflows/valgrind-testing.yaml
diff --git a/.github/actions/setup-valgrind/README.md b/.github/actions/setup-valgrind/README.md
new file mode 100644
index 00000000..e7d66a3a
--- /dev/null
+++ b/.github/actions/setup-valgrind/README.md
@@ -0,0 +1,12 @@
+# Setup environment for Valgrind on Linux
+
+Action setups the environment on Linux runners (install requirements, setup the
+workflow environment, etc) for testing with Valgrind.
+
+## How to use Github Action from Github workflow
+
+Add the following code to the running steps before LuaJIT configuration:
+```
+- uses: ./.github/actions/setup-valgrind
+ if: ${{ matrix.OS == 'Linux' }}
+```
\ No newline at end of file
diff --git a/.github/actions/setup-valgrind/action.yml b/.github/actions/setup-valgrind/action.yml
new file mode 100644
index 00000000..5c11fdaa
--- /dev/null
+++ b/.github/actions/setup-valgrind/action.yml
@@ -0,0 +1,12 @@
+name: Setup CI environment with Valgrind on Linux
+description: Extend setup-linux with Valgrind installation
+
+runs:
+ using: composite
+ steps:
+ - name: Setup CI environment (Linux)
+ uses: ./.github/actions/setup-linux
+ - name: Install Valgrind
+ run: |
+ apt -y install valgrind
+ shell: bash
diff --git a/.github/workflows/valgrind-testing.yaml b/.github/workflows/valgrind-testing.yaml
new file mode 100644
index 00000000..15038092
--- /dev/null
+++ b/.github/workflows/valgrind-testing.yaml
@@ -0,0 +1,95 @@
+name: Valgrind testing
+
+on:
+ push:
+ branches-ignore:
+ - '**-notest'
+ - 'upstream-**'
+ tags-ignore:
+ - '**'
+
+concurrency:
+ # An update of a developer branch cancels the previously
+ # scheduled workflow run for this branch. However, the default
+ # branch, and long-term branch (tarantool/release/2.11,
+ # tarantool/release/2.10, etc) workflow runs are never canceled.
+ #
+ # We use a trick here: define the concurrency group as 'workflow
+ # run ID' + # 'workflow run attempt' because it is a unique
+ # combination for any run. So it effectively discards grouping.
+ #
+ # XXX: we cannot use `github.sha` as a unique identifier because
+ # pushing a tag may cancel a run that works on a branch push
+ # event.
+ group: ${{ startsWith(github.ref, 'refs/heads/tarantool/')
+ && format('{0}-{1}', github.run_id, github.run_attempt)
+ || format('{0}-{1}', github.workflow, github.ref) }}
+ cancel-in-progress: true
+
+jobs:
+ test-valgrind:
+ strategy:
+ fail-fast: false
+ matrix:
+ # XXX: Let's start with only Linux/x86_64
+ # We don't test on Linux/ARM64 because the address returned by the
+ # system allocator may exceed 47 bits. As a result, we are unable to
+ # allocate memory for `lua_State`. Therefore, testing on this platform
+ # is currently disabled.
+ BUILDTYPE: [Debug, Release]
+ VALGRIND_SCENARIO: [full, malloc-free-fill-0x00, malloc-free-fill-0xff]
+ include:
+ - BUILDTYPE: Debug
+ CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
+ - BUILDTYPE: Release
+ CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
+ - VALGRIND_SCENARIO: full
+ VALGRIND_OPTIONS: --leak-check=full --show-leak-kinds=all --track-origins=yes --verbose
+ JOB_POSTFIX: "leak-check: full"
+ - VALGRIND_SCENARIO: malloc-free-fill-0x00
+ VALGRIND_OPTIONS: --leak-check=no --malloc-fill=0x00 --free-fill=0x00
+ JOB_POSTFIX: "malloc/free-fill: 0x00"
+ - VALGRIND_SCENARIO: malloc-free-fill-0xff
+ VALGRIND_OPTIONS: --leak-check=no --malloc-fill=0xff --free-fill=0xff
+ JOB_POSTFIX: "malloc/free-fill: 0xff"
+ runs-on: [self-hosted, regular, Linux, x86_64]
+ name: >
+ LuaJIT with Valgrind (Linux/x86_64)
+ ${{ matrix.BUILDTYPE }}
+ CC: gcc
+ GC64:ON SYSMALLOC:ON
+ ${{ matrix.JOB_POSTFIX }}
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: setup Linux for Valgrind
+ uses: ./.github/actions/setup-valgrind
+ - name: configure
+ # XXX: LuaJIT configuration requires a couple of tweaks:
+ # LUAJIT_USE_SYSMALLOC=ON: 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).
+ # For more info, see root CMakeLists.txt.
+ # LUAJIT_ENABLE_GC64=ON: 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.
+ env:
+ VALGRIND_OPTIONS: ${{ matrix.VALGRIND_OPTIONS }}
+ run: >
+ cmake -S . -B ${{ env.BUILDDIR }}
+ -G Ninja
+ ${{ matrix.CMAKEFLAGS }}
+ -DLUAJIT_USE_VALGRIND=ON
+ -DLUAJIT_ENABLE_GC64=ON
+ -DLUAJIT_USE_SYSMALLOC=ON
+ - name: build
+ run: cmake --build . --parallel
+ working-directory: ${{ env.BUILDDIR }}
+ - name: test
+ run: cmake --build . --parallel --target LuaJIT-test
+ working-directory: ${{ env.BUILDDIR }}
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Tarantool-patches] [PATCH v3 luajit 2/2] ci: add Valgrind testing workflow
2024-09-18 8:50 ` [Tarantool-patches] [PATCH v3 luajit 2/2] ci: add Valgrind testing workflow mandesero--- via Tarantool-patches
@ 2024-11-01 14:17 ` Sergey Bronnikov via Tarantool-patches
2024-11-11 13:04 ` mandesero--- via Tarantool-patches
0 siblings, 1 reply; 10+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-11-01 14:17 UTC (permalink / raw)
To: mandesero, tarantool-patches, skaplun, m.kokryashkin
[-- Attachment #1: Type: text/plain, Size: 7213 bytes --]
Hi, Maxim,
thanks for the patch! See my comments below.
On 18.09.2024 11:50, mandesero--- via Tarantool-patches wrote:
> From: Maksim Tiushev<mandesero@gmail.com>
>
> This patch adds CI testing with Valgrind in three scenarios:
> - Full checks enabled.
> - No leak checks, with memory fill set to `--malloc-fill=0x00`
> and `--free-fill=0x00`.
> - No leak checks, with memory fill set to `--malloc-fill=0xFF`
> and `--free-fill=0xFF`.
Motivation behind chosen configurations is not clear.
And what is a difference for "--malloc-fill=0x00"/"--free-fill=0x00"
and "--malloc-fill=0xFF"/"--free-fill=0xFF". Why the fill value make
sense here?
Also, we usually mention an issue number in commit messages.
Cover letter says that the issue behind these patches is
https://github.com/tarantool/tarantool/issues/3705
but commit messages for both patches doesn't mention the issue.
I propose to add "Part of #3705" to the first patch and
"Closes #3705" to the second patch.
> ---
> .github/actions/setup-valgrind/README.md | 12 +++
> .github/actions/setup-valgrind/action.yml | 12 +++
> .github/workflows/valgrind-testing.yaml | 95 +++++++++++++++++++++++
> 3 files changed, 119 insertions(+)
> create mode 100644 .github/actions/setup-valgrind/README.md
> create mode 100644 .github/actions/setup-valgrind/action.yml
> create mode 100644 .github/workflows/valgrind-testing.yaml
>
> diff --git a/.github/actions/setup-valgrind/README.md b/.github/actions/setup-valgrind/README.md
> new file mode 100644
> index 00000000..e7d66a3a
> --- /dev/null
> +++ b/.github/actions/setup-valgrind/README.md
> @@ -0,0 +1,12 @@
> +# Setup environment for Valgrind on Linux
> +
> +Action setups the environment on Linux runners (install requirements, setup the
> +workflow environment, etc) for testing with Valgrind.
> +
> +## How to use Github Action from Github workflow
> +
> +Add the following code to the running steps before LuaJIT configuration:
> +```
> +- uses: ./.github/actions/setup-valgrind
> + if: ${{ matrix.OS == 'Linux' }}
> +```
> \ No newline at end of file
> diff --git a/.github/actions/setup-valgrind/action.yml b/.github/actions/setup-valgrind/action.yml
> new file mode 100644
> index 00000000..5c11fdaa
> --- /dev/null
> +++ b/.github/actions/setup-valgrind/action.yml
> @@ -0,0 +1,12 @@
> +name: Setup CI environment with Valgrind on Linux
> +description: Extend setup-linux with Valgrind installation
> +
> +runs:
> + using: composite
> + steps:
> + - name: Setup CI environment (Linux)
> + uses: ./.github/actions/setup-linux
> + - name: Install Valgrind
> + run: |
> + apt -y install valgrind
> + shell: bash
> diff --git a/.github/workflows/valgrind-testing.yaml b/.github/workflows/valgrind-testing.yaml
> new file mode 100644
> index 00000000..15038092
> --- /dev/null
> +++ b/.github/workflows/valgrind-testing.yaml
> @@ -0,0 +1,95 @@
> +name: Valgrind testing
> +
> +on:
> + push:
> + branches-ignore:
> + - '**-notest'
> + - 'upstream-**'
> + tags-ignore:
> + - '**'
> +
> +concurrency:
> + # An update of a developer branch cancels the previously
> + # scheduled workflow run for this branch. However, the default
> + # branch, and long-term branch (tarantool/release/2.11,
> + # tarantool/release/2.10, etc) workflow runs are never canceled.
> + #
> + # We use a trick here: define the concurrency group as 'workflow
> + # run ID' + # 'workflow run attempt' because it is a unique
> + # combination for any run. So it effectively discards grouping.
> + #
> + # XXX: we cannot use `github.sha` as a unique identifier because
> + # pushing a tag may cancel a run that works on a branch push
> + # event.
> + group: ${{ startsWith(github.ref, 'refs/heads/tarantool/')
> + && format('{0}-{1}', github.run_id, github.run_attempt)
> + || format('{0}-{1}', github.workflow, github.ref) }}
> + cancel-in-progress: true
> +
> +jobs:
> + test-valgrind:
> + strategy:
> + fail-fast: false
> + matrix:
> + # XXX: Let's start with only Linux/x86_64
> + # We don't test on Linux/ARM64 because the address returned by the
> + # system allocator may exceed 47 bits. As a result, we are unable to
> + # allocate memory for `lua_State`. Therefore, testing on this platform
> + # is currently disabled.
> + BUILDTYPE: [Debug, Release]
> + VALGRIND_SCENARIO: [full, malloc-free-fill-0x00, malloc-free-fill-0xff]
> + include:
> + - BUILDTYPE: Debug
> + CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
This config doesn't have VALGRIND_OPTIONS, is it intentional?
> + - BUILDTYPE: Release
> + CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
Why JOB_POSTFIX is empty here?
> + - VALGRIND_SCENARIO: full
> + VALGRIND_OPTIONS: --leak-check=full --show-leak-kinds=all --track-origins=yes --verbose
Please explain a meaning of specified options.
> + JOB_POSTFIX: "leak-check: full"
> + - VALGRIND_SCENARIO: malloc-free-fill-0x00
> + VALGRIND_OPTIONS: --leak-check=no --malloc-fill=0x00 --free-fill=0x00
> + JOB_POSTFIX: "malloc/free-fill: 0x00"
> + - VALGRIND_SCENARIO: malloc-free-fill-0xff
> + VALGRIND_OPTIONS: --leak-check=no --malloc-fill=0xff --free-fill=0xff
> + JOB_POSTFIX: "malloc/free-fill: 0xff"
> + runs-on: [self-hosted, regular, Linux, x86_64]
> + name: >
> + LuaJIT with Valgrind (Linux/x86_64)
> + ${{ matrix.BUILDTYPE }}
> + CC: gcc
> + GC64:ON SYSMALLOC:ON
> + ${{ matrix.JOB_POSTFIX }}
> + steps:
> + - uses: actions/checkout@v4
> + with:
> + fetch-depth: 0
> + submodules: recursive
> + - name: setup Linux for Valgrind
> + uses: ./.github/actions/setup-valgrind
> + - name: configure
> + # XXX: LuaJIT configuration requires a couple of tweaks:
> + # LUAJIT_USE_SYSMALLOC=ON: 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).
> + # For more info, see root CMakeLists.txt.
> + # LUAJIT_ENABLE_GC64=ON: 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.
> + env:
> + VALGRIND_OPTIONS: ${{ matrix.VALGRIND_OPTIONS }}
> + run: >
> + cmake -S . -B ${{ env.BUILDDIR }}
> + -G Ninja
> + ${{ matrix.CMAKEFLAGS }}
> + -DLUAJIT_USE_VALGRIND=ON
> + -DLUAJIT_ENABLE_GC64=ON
> + -DLUAJIT_USE_SYSMALLOC=ON
> + - name: build
> + run: cmake --build . --parallel
> + working-directory: ${{ env.BUILDDIR }}
> + - name: test
> + run: cmake --build . --parallel --target LuaJIT-test
> + working-directory: ${{ env.BUILDDIR }}
[-- Attachment #2: Type: text/html, Size: 8344 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* [Tarantool-patches] [PATCH v3 luajit 2/2] ci: add Valgrind testing workflow
2024-11-01 14:17 ` Sergey Bronnikov via Tarantool-patches
@ 2024-11-11 13:04 ` mandesero--- via Tarantool-patches
0 siblings, 0 replies; 10+ messages in thread
From: mandesero--- via Tarantool-patches @ 2024-11-11 13:04 UTC (permalink / raw)
To: tarantool-patches; +Cc: mandesero
From: Maksim Tiushev <mandesero@gmail.com>
Hi, Sergey!
Thanks for the review!
I updated the commit message, see below.
> > + CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> This config doesn't have VALGRIND_OPTIONS, is it intentional?
Yes, according to the previous patch, `VALGRIND_OPTIONS` is an
environment variable and must be set before building. It is not
a CMake option. It is setted in the `configure` step in this
workflow.
> > + - BUILDTYPE: Release
> > + CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> Why JOB_POSTFIX is empty here?
`JOB_POSTFIX` describes Valgrind options, not the build type
or CMake options.
---------------------------------------------------------------
This patch adds CI testing with Valgrind in three scenarios:
- Full checks enabled.
- No leak checks, with memory fill set to `--malloc-fill=0x00`
and `--free-fill=0x00`.
- No leak checks, with memory fill set to `--malloc-fill=0xFF`
and `--free-fill=0xFF`.
The use of `0x00` and `0xFF` for memory fill helps to detect dirty
reads. `0x00` mimics zero-initialized memory, which can mask some
uninitialized memory usage. `0xFF` fills memory with a non-zero
values to make such errors easier to spot.
Closes #3705
---
.github/actions/setup-valgrind/README.md | 12 +++
.github/actions/setup-valgrind/action.yml | 12 +++
.github/workflows/valgrind-testing.yaml | 99 +++++++++++++++++++++++
3 files changed, 123 insertions(+)
create mode 100644 .github/actions/setup-valgrind/README.md
create mode 100644 .github/actions/setup-valgrind/action.yml
create mode 100644 .github/workflows/valgrind-testing.yaml
diff --git a/.github/actions/setup-valgrind/README.md b/.github/actions/setup-valgrind/README.md
new file mode 100644
index 00000000..e7d66a3a
--- /dev/null
+++ b/.github/actions/setup-valgrind/README.md
@@ -0,0 +1,12 @@
+# Setup environment for Valgrind on Linux
+
+Action setups the environment on Linux runners (install requirements, setup the
+workflow environment, etc) for testing with Valgrind.
+
+## How to use Github Action from Github workflow
+
+Add the following code to the running steps before LuaJIT configuration:
+```
+- uses: ./.github/actions/setup-valgrind
+ if: ${{ matrix.OS == 'Linux' }}
+```
\ No newline at end of file
diff --git a/.github/actions/setup-valgrind/action.yml b/.github/actions/setup-valgrind/action.yml
new file mode 100644
index 00000000..5c11fdaa
--- /dev/null
+++ b/.github/actions/setup-valgrind/action.yml
@@ -0,0 +1,12 @@
+name: Setup CI environment with Valgrind on Linux
+description: Extend setup-linux with Valgrind installation
+
+runs:
+ using: composite
+ steps:
+ - name: Setup CI environment (Linux)
+ uses: ./.github/actions/setup-linux
+ - name: Install Valgrind
+ run: |
+ apt -y install valgrind
+ shell: bash
diff --git a/.github/workflows/valgrind-testing.yaml b/.github/workflows/valgrind-testing.yaml
new file mode 100644
index 00000000..44056fcd
--- /dev/null
+++ b/.github/workflows/valgrind-testing.yaml
@@ -0,0 +1,99 @@
+name: Valgrind testing
+
+on:
+ push:
+ branches-ignore:
+ - '**-notest'
+ - 'upstream-**'
+ tags-ignore:
+ - '**'
+
+concurrency:
+ # An update of a developer branch cancels the previously
+ # scheduled workflow run for this branch. However, the default
+ # branch, and long-term branch (tarantool/release/2.11,
+ # tarantool/release/2.10, etc) workflow runs are never canceled.
+ #
+ # We use a trick here: define the concurrency group as 'workflow
+ # run ID' + # 'workflow run attempt' because it is a unique
+ # combination for any run. So it effectively discards grouping.
+ #
+ # XXX: we cannot use `github.sha` as a unique identifier because
+ # pushing a tag may cancel a run that works on a branch push
+ # event.
+ group: ${{ startsWith(github.ref, 'refs/heads/tarantool/')
+ && format('{0}-{1}', github.run_id, github.run_attempt)
+ || format('{0}-{1}', github.workflow, github.ref) }}
+ cancel-in-progress: true
+
+jobs:
+ test-valgrind:
+ strategy:
+ fail-fast: false
+ matrix:
+ # XXX: Let's start with only Linux/x86_64
+ # We don't test on Linux/ARM64 because the address returned by the
+ # system allocator may exceed 47 bits. As a result, we are unable to
+ # allocate memory for `lua_State`. Therefore, testing on this platform
+ # is currently disabled.
+ BUILDTYPE: [Debug, Release]
+ VALGRIND_SCENARIO: [full, malloc-free-fill-0x00, malloc-free-fill-0xff]
+ include:
+ - BUILDTYPE: Debug
+ CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
+ - BUILDTYPE: Release
+ CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
+ - VALGRIND_SCENARIO: full
+ VALGRIND_OPTIONS: --leak-check=full --show-leak-kinds=all --track-origins=yes --verbose
+ JOB_POSTFIX: "leak-check: full"
+ # The use of `0x00` and `0xFF` for memory fill helps to detect dirty
+ # reads. `0x00` mimics zero-initialized memory, which can mask some
+ # uninitialized memory usage. `0xFF` fills memory with a non-zero
+ # values to make such errors easier to spot.
+ - VALGRIND_SCENARIO: malloc-free-fill-0x00
+ VALGRIND_OPTIONS: --leak-check=no --malloc-fill=0x00 --free-fill=0x00
+ JOB_POSTFIX: "malloc/free-fill: 0x00"
+ - VALGRIND_SCENARIO: malloc-free-fill-0xff
+ VALGRIND_OPTIONS: --leak-check=no --malloc-fill=0xff --free-fill=0xff
+ JOB_POSTFIX: "malloc/free-fill: 0xff"
+ runs-on: [self-hosted, regular, Linux, x86_64]
+ name: >
+ LuaJIT with Valgrind (Linux/x86_64)
+ ${{ matrix.BUILDTYPE }}
+ CC: gcc
+ GC64:ON SYSMALLOC:ON
+ ${{ matrix.JOB_POSTFIX }}
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: setup Linux for Valgrind
+ uses: ./.github/actions/setup-valgrind
+ - name: configure
+ # XXX: LuaJIT configuration requires a couple of tweaks:
+ # LUAJIT_USE_SYSMALLOC=ON: 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).
+ # For more info, see root CMakeLists.txt.
+ # LUAJIT_ENABLE_GC64=ON: 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.
+ env:
+ VALGRIND_OPTIONS: ${{ matrix.VALGRIND_OPTIONS }}
+ run: >
+ cmake -S . -B ${{ env.BUILDDIR }}
+ -G Ninja
+ ${{ matrix.CMAKEFLAGS }}
+ -DLUAJIT_USE_VALGRIND=ON
+ -DLUAJIT_ENABLE_GC64=ON
+ -DLUAJIT_USE_SYSMALLOC=ON
+ - name: build
+ run: cmake --build . --parallel
+ working-directory: ${{ env.BUILDDIR }}
+ - name: test
+ run: cmake --build . --parallel --target LuaJIT-test
+ working-directory: ${{ env.BUILDDIR }}
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v3 luajit 0/2] Enable running tests with Valgrind, add CI Valgrind testing workflow
2024-09-18 8:50 [Tarantool-patches] [PATCH v3 luajit 0/2] Enable running tests with Valgrind, add CI Valgrind testing workflow mandesero--- via Tarantool-patches
2024-09-18 8:50 ` [Tarantool-patches] [PATCH v3 luajit 1/2] cmake: run tests with Valgrind mandesero--- via Tarantool-patches
2024-09-18 8:50 ` [Tarantool-patches] [PATCH v3 luajit 2/2] ci: add Valgrind testing workflow mandesero--- via Tarantool-patches
@ 2024-11-01 12:51 ` Sergey Bronnikov via Tarantool-patches
2 siblings, 0 replies; 10+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-11-01 12:51 UTC (permalink / raw)
To: mandesero, tarantool-patches, skaplun, m.kokryashkin
[-- Attachment #1: Type: text/plain, Size: 2358 bytes --]
Hi, Maxim!
thanks for new iteration!
Please see my comments.
On 18.09.2024 11:50, mandesero--- via Tarantool-patches wrote:
> From: Maksim Tyushev<mandesero@gmail.com>
>
> This patchset enables running LuaJIT tests under Valgrind, with the
> option to set custom Valgrind options using the `VALGRIND_OPTIONS`
> environment variable. Please note that this environment variable must
> be set before building, and any updates to it will require a project
> rebuild. The patchset also introduces a Valgrind testing workflow
> with three scenarios: full checks, and two memory checks without leak
> detection, where memory is filled with `0x00` and `0xff`.
>
> Some tests consistently fail under Valgrind due to various reasons,
> such as SIGPROF, timeouts, or flaky behavior. These tests are
s/SIGPROF/sysprof/?
> disabled when `LUAJIT_USE_VALGRIND=ON`.
>
> Branch:https://githb.com/tarantool/luajit/tree/mandesero/lj-3705-turn-off-strcmp-opt-in-debug
> Issue:https://github.com/tarantool/tarantool/issues/3705
>
> Changes in v3:
> - Squashed commits 'run tests with Valgrind (1/3 v2)' and
> 'disable failed tests (3/3 v2)'.
> - Simplified `.github/actions/setup-valgrind`, now dependents
> on `.github/actions/setup-linux`.
> - Updated `test/CMakeLists.txt` with minor adjustments.
>
> Maksim Tiushev (2):
> cmake: run tests with Valgrind
> ci: add Valgrind testing workflow
>
> .github/actions/setup-valgrind/README.md | 12 +++
> .github/actions/setup-valgrind/action.yml | 12 +++
> .github/workflows/valgrind-testing.yaml | 95 +++++++++++++++++++
> CMakeLists.txt | 5 +
> test/CMakeLists.txt | 16 ++++
> test/tarantool-tests/CMakeLists.txt | 3 +-
> ...4-add-proto-trace-sysprof-default.test.lua | 1 +
> .../gh-7745-oom-on-trace.test.lua | 1 +
> .../lj-1034-tabov-error-frame.test.lua | 1 +
> .../lj-512-profiler-hook-finalizers.test.lua | 4 +-
> .../lj-726-profile-flush-close.test.lua | 4 +-
> .../misclib-sysprof-lapi.test.lua | 1 +
> 12 files changed, 152 insertions(+), 3 deletions(-)
> create mode 100644 .github/actions/setup-valgrind/README.md
> create mode 100644 .github/actions/setup-valgrind/action.yml
> create mode 100644 .github/workflows/valgrind-testing.yaml
>
[-- Attachment #2: Type: text/html, Size: 3199 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread