diff --git a/.github/workflows/daily.yml b/.github/workflows/daily.yml index 9d11dbfb4..7c41dc8d9 100644 --- a/.github/workflows/daily.yml +++ b/.github/workflows/daily.yml @@ -606,6 +606,50 @@ jobs: if: true && !contains(github.event.inputs.skiptests, 'unittest') run: ./src/redis-server test all + test-sanitizer-memory: + runs-on: ubuntu-latest + if: | + (github.event_name == 'workflow_dispatch' || (github.event_name != 'workflow_dispatch' && github.repository == 'redis/redis')) && + !contains(github.event.inputs.skipjobs, 'sanitizer') + timeout-minutes: 14400 + env: + CC: clang # MSan work only with clang + steps: + - name: prep + if: github.event_name == 'workflow_dispatch' + run: | + echo "GITHUB_REPOSITORY=${{github.event.inputs.use_repo}}" >> $GITHUB_ENV + echo "GITHUB_HEAD_REF=${{github.event.inputs.use_git_ref}}" >> $GITHUB_ENV + echo "skipjobs: ${{github.event.inputs.skipjobs}}" + echo "skiptests: ${{github.event.inputs.skiptests}}" + echo "test_args: ${{github.event.inputs.test_args}}" + echo "cluster_test_args: ${{github.event.inputs.cluster_test_args}}" + - uses: actions/checkout@v4 + with: + repository: ${{ env.GITHUB_REPOSITORY }} + ref: ${{ env.GITHUB_HEAD_REF }} + - name: make + run: make SANITIZER=memory REDIS_CFLAGS='-DREDIS_TEST -Werror -DDEBUG_ASSERTIONS' + - name: testprep + run: | + sudo apt-get update + sudo apt-get install tcl8.6 tclx -y + - name: test + if: true && !contains(github.event.inputs.skiptests, 'redis') + run: ./runtest --accurate --verbose --dump-logs ${{github.event.inputs.test_args}} + - name: module api test + if: true && !contains(github.event.inputs.skiptests, 'modules') + run: SANITIZER=memory CFLAGS='-Werror' ./runtest-moduleapi --verbose --dump-logs ${{github.event.inputs.test_args}} + - name: sentinel tests + if: true && !contains(github.event.inputs.skiptests, 'sentinel') + run: ./runtest-sentinel ${{github.event.inputs.cluster_test_args}} + - name: cluster tests + if: true && !contains(github.event.inputs.skiptests, 'cluster') + run: ./runtest-cluster ${{github.event.inputs.cluster_test_args}} + - name: unittest + if: true && !contains(github.event.inputs.skiptests, 'unittest') + run: ./src/redis-server test all + test-sanitizer-undefined: runs-on: ubuntu-latest if: | diff --git a/deps/Makefile b/deps/Makefile index 5593e0cf7..56df41d34 100644 --- a/deps/Makefile +++ b/deps/Makefile @@ -12,6 +12,23 @@ BINCOLOR="\033[37;1m" MAKECOLOR="\033[32;1m" ENDCOLOR="\033[0m" +DEPS_CFLAGS := $(CFLAGS) +DEPS_LDFLAGS := $(LDFLAGS) +CLANG := $(findstring clang,$(shell sh -c '$(CC) --version | head -1')) + +# MSan looks for errors related to uninitialized memory. +# Make sure to build the dependencies with MSan as it needs all the code to be instrumented. +# A library could be used to initialize memory but if it's not build with --fsanitize=memory then +# MSan doesn't know about it and will spit false positive error when that memory is then used. +ifeq ($(SANITIZER),memory) +ifeq (clang, $(CLANG)) + DEPS_CFLAGS+=-fsanitize=memory -fsanitize-memory-track-origins=2 -fno-sanitize-recover=all -fno-omit-frame-pointer + DEPS_LDFLAGS+=-fsanitize=memory +else + $(error "MemorySanitizer needs to be compiled and linked with clang. Please use CC=clang") +endif +endif + default: @echo "Explicit target required" @@ -53,31 +70,31 @@ endif hiredis: .make-prerequisites @printf '%b %b\n' $(MAKECOLOR)MAKE$(ENDCOLOR) $(BINCOLOR)$@$(ENDCOLOR) - cd hiredis && $(MAKE) static $(HIREDIS_MAKE_FLAGS) + cd hiredis && $(MAKE) static $(HIREDIS_MAKE_FLAGS) CFLAGS="$(DEPS_CFLAGS)" LDFLAGS="$(DEPS_LDFLAGS)" .PHONY: hiredis linenoise: .make-prerequisites @printf '%b %b\n' $(MAKECOLOR)MAKE$(ENDCOLOR) $(BINCOLOR)$@$(ENDCOLOR) - cd linenoise && $(MAKE) + cd linenoise && $(MAKE) CFLAGS="$(DEPS_CFLAGS)" LDFLAGS="$(DEPS_LDFLAGS)" .PHONY: linenoise hdr_histogram: .make-prerequisites @printf '%b %b\n' $(MAKECOLOR)MAKE$(ENDCOLOR) $(BINCOLOR)$@$(ENDCOLOR) - cd hdr_histogram && $(MAKE) + cd hdr_histogram && $(MAKE) CFLAGS="$(DEPS_CFLAGS)" LDFLAGS="$(DEPS_LDFLAGS)" .PHONY: hdr_histogram fpconv: .make-prerequisites @printf '%b %b\n' $(MAKECOLOR)MAKE$(ENDCOLOR) $(BINCOLOR)$@$(ENDCOLOR) - cd fpconv && $(MAKE) + cd fpconv && $(MAKE) CFLAGS="$(DEPS_CFLAGS)" LDFLAGS="$(DEPS_LDFLAGS)" .PHONY: fpconv fast_float: .make-prerequisites @printf '%b %b\n' $(MAKECOLOR)MAKE$(ENDCOLOR) $(BINCOLOR)$@$(ENDCOLOR) - cd fast_float && $(MAKE) libfast_float + cd fast_float && $(MAKE) libfast_float CFLAGS="$(DEPS_CFLAGS)" LDFLAGS="$(DEPS_LDFLAGS)" .PHONY: fast_float @@ -86,8 +103,8 @@ ifeq ($(uname_S),SunOS) LUA_CFLAGS= -D__C99FEATURES__=1 endif -LUA_CFLAGS+= -Wall -DLUA_ANSI -DENABLE_CJSON_GLOBAL -DREDIS_STATIC='' -DLUA_USE_MKSTEMP $(CFLAGS) -LUA_LDFLAGS+= $(LDFLAGS) +LUA_CFLAGS+= -Wall -DLUA_ANSI -DENABLE_CJSON_GLOBAL -DREDIS_STATIC='' -DLUA_USE_MKSTEMP $(DEPS_CFLAGS) +LUA_LDFLAGS+= $(DEPS_LDFLAGS) ifeq ($(LUA_DEBUG),yes) LUA_CFLAGS+= -O0 -g -DLUA_USE_APICHECK else diff --git a/src/Makefile b/src/Makefile index 40079b9f3..6cfac4aac 100644 --- a/src/Makefile +++ b/src/Makefile @@ -118,12 +118,24 @@ else ifeq ($(SANITIZER),thread) CFLAGS+=-fsanitize=thread -fno-sanitize-recover=all -fno-omit-frame-pointer LDFLAGS+=-fsanitize=thread +else +ifeq ($(SANITIZER),memory) +ifeq (clang, $(CLANG)) + export CXX:=clang + export LD:=clang + MALLOC=libc # MSan provides its own allocator so make sure not to use jemalloc as they clash + CFLAGS+=-fsanitize=memory -fsanitize-memory-track-origins=2 -fno-sanitize-recover=all -fno-omit-frame-pointer + LDFLAGS+=-fsanitize=memory +else + $(error "MemorySanitizer needs to be compiled and linked with clang. Please use CC=clang") +endif else $(error "unknown sanitizer=${SANITIZER}") endif endif endif endif +endif # Override default settings if possible -include .make-settings diff --git a/src/config.h b/src/config.h index 47338554a..2427a7364 100644 --- a/src/config.h +++ b/src/config.h @@ -168,6 +168,12 @@ #define REDIS_NO_SANITIZE(sanitizer) #endif +#if defined(__clang__) +#define REDIS_NO_SANITIZE_MSAN(sanitizer) REDIS_NO_SANITIZE(sanitizer) +#else +#define REDIS_NO_SANITIZE_MSAN(sanitizer) +#endif + /* Define rdb_fsync_range to sync_file_range() on Linux, otherwise we use * the plain fsync() call. */ #if (defined(__linux__) && defined(SYNC_FILE_RANGE_WAIT_BEFORE)) diff --git a/src/debug.c b/src/debug.c index 77a4cf78c..30df823f8 100644 --- a/src/debug.c +++ b/src/debug.c @@ -1378,6 +1378,7 @@ static void* getAndSetMcontextEip(ucontext_t *uc, void *eip) { #undef NOT_SUPPORTED } +REDIS_NO_SANITIZE_MSAN("memory") REDIS_NO_SANITIZE("address") void logStackContent(void **sp) { if (server.hide_user_data_from_log) { @@ -2729,7 +2730,7 @@ static size_t get_ready_to_signal_threads_tids(int sig_num, pid_t tids[TIDS_MAX_ pid_t calling_tid = syscall(SYS_gettid); int current_thread_index = -1; long nread; - char buff[PATH_MAX]; + char buff[PATH_MAX] = {0}; /* readdir() is not async-signal-safe (AS-safe). Hence, we read the file using SYS_getdents64, which is considered AS-sync*/ diff --git a/src/lzf_c.c b/src/lzf_c.c index 7cbbc82fa..9f858a2d3 100644 --- a/src/lzf_c.c +++ b/src/lzf_c.c @@ -96,6 +96,12 @@ # define NO_SANITIZE(sanitizer) #endif +#if defined(__clang__) +#define NO_SANITIZE_MSAN(sanitizer) NO_SANITIZE(sanitizer) +#else +#define NO_SANITIZE_MSAN(sanitizer) +#endif + /* * compressed format * @@ -105,6 +111,7 @@ * */ NO_SANITIZE("alignment") +NO_SANITIZE_MSAN("memory") size_t lzf_compress (const void *const in_data, size_t in_len, void *out_data, size_t out_len diff --git a/src/memtest.c b/src/memtest.c index f75d6b21c..e21cc9cf8 100644 --- a/src/memtest.c +++ b/src/memtest.c @@ -36,16 +36,6 @@ #define ULONG_ZEROONE 0x5555555555555555UL #endif -#if defined(__has_attribute) -#if __has_attribute(no_sanitize) -#define NO_SANITIZE(sanitizer) __attribute__((no_sanitize(sanitizer))) -#endif -#endif - -#if !defined(NO_SANITIZE) -#define NO_SANITIZE(sanitizer) -#endif - static struct winsize ws; size_t progress_printed; /* Printed chars in screen-wide progress bar. */ size_t progress_full; /* How many chars to write to fill the progress bar. */ @@ -268,7 +258,7 @@ int memtest_test(unsigned long *m, size_t bytes, int passes, int interactive) { * the cache. */ #define MEMTEST_DECACHE_SIZE (1024*8) -NO_SANITIZE("undefined") +REDIS_NO_SANITIZE("undefined") int memtest_preserving_test(unsigned long *m, size_t bytes, int passes) { unsigned long backup[MEMTEST_BACKUP_WORDS]; unsigned long *p = m; diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index 2dbdbd6a1..8319b3a56 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -438,6 +438,7 @@ static void clientDone(client c) { } } +REDIS_NO_SANITIZE_MSAN("memory") static void readHandler(aeEventLoop *el, int fd, void *privdata, int mask) { client c = privdata; void *reply = NULL; diff --git a/src/t_set.c b/src/t_set.c index 5e8c21878..b859e15f2 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -74,8 +74,8 @@ static void maybeConvertToIntset(robj *set) { if (setTypeSize(set) > intsetMaxEntries()) return; /* can't use intset */ intset *is = intsetNew(); char *str; - size_t len; - int64_t llval; + size_t len = 0; + int64_t llval = 0; setTypeIterator *si = setTypeInitIterator(set); while (setTypeNext(si, &str, &len, &llval) != -1) { if (str) { @@ -371,7 +371,7 @@ int setTypeNext(setTypeIterator *si, char **str, size_t *len, int64_t *llele) { } if (lpi == NULL) return -1; si->lpi = lpi; - unsigned int l; + unsigned int l = 0; *str = (char *)lpGetValue(lpi, &l, (long long *)llele); *len = (size_t)l; } else { @@ -388,9 +388,9 @@ int setTypeNext(setTypeIterator *si, char **str, size_t *len, int64_t *llele) { * This function is the way to go for write operations where COW is not * an issue. */ sds setTypeNextObject(setTypeIterator *si) { - int64_t intele; + int64_t intele = 0; char *str; - size_t len; + size_t len = 0; if (setTypeNext(si, &str, &len, &intele) == -1) return NULL; if (str != NULL) return sdsnewlen(str, len); @@ -522,8 +522,8 @@ int setTypeConvertAndExpand(robj *setobj, int enc, unsigned long cap, int panic) } unsigned char *lp = lpNew(estcap); char *str; - size_t len; - int64_t llele; + size_t len = 0; + int64_t llele = 0; si = setTypeInitIterator(setobj); while (setTypeNext(si, &str, &len, &llele) != -1) { if (str != NULL) @@ -574,8 +574,8 @@ robj *setTypeDup(robj *o) { dictExpand(set->ptr, dictSize(d)); si = setTypeInitIterator(o); char *str; - size_t len; - int64_t intobj; + size_t len = 0; + int64_t intobj = 0; while (setTypeNext(si, &str, &len, &intobj) != -1) { setTypeAdd(set, (sds)str); } @@ -818,8 +818,8 @@ void spopWithCountCommand(client *c) { /* Common iteration vars. */ char *str; - size_t len; - int64_t llele; + size_t len = 0; + int64_t llele = 0; unsigned long remaining = size-count; /* Elements left after SPOP. */ /* If we are here, the number of requested elements is less than the @@ -839,7 +839,7 @@ void spopWithCountCommand(client *c) { unsigned char **ps = zmalloc(sizeof(char *) * count); for (unsigned long i = 0; i < count; i++) { p = lpNextRandom(lp, p, &index, count - i, 1); - unsigned int len; + unsigned int len = 0; str = (char *)lpGetValue(p, &len, (long long *)&llele); if (str) { @@ -903,7 +903,7 @@ void spopWithCountCommand(client *c) { unsigned char **ps = zmalloc(sizeof(char *) * remaining); for (unsigned long i = 0; i < remaining; i++) { p = lpNextRandom(lp, p, &index, remaining - i, 1); - unsigned int len; + unsigned int len = 0; str = (char *)lpGetValue(p, &len, (long long *)&llele); setTypeAddAux(newset, str, len, llele, 0); ps[i] = p; @@ -1030,8 +1030,8 @@ void srandmemberWithCountCommand(client *c) { int uniq = 1; robj *set; char *str; - size_t len; - int64_t llele; + size_t len = 0; + int64_t llele = 0; dict *d; @@ -1232,8 +1232,8 @@ void srandmemberWithCountCommand(client *c) { void srandmemberCommand(client *c) { robj *set; char *str; - size_t len; - int64_t llele; + size_t len = 0; + int64_t llele = 0; if (c->argc == 3) { srandmemberWithCountCommand(c); @@ -1288,8 +1288,8 @@ void sinterGenericCommand(client *c, robj **setkeys, setTypeIterator *si; robj *dstset = NULL; char *str; - size_t len; - int64_t intobj; + size_t len = 0; + int64_t intobj = 0; void *replylen = NULL; unsigned long j, cardinality = 0; int encoding, empty = 0; @@ -1453,8 +1453,8 @@ void sinterCommand(client *c) { void smembersCommand(client *c) { setTypeIterator *si; char *str; - size_t len; - int64_t intobj; + size_t len = 0; + int64_t intobj = 0; robj *setobj = lookupKeyRead(c->db, c->argv[1]); if (checkType(c,setobj,OBJ_SET)) return; if (!setobj) { @@ -1523,8 +1523,8 @@ void sunionDiffGenericCommand(client *c, robj **setkeys, int setnum, robj *dstset = NULL; int dstset_encoding = OBJ_ENCODING_INTSET; char *str; - size_t len; - int64_t llval; + size_t len = 0; + int64_t llval = 0; int encoding; int j, cardinality = 0; int diff_algo = 1; diff --git a/src/t_stream.c b/src/t_stream.c index dae1ce372..a242b02ca 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -257,7 +257,7 @@ static inline int64_t lpGetIntegerIfValid(unsigned char *ele, int *valid) { /* The following code path should never be used for how listpacks work: * they should always be able to store an int64_t value in integer * encoded form. However the implementation may change. */ - long long ll; + long long ll = 0; int ret = string2ll((char*)e,v,&ll); if (valid) *valid = ret; diff --git a/tests/modules/Makefile b/tests/modules/Makefile index 5b53d44e4..09855996f 100644 --- a/tests/modules/Makefile +++ b/tests/modules/Makefile @@ -11,9 +11,27 @@ else # Linux, others SHOBJ_LDFLAGS ?= -shared endif +CLANG := $(findstring clang,$(shell sh -c '$(CC) --version | head -1')) + +ifeq ($(SANITIZER),memory) +ifeq (clang, $(CLANG)) + LD=clang + MALLOC=libc + CFLAGS+=-fsanitize=memory -fsanitize-memory-track-origins=2 -fno-sanitize-recover=all -fno-omit-frame-pointer + LDFLAGS+=-fsanitize=memory +else + $(error "MemorySanitizer needs to be compiled and linked with clang. Please use CC=clang") +endif +endif + + +# This is a hack to override the default CC. When running with SANITIZER=memory +# tough we want to keep the compiler as clang as MSan is not supported for gcc ifeq ($(uname_S),Linux) - LD = gcc - CC = gcc +ifneq ($(SANITIZER),memory) + LD = gcc + CC = gcc +endif endif # OS X 11.x doesn't have /usr/lib/libSystem.dylib and needs an explicit setting. diff --git a/tests/support/server.tcl b/tests/support/server.tcl index e429043fc..6e7bfa1ab 100644 --- a/tests/support/server.tcl +++ b/tests/support/server.tcl @@ -294,7 +294,7 @@ proc spawn_server {config_file stdout stderr args} { # ASAN_OPTIONS environment variable is for address sanitizer. If a test # tries to allocate huge memory area and expects allocator to return # NULL, address sanitizer throws an error without this setting. - set pid [exec /usr/bin/env ASAN_OPTIONS=allocator_may_return_null=1 {*}$cmd >> $stdout 2>> $stderr &] + set pid [exec /usr/bin/env ASAN_OPTIONS=allocator_may_return_null=1 MSAN_OPTIONS=allocator_may_return_null=1 {*}$cmd >> $stdout 2>> $stderr &] } if {$::wait_server} { diff --git a/tests/support/util.tcl b/tests/support/util.tcl index 0d7d88516..ddf5998f9 100644 --- a/tests/support/util.tcl +++ b/tests/support/util.tcl @@ -70,11 +70,15 @@ proc sanitizer_errors_from_file {filename} { set lines [split [exec cat $filename] "\n"] foreach line $lines { - # Ignore huge allocation warnings + # Ignore huge allocation warnings for both ASan and MSan if ([string match {*WARNING: AddressSanitizer failed to allocate*} $line]) { continue } + if ([string match {*WARNING: MemorySanitizer failed to allocate*} $line]) { + continue + } + # GCC UBSAN output does not contain 'Sanitizer' but 'runtime error'. if {[string match {*runtime error*} $line] || [string match {*Sanitizer*} $line]} {