From 88e83e517bf2b5271c4df435ec9daf4a6c132230 Mon Sep 17 00:00:00 2001 From: Roshan Khatri <117414976+roshkhatri@users.noreply.github.com> Date: Sun, 12 Nov 2023 04:31:34 +0000 Subject: [PATCH] Add DEBUG_ASSERTIONS option to custom assert (#12667) This PR introduces a new macro, serverAssertWithInfoDebug, to do complex assertions only for debugging. The main intention is to allow running complex operations during tests without impacting runtime performance. This assertion is enabled when setting DEBUG_ASSERTIONS. The DEBUG_ASSERTIONS flag is set for the daily and CI variants of `test-sanitizer-address`. --- .github/workflows/ci.yml | 2 +- .github/workflows/daily.yml | 2 +- src/db.c | 1 + src/server.h | 8 ++++++++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 43547a431..f1a9efcca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,7 +31,7 @@ jobs: - uses: actions/checkout@v3 - name: make # build with TLS module just for compilation coverage - run: make SANITIZER=address REDIS_CFLAGS='-Werror' BUILD_TLS=module + run: make SANITIZER=address REDIS_CFLAGS='-Werror -DDEBUG_ASSERTIONS' BUILD_TLS=module - name: testprep run: sudo apt-get install tcl8.6 tclx -y - name: test diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml index 3cd66b794..19d97df9c 100644 --- a/.github/workflows/daily.yml +++ b/.github/workflows/daily.yml @@ -587,7 +587,7 @@ jobs: repository: ${{ env.GITHUB_REPOSITORY }} ref: ${{ env.GITHUB_HEAD_REF }} - name: make - run: make SANITIZER=address REDIS_CFLAGS='-DREDIS_TEST -Werror' + run: make SANITIZER=address REDIS_CFLAGS='-DREDIS_TEST -Werror -DDEBUG_ASSERTIONS' - name: testprep run: | sudo apt-get update diff --git a/src/db.c b/src/db.c index da9722273..77bcb8e95 100644 --- a/src/db.c +++ b/src/db.c @@ -304,6 +304,7 @@ int getKeySlot(sds key) { * the key slot would fallback to calculateKeySlot. */ if (server.current_client && server.current_client->slot >= 0 && server.current_client->flags & CLIENT_EXECUTING_COMMAND) { + debugServerAssertWithInfo(server.current_client, NULL, calculateKeySlot(key)==server.current_client->slot); return server.current_client->slot; } return calculateKeySlot(key); diff --git a/src/server.h b/src/server.h index c373e4dd4..4f20fe231 100644 --- a/src/server.h +++ b/src/server.h @@ -673,6 +673,14 @@ typedef enum { #define serverAssert(_e) (likely(_e)?(void)0 : (_serverAssert(#_e,__FILE__,__LINE__),redis_unreachable())) #define serverPanic(...) _serverPanic(__FILE__,__LINE__,__VA_ARGS__),redis_unreachable() +/* The following macros provide assertions that are only executed during test builds and should be used to add + * assertions that are too computationally expensive or dangerous to run during normal operations. */ +#ifdef DEBUG_ASSERTIONS +#define debugServerAssertWithInfo(...) serverAssertWithInfo(__VA_ARGS__) +#else +#define debugServerAssertWithInfo(...) +#endif + /* latency histogram per command init settings */ #define LATENCY_HISTOGRAM_MIN_VALUE 1L /* >= 1 nanosec */ #define LATENCY_HISTOGRAM_MAX_VALUE 1000000000L /* <= 1 secs */