Skip to content

Commit dfd71f9

Browse files
meiravgrimeiravgri
authored
[0.7] Replace Valgrind with address-sanitizer [MOD-10409] (#804)
backport of #803 to 0.7
1 parent fb7a3ce commit dfd71f9

File tree

11 files changed

+66
-95
lines changed

11 files changed

+66
-95
lines changed

‎.github/workflows/arm.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ jobs:
3434
uses: ./.github/workflows/task-unit-test.yml
3535
with:
3636
env: ${{ needs.start-runner.outputs.label }} # run the job on the newly created runner
37-
run-valgrind: false # run the job without valgrind
3837

3938
stop-runner:
4039
name: Stop self-hosted EC2 runner

‎.github/workflows/debian11.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
name: debian bullseye flow
22

3-
on: [workflow_dispatch]
3+
on: [workflow_dispatch, workflow_call]
44

55
jobs:
66
bullseye:
77
uses: ./.github/workflows/task-unit-test.yml
88
with:
9-
container: debian:bullseye
9+
container: gcc:11-bullseye

‎.github/workflows/event-merge-to-queue.yml‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@ jobs:
2424
# with:
2525
# container: ubuntu:bionic
2626
bullseye:
27-
uses: ./.github/workflows/task-unit-test.yml
28-
with:
29-
container: debian:bullseye
27+
uses: ./.github/workflows/debian11.yml
3028
# amazonlinux2:
3129
# needs: [check-if-docs-only]
3230
# if: ${{ needs.check-if-docs-only.outputs.only-docs-changed == 'false' }}

‎.github/workflows/event-nightly.yml‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,43 +16,38 @@ jobs:
1616
uses: ./.github/workflows/task-unit-test.yml
1717
with:
1818
container: ubuntu:jammy
19-
run-valgrind: false
2019
focal:
2120
uses: ./.github/workflows/task-unit-test.yml
2221
with:
2322
container: ubuntu:focal
24-
run-valgrind: false
2523
# bionic:
2624
# uses: ./.github/workflows/task-unit-test.yml
2725
# with:
2826
# container: ubuntu:bionic
29-
# run-valgrind: false
3027
bullseye:
31-
uses: ./.github/workflows/task-unit-test.yml
32-
with:
33-
container: debian:bullseye
34-
run-valgrind: false
35-
# centos7:
36-
# uses: ./.github/workflows/task-unit-test.yml
37-
# with:
38-
# container: centos:7
39-
# run-valgrind: false
40-
# amazonlinux2:
41-
# uses: ./.github/workflows/amazon2.yml
28+
uses: ./.github/workflows/debian11.yml
29+
# amazonlinux2:
30+
# uses: ./.github/workflows/amazon2.yml
4231
mariner2:
4332
uses: ./.github/workflows/mariner2.yml
4433
rocky8:
4534
uses: ./.github/workflows/task-unit-test.yml
4635
with:
4736
container: rockylinux:8
48-
run-valgrind: false
4937
rocky9:
5038
uses: ./.github/workflows/task-unit-test.yml
5139
with:
5240
container: rockylinux:9
53-
run-valgrind: false
5441
macos:
5542
uses: ./.github/workflows/macos.yml
5643
arm:
5744
uses: ./.github/workflows/arm.yml
5845
secrets: inherit
46+
coverage:
47+
uses: ./.github/workflows/coverage.yml
48+
secrets: inherit
49+
sanitizer:
50+
uses: ./.github/workflows/task-unit-test.yml
51+
with:
52+
san: address
53+
secrets: inherit

‎.github/workflows/event-pull_request.yml‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,13 @@ jobs:
3030
- name: flow tests
3131
run: make flow_test
3232

33+
sanitizer:
34+
uses: ./.github/workflows/task-unit-test.yml
35+
with:
36+
san: address
37+
secrets: inherit
38+
3339
coverage:
34-
needs: [basic-tests]
3540
if: ${{ !github.event.pull_request.draft}}
3641
uses: ./.github/workflows/coverage.yml
3742
secrets: inherit
@@ -54,6 +59,7 @@ jobs:
5459
pr-validation:
5560
needs:
5661
- basic-tests
62+
- sanitizer
5763
- coverage
5864
- codeql-analysis
5965
- spellcheck

‎.github/workflows/macos.yml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,8 @@ jobs:
77
uses: ./.github/workflows/task-unit-test.yml
88
with:
99
env: macos-latest
10-
run-valgrind: false
10+
macos-san:
11+
uses: ./.github/workflows/task-unit-test.yml
12+
with:
13+
env: macos-latest
14+
san: address

‎.github/workflows/mariner2.yml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,3 @@ jobs:
88
with:
99
container: mcr.microsoft.com/cbl-mariner/base/core:2.0
1010
pre-checkout-script: tdnf install -y --noplugins --skipsignature tar gzip
11-
run-valgrind: false # TODO: enable valgrind? (requires to install valgrind)

‎.github/workflows/task-unit-test.yml‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,15 @@ on:
1313
pre-checkout-script:
1414
description: 'Script to run before checkout'
1515
type: string
16-
run-valgrind:
17-
description: 'Run valgrind tests'
18-
type: boolean
19-
default: true
16+
san:
17+
type: string
18+
19+
env:
20+
SAN_VALUE: ${{ inputs.san }}
2021

2122
jobs:
2223
test:
23-
name: Test ${{ inputs.container && format('{0} (on {1})', inputs.container, inputs.env) || inputs.env }}
24+
name: Test ${{ inputs.container && format('{0} (on {1})', inputs.container, inputs.env) || inputs.env }}${{ inputs.san && ' sanitizer' || '' }}
2425
runs-on: ${{ inputs.env }}
2526
container: ${{ inputs.container || null }}
2627
defaults:
@@ -47,13 +48,16 @@ jobs:
4748
echo "name=$NAME" >> $GITHUB_OUTPUT
4849
4950
- name: unit tests
50-
run: make unit_test
51-
- name: valgrind
52-
if: ${{ inputs.run-valgrind }}
53-
run: make valgrind
54-
- name: Archive valgrind tests reports
55-
if: ${{ inputs.run-valgrind && failure() }}
51+
run: make unit_test SAN="$SAN_VALUE"
52+
- name: Set test path
53+
if: ${{ inputs.san == 'address' && failure() }}
54+
id: tests-artifact-path
55+
run: |
56+
FULL_VARIANT=$(uname)-$(uname -m)-debug-asan
57+
echo "path=bin/${FULL_VARIANT}/unit_tests/Testing/Temporary/" >> $GITHUB_OUTPUT
58+
- name: Archive san tests reports
59+
if: ${{ inputs.san == 'address' && failure() }}
5660
uses: actions/upload-artifact@v4
5761
with:
58-
name: valgrind tests reports on ${{ steps.artifact-name.outputs.name }}
59-
path: bin/Linux-x86_64-debug/unit_tests/Testing/Temporary/
62+
name: san tests reports on ${{ steps.artifact-name.outputs.name }}
63+
path: ${{ steps.tests-artifact-path.outputs.path }}

‎.install/ubuntu_22.04.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ export DEBIAN_FRONTEND=noninteractive
44
MODE=$1 # whether to install using sudo or not
55

66
$MODE apt-get update -qq
7-
$MODE apt-get install -yqq git wget build-essential valgrind lcov
7+
$MODE apt-get install -yqq git wget build-essential lcov
88
source install_cmake.sh $MODE

‎Makefile‎

Lines changed: 19 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,6 @@ ifneq ($(filter coverage show-cov upload-cov,$(MAKECMDGOALS)),)
88
COV=1
99
endif
1010

11-
ifneq ($(VG),)
12-
VALGRIND=$(VG)
13-
endif
14-
15-
ifeq ($(VALGRIND),1)
16-
override DEBUG ?= 1
17-
endif
18-
1911
ifeq ($(COV),1)
2012
override DEBUG ?= 1
2113
CMAKE_COV += -DUSE_COVERAGE=ON
@@ -26,30 +18,18 @@ CMAKE_TESTS += -DVECSIM_BUILD_TESTS=off
2618
endif
2719

2820
ifneq ($(SAN),)
29-
override DEBUG ?= 1
30-
export ASAN_OPTIONS=detect_odr_violation=0:allocator_may_return_null=1
31-
export MSAN_OPTIONS=allocator_may_return_null=1
3221

33-
ifeq ($(SAN),mem)
34-
override SAN=memory
35-
else ifeq ($(SAN),addr)
22+
ifeq ($(SAN),address)
3623
override SAN=address
37-
endif
38-
39-
ifeq ($(SAN),memory)
40-
CMAKE_SAN=-DUSE_MSAN=ON
41-
override CTEST_ARGS += --exclude-regex BruteForceTest.sanity_rinsert_1280
42-
43-
else ifeq ($(SAN),address)
24+
DEBUG=1
25+
export ASAN_OPTIONS=detect_odr_violation=0:allocator_may_return_null=1
4426
CMAKE_SAN=-DUSE_ASAN=ON
45-
else ifeq ($(SAN),leak)
46-
else ifeq ($(SAN),thread)
4727
else
48-
$(error SAN=mem|addr|leak|thread)
28+
$(error SAN=address is currently the only supported option)
4929
endif
5030

5131
export SAN
52-
endif # SAN
32+
endif # SAN != ''
5333

5434
ROOT=.
5535
export ROOT
@@ -60,19 +40,18 @@ make build
6040
DEBUG=1 # build debug variant
6141
COV=1 # build for code coverage
6242
VERBOSE=1 # print detailed build info
63-
VG|VALGRIND=1 # build for Valgrind
64-
SAN=type # build with LLVM sanitizer (type=address|memory|leak|thread)
65-
SLOW=1 # don't run build in parallel (for diagnostics)
66-
PROFILE=1 # enable profiling compile flags (and debug symbols) for release type.
43+
SAN=address # build with AddressSanitizer (clang)
44+
SLOW=1 # don't run build in parallel (for diagnostics)
45+
PROFILE=1 # enable profiling compile flags (and debug symbols) for release type.
6746
make pybind # build Python bindings
6847
make clean # remove binary files
6948
ALL=1 # remove binary directories
7049

7150
make unit_test # run unit tests
7251
CTEST_ARGS=args # extra CTest arguments
73-
VG|VALGRIND=1 # run tests with valgrind
52+
SAN=address # run tests with AddressSanitizer
7453
FP_64=1 # run tests with 64-bit floating point
75-
make valgrind # build for Valgrind and run tests
54+
make asan # build with AddressSanitizer and run unit tests
7655
make flow_test # run flow tests (with pytest)
7756
TEST=file::name # run specific test
7857
make mod_test # run Redis module intergration tests (with RLTest)
@@ -95,6 +74,11 @@ FLAVOR=debug
9574
else
9675
FLAVOR=release
9776
endif
77+
78+
ifeq ($(SAN),address)
79+
FLAVOR := ${FLAVOR}-asan
80+
endif
81+
9882
FULL_VARIANT:=$(shell uname)-$(shell uname -m)-$(FLAVOR)
9983
BINROOT=$(ROOT)/bin/$(FULL_VARIANT)
10084
BINDIR=$(BINROOT)
@@ -172,22 +156,18 @@ ifeq ($(VERBOSE),1)
172156
_CTEST_ARGS += -V
173157
endif
174158

175-
ifeq ($(VALGRIND),1)
176-
_CTEST_ARGS += \
177-
-T memcheck \
178-
--overwrite MemoryCheckCommandOptions="--leak-check=full --error-exitcode=255"
179-
endif
159+
# AddressSanitizer is handled via SAN=address in cmake/san.cmake
180160

181161
unit_test:
182162
$(SHOW)mkdir -p $(BINDIR)
183163
$(SHOW)cd $(BINDIR) && cmake $(CMAKE_FLAGS) $(CMAKE_DIR)
184164
@make --no-print-directory -C $(BINDIR) $(MAKE_J)
185165
$(SHOW)cd $(TESTDIR) && GTEST_COLOR=1 ctest $(_CTEST_ARGS)
186166

187-
valgrind:
188-
$(SHOW)$(MAKE) VG=1 unit_test
167+
asan:
168+
$(SHOW)$(MAKE) SAN=address unit_test
189169

190-
.PHONY: unit_test valgrind
170+
.PHONY: unit_test asan
191171

192172
#----------------------------------------------------------------------------------------------
193173

0 commit comments

Comments
 (0)