* [Tarantool-patches] [PATCH luajit 1/2] c: disable strcmp optimization in Valgrind build
2024-06-26 12:27 [Tarantool-patches] [PATCH luajit 0/2] Disable strcmp optimizations in Valgrind build mandesero--- via Tarantool-patches
@ 2024-06-26 12:27 ` mandesero--- via Tarantool-patches
2024-07-03 10:10 ` Sergey Kaplun via Tarantool-patches
2024-06-26 12:27 ` [Tarantool-patches] [PATCH luajit 2/2] cmake: running tests under Valgrind, disable tests that failed under Valgrind mandesero--- via Tarantool-patches
1 sibling, 1 reply; 4+ messages in thread
From: mandesero--- via Tarantool-patches @ 2024-06-26 12:27 UTC (permalink / raw)
To: tarantool-patches, skaplun, m.kokryashkin; +Cc: mandesero
From: mandesero <mandesero@gmail.com>
---
src/lj_str.c | 43 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/src/lj_str.c b/src/lj_str.c
index 321e8c4f..00cced03 100644
--- a/src/lj_str.c
+++ b/src/lj_str.c
@@ -26,10 +26,29 @@ static LJ_AINLINE int str_fastcmp(const char *a, const char *b, MSize len)
/* -- String helpers ------------------------------------------------------ */
+#if LUAJIT_USE_VALGRIND
+int lj_str_cmp_no_opt(const char *a, const char *b, MSize len)
+{
+ for (MSize i = 0; i < len; i++)
+ if (a[i] != b[i])
+ return 1;
+ return 0;
+}
+#endif
+
/* Ordered compare of strings. Assumes string data is 4-byte aligned. */
int32_t LJ_FASTCALL lj_str_cmp(GCstr *a, GCstr *b)
{
MSize i, n = a->len > b->len ? b->len : a->len;
+#if LUAJIT_USE_VALGRIND
+ const uint8_t *sa = (const uint8_t *)strdata(a);
+ const uint8_t *sb = (const uint8_t *)strdata(b);
+
+ for (i = 0; i < n; i++) {
+ if (sa[i] != sb[i])
+ return (uint8_t)sa[i] < (uint8_t)sb[i] ? -1 : 1;
+ }
+#else
for (i = 0; i < n; i += 4) {
/* Note: innocuous access up to end of string + 3. */
uint32_t va = *(const uint32_t *)(strdata(a)+i);
@@ -46,6 +65,7 @@ int32_t LJ_FASTCALL lj_str_cmp(GCstr *a, GCstr *b)
return va < vb ? -1 : 1;
}
}
+#endif
return (int32_t)(a->len - b->len);
}
@@ -53,6 +73,12 @@ int32_t LJ_FASTCALL lj_str_cmp(GCstr *a, GCstr *b)
static LJ_AINLINE int str_fastcmp(const char *a, const char *b, MSize len)
{
MSize i = 0;
+#if LUAJIT_USE_VALGRIND
+ for (i = 0; i < len; i++) {
+ if (a[i] != b[i])
+ return (uint8_t)a[i] < (uint8_t)b[i] ? -1 : 1;
+ }
+#else
lj_assertX(len > 0, "fast string compare with zero length");
lj_assertX((((uintptr_t)a+len-1) & (LJ_PAGESIZE-1)) <= LJ_PAGESIZE-4,
"fast string compare crossing page boundary");
@@ -68,6 +94,7 @@ static LJ_AINLINE int str_fastcmp(const char *a, const char *b, MSize len)
}
i += 4;
} while (i < len);
+#endif
return 0;
}
@@ -83,7 +110,11 @@ const char *lj_str_find(const char *s, const char *p, MSize slen, MSize plen)
while (slen) {
const char *q = (const char *)memchr(s, c, slen);
if (!q) break;
+#if LUAJIT_USE_VALGRIND
+ if (lj_str_cmp_no_opt(q+1, p, plen) == 0) return q;
+#else
if (memcmp(q+1, p, plen) == 0) return q;
+#endif
q++; slen -= (MSize)(q-s); s = q;
}
}
@@ -232,8 +263,13 @@ GCstr *lj_str_new(lua_State *L, const char *str, size_t lenx)
} else { /* Slow path: end of string is too close to a page boundary. */
while (o != NULL) {
GCstr *sx = gco2str(o);
+#if LUAJIT_USE_VALGRIND
+ if (sx->hash == h && sx->len == len && inc_collision_hard() &&
+ lj_str_cmp_no_opt(str, strdata(sx), len) == 0) {
+#else
if (sx->hash == h && sx->len == len && inc_collision_hard() &&
memcmp(str, strdata(sx), len) == 0) {
+#endif
/* Resurrect if dead. Can only happen with fixstring() (keywords). */
if (isdead(g, o)) flipwhite(o);
g->strhash_hit++;
@@ -277,7 +313,11 @@ GCstr *lj_str_new(lua_State *L, const char *str, size_t lenx)
} else { /* Slow path: end of string is too close to a page boundary. */
while (o != NULL) {
GCstr *sx = gco2str(o);
+#if LUAJIT_USE_VALGRIND
+ if (sx->hash == fh && sx->len == len && lj_str_cmp_no_opt(str, strdata(sx), len) == 0) {
+#else
if (sx->hash == fh && sx->len == len && memcmp(str, strdata(sx), len) == 0) {
+#endif
/* Resurrect if dead. Can only happen with fixstring() (keywords). */
if (isdead(g, o)) flipwhite(o);
g->strhash_hit++;
@@ -323,5 +363,4 @@ void LJ_FASTCALL lj_str_free(global_State *g, GCstr *s)
{
g->strnum--;
lj_mem_free(g, s, sizestring(s));
-}
-
+}
\ No newline at end of file
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread* [Tarantool-patches] [PATCH luajit 2/2] cmake: running tests under Valgrind, disable tests that failed under Valgrind
2024-06-26 12:27 [Tarantool-patches] [PATCH luajit 0/2] Disable strcmp optimizations in Valgrind build mandesero--- via Tarantool-patches
2024-06-26 12:27 ` [Tarantool-patches] [PATCH luajit 1/2] c: disable strcmp optimization " mandesero--- via Tarantool-patches
@ 2024-06-26 12:27 ` mandesero--- via Tarantool-patches
1 sibling, 0 replies; 4+ messages in thread
From: mandesero--- via Tarantool-patches @ 2024-06-26 12:27 UTC (permalink / raw)
To: tarantool-patches, skaplun, m.kokryashkin; +Cc: mandesero
From: mandesero <mandesero@gmail.com>
---
.github/actions/setup-sanitizers/action.yml | 2 +-
.github/workflows/sanitizers-testing.yml | 53 +++++++++++++++++++++
src/lj_no_str_opt.supp | 16 +++++++
test/CMakeLists.txt | 6 ++-
test/tarantool-tests/CMakeLists.txt | 17 +++++++
5 files changed, 92 insertions(+), 2 deletions(-)
create mode 100644 src/lj_no_str_opt.supp
diff --git a/.github/actions/setup-sanitizers/action.yml b/.github/actions/setup-sanitizers/action.yml
index 8642d553..53f95075 100644
--- a/.github/actions/setup-sanitizers/action.yml
+++ b/.github/actions/setup-sanitizers/action.yml
@@ -20,7 +20,7 @@ runs:
- name: Install build and test dependencies
run: |
apt -y update
- apt -y install ${CC_NAME} libstdc++-10-dev cmake ninja-build make perl
+ apt -y install ${CC_NAME} libstdc++-10-dev cmake ninja-build make perl valgrind
shell: bash
env:
CC_NAME: ${{ inputs.cc_name }}
diff --git a/.github/workflows/sanitizers-testing.yml b/.github/workflows/sanitizers-testing.yml
index 154ebe40..85ccf580 100644
--- a/.github/workflows/sanitizers-testing.yml
+++ b/.github/workflows/sanitizers-testing.yml
@@ -93,3 +93,56 @@ jobs:
"
run: cmake --build . --parallel --target LuaJIT-test
working-directory: ${{ env.BUILDDIR }}
+
+
+ test-valgrind:
+ strategy:
+ fail-fast: false
+ matrix:
+ # XXX: Let's start with only Linux/x86_64
+ BUILDTYPE: [Debug, Release]
+ CC: [gcc-10, clang-11]
+ include:
+ - BUILDTYPE: Debug
+ CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
+ - BUILDTYPE: Release
+ CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
+ runs-on: [self-hosted, regular, Linux, x86_64]
+ name: >
+ LuaJIT with Valgrind (Linux/x86_64)
+ ${{ matrix.BUILDTYPE }}
+ CC:${{ matrix.CC }}
+ GC64:ON SYSMALLOC:ON
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ submodules: recursive
+ - name: setup Linux for sanitizers
+ uses: ./.github/actions/setup-sanitizers
+ with:
+ cc_name: ${{ matrix.CC }}
+ - name: configure
+ # XXX: LuaJIT configuration requires a couple of tweaks:
+ # LUAJIT_USE_SYSMALLOC=ON: Valgrind requires the use of
+ # a system-provided memory allocator (realloc) instead
+ # of the built-in memory allocator.
+ # 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.
+ run: >
+ cmake -S . -B ${{ env.BUILDDIR }}
+ -G Ninja
+ ${{ matrix.CMAKEFLAGS }}
+ -DLUAJIT_USE_VALGRIND=ON
+ -DLUAJIT_USE_SYSMALLOC=ON
+ -DLUAJIT_ENABLE_GC64=ON
+ - name: build
+ run: cmake --build . --parallel
+ working-directory: ${{ env.BUILDDIR }}
+ - name: test
+ run: cmake --build . --parallel --target LuaJIT-test
+ working-directory: ${{ env.BUILDDIR }}
\ No newline at end of file
diff --git a/src/lj_no_str_opt.supp b/src/lj_no_str_opt.supp
new file mode 100644
index 00000000..1ec470f8
--- /dev/null
+++ b/src/lj_no_str_opt.supp
@@ -0,0 +1,16 @@
+# Valgrind suppression file for LuaJIT 2.0.
+{
+ Optimized string compare
+ Memcheck:Addr4
+ fun:lj_str_new
+}
+{
+ Optimized string compare
+ Memcheck:Addr1
+ fun:lj_str_new
+}
+{
+ Optimized string compare
+ Memcheck:Cond
+ fun:lj_str_new
+}
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 19726f5a..fc2791f6 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -71,7 +71,11 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS
${PROJECT_NAME}-codespell
)
-set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
+if(LUAJIT_USE_VALGRIND)
+ set(LUAJIT_TEST_COMMAND "valgrind --suppressions=${CMAKE_SOURCE_DIR}/src/lj_no_str_opt.supp ${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
+else()
+ set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
+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 d7c96078..ac09e993 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -121,9 +121,26 @@ add_test_suite_target(tarantool-tests
DEPENDS ${LUAJIT_TEST_DEPS} tarantool-tests-libs
)
+# Tests are disabled as they fail with Valgrind enabled due to SIGPROF
+# signals from the profiler or exceeding the maximum test duration time.
+list(APPEND valgrind_excluded_tests
+ "gh-5688-tool-cli-flag.test.lua"
+ "gh-5813-resolving-of-c-symbols.test.lua"
+ "gh-7264-add-proto-trace-sysprof-default.test.lua"
+ "gh-7745-oom-on-trace.test.lua"
+ "lj-1034-tabov-error-frame.test.lua"
+ "lj-512-profiler-hook-finalizers.test.lua"
+ "lj-726-profile-flush-close.test.lua"
+ "misclib-sysprof-lapi.test.lua"
+)
+
file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}")
foreach(test_path ${tests})
get_filename_component(test_name ${test_path} NAME)
+ list(FIND valgrind_excluded_tests ${test_name} test_index)
+ if(LUAJIT_USE_VALGRIND AND NOT test_index EQUAL -1)
+ continue()
+ endif()
set(test_title "test/${TEST_SUITE_NAME}/${test_name}")
add_test(NAME ${test_title}
COMMAND ${LUAJIT_TEST_COMMAND} ${test_path}
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread