diff options
author | sundb <sundbcn@gmail.com> | 2023-04-18 14:53:51 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-04-18 09:53:51 +0300 |
commit | 42c8c61813a8c3287b8acb3155473551a491aa2b (patch) | |
tree | 3d29465026577850d9a0e1639422c3065160c092 /src/redis-cli.c | |
parent | d2db4aa7532885fb0eaf9ce7db15af28feb66b05 (diff) | |
download | redis-42c8c61813a8c3287b8acb3155473551a491aa2b.tar.gz |
Fix some compile warnings and errors when building with gcc-12 or clang (#12035)
This PR is to fix the compilation warnings and errors generated by the latest
complier toolchain, and to add a new runner of the latest toolchain for daily CI.
## Fix various compilation warnings and errors
1) jemalloc.c
COMPILER: clang-14 with FORTIFY_SOURCE
WARNING:
```
src/jemalloc.c:1028:7: warning: suspicious concatenation of string literals in an array initialization; did you mean to separate the elements with a comma? [-Wstring-concatenation]
"/etc/malloc.conf",
^
src/jemalloc.c:1027:3: note: place parentheses around the string literal to silence warning
"\"name\" of the file referenced by the symbolic link named "
^
```
REASON: the compiler to alert developers to potential issues with string concatenation
that may miss a comma,
just like #9534 which misses a comma.
SOLUTION: use `()` to tell the compiler that these two line strings are continuous.
2) config.h
COMPILER: clang-14 with FORTIFY_SOURCE
WARNING:
```
In file included from quicklist.c:36:
./config.h:319:76: warning: attribute declaration must precede definition [-Wignored-attributes]
char *strcat(char *restrict dest, const char *restrict src) __attribute__((deprecated("please avoid use of unsafe C functions. prefer use of redis_strlcat instead")));
```
REASON: Enabling _FORTIFY_SOURCE will cause the compiler to use `strcpy()` with check,
it results in a deprecated attribute declaration after including <features.h>.
SOLUTION: move the deprecated attribute declaration from config.h to fmacro.h before "#include <features.h>".
3) networking.c
COMPILER: GCC-12
WARNING:
```
networking.c: In function ‘addReplyDouble.part.0’:
networking.c:876:21: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
876 | dbuf[start] = '$';
| ^
networking.c:868:14: note: at offset -5 into destination object ‘dbuf’ of size 5152
868 | char dbuf[MAX_LONG_DOUBLE_CHARS+32];
| ^
networking.c:876:21: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
876 | dbuf[start] = '$';
| ^
networking.c:868:14: note: at offset -6 into destination object ‘dbuf’ of size 5152
868 | char dbuf[MAX_LONG_DOUBLE_CHARS+32];
```
REASON: GCC-12 predicts that digits10() may return 9 or 10 through `return 9 + (v >= 1000000000UL)`.
SOLUTION: add an assert to let the compiler know the possible length;
4) redis-cli.c & redis-benchmark.c
COMPILER: clang-14 with FORTIFY_SOURCE
WARNING:
```
redis-benchmark.c:1621:2: warning: embedding a directive within macro arguments has undefined behavior [-Wembedded-directive] #ifdef USE_OPENSSL
redis-cli.c:3015:2: warning: embedding a directive within macro arguments has undefined behavior [-Wembedded-directive] #ifdef USE_OPENSSL
```
REASON: when _FORTIFY_SOURCE is enabled, the compiler will use the print() with
check, which is a macro. this may result in the use of directives within the macro, which
is undefined behavior.
SOLUTION: move the directives-related code out of `print()`.
5) server.c
COMPILER: gcc-13 with FORTIFY_SOURCE
WARNING:
```
In function 'lookupCommandLogic',
inlined from 'lookupCommandBySdsLogic' at server.c:3139:32:
server.c:3102:66: error: '*(robj **)argv' may be used uninitialized [-Werror=maybe-uninitialized]
3102 | struct redisCommand *base_cmd = dictFetchValue(commands, argv[0]->ptr);
| ~~~~^~~
```
REASON: The compiler thinks that the `argc` returned by `sdssplitlen()` could be 0,
resulting in an empty array of size 0 being passed to lookupCommandLogic.
this should be a false positive, `argc` can't be 0 when strings are not NULL.
SOLUTION: add an assert to let the compiler know that `argc` is positive.
6) sha1.c
COMPILER: gcc-12
WARNING:
```
In function ‘SHA1Update’,
inlined from ‘SHA1Final’ at sha1.c:195:5:
sha1.c:152:13: warning: ‘SHA1Transform’ reading 64 bytes from a region of size 0 [-Wstringop-overread]
152 | SHA1Transform(context->state, &data[i]);
| ^
sha1.c:152:13: note: referencing argument 2 of type ‘const unsigned char[64]’
sha1.c: In function ‘SHA1Final’:
sha1.c:56:6: note: in a call to function ‘SHA1Transform’
56 | void SHA1Transform(uint32_t state[5], const unsigned char buffer[64])
| ^
In function ‘SHA1Update’,
inlined from ‘SHA1Final’ at sha1.c:198:9:
sha1.c:152:13: warning: ‘SHA1Transform’ reading 64 bytes from a region of size 0 [-Wstringop-overread]
152 | SHA1Transform(context->state, &data[i]);
| ^
sha1.c:152:13: note: referencing argument 2 of type ‘const unsigned char[64]’
sha1.c: In function ‘SHA1Final’:
sha1.c:56:6: note: in a call to function ‘SHA1Transform’
56 | void SHA1Transform(uint32_t state[5], const unsigned char buffer[64])
```
REASON: due to the bug[https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922], when
enable LTO, gcc-12 will not see `diagnostic ignored "-Wstringop-overread"`, resulting in a warning.
SOLUTION: temporarily set SHA1Update to noinline to avoid compiler warnings due
to LTO being enabled until the above gcc bug is fixed.
7) zmalloc.h
COMPILER: GCC-12
WARNING:
```
In function ‘memset’,
inlined from ‘moduleCreateContext’ at module.c:877:5,
inlined from ‘RM_GetDetachedThreadSafeContext’ at module.c:8410:5:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:59:10: warning: ‘__builtin_memset’ writing 104 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
59 | return __builtin___memset_chk (__dest, __ch, __len,
```
REASON: due to the GCC-12 bug [https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96503],
GCC-12 cannot see alloc_size, which causes GCC to think that the actual size of memory
is 0 when checking with __glibc_objsize0().
SOLUTION: temporarily set malloc-related interfaces to `noinline` to avoid compiler warnings
due to LTO being enabled until the above gcc bug is fixed.
## Other changes
1) Fixed `ps -p [pid]` doesn't output `<defunct>` when using procps 4.x causing `replication
child dies when parent is killed - diskless` test to fail.
2) Add a new fortify CI with GCC-13 and ubuntu-lunar docker image.
Diffstat (limited to 'src/redis-cli.c')
-rw-r--r-- | src/redis-cli.c | 47 |
1 files changed, 26 insertions, 21 deletions
diff --git a/src/redis-cli.c b/src/redis-cli.c index 2bb2d6e67..ad2c79840 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -2981,6 +2981,29 @@ static void parseEnv() { static void usage(int err) { sds version = cliVersion(); FILE *target = err ? stderr: stdout; + const char *tls_usage = +#ifdef USE_OPENSSL +" --tls Establish a secure TLS connection.\n" +" --sni <host> Server name indication for TLS.\n" +" --cacert <file> CA Certificate file to verify with.\n" +" --cacertdir <dir> Directory where trusted CA certificates are stored.\n" +" If neither cacert nor cacertdir are specified, the default\n" +" system-wide trusted root certs configuration will apply.\n" +" --insecure Allow insecure TLS connection by skipping cert validation.\n" +" --cert <file> Client certificate to authenticate with.\n" +" --key <file> Private key file to authenticate with.\n" +" --tls-ciphers <list> Sets the list of preferred ciphers (TLSv1.2 and below)\n" +" in order of preference from highest to lowest separated by colon (\":\").\n" +" See the ciphers(1ssl) manpage for more information about the syntax of this string.\n" +#ifdef TLS1_3_VERSION +" --tls-ciphersuites <list> Sets the list of preferred ciphersuites (TLSv1.3)\n" +" in order of preference from highest to lowest separated by colon (\":\").\n" +" See the ciphers(1ssl) manpage for more information about the syntax of this string,\n" +" and specifically for TLSv1.3 ciphersuites.\n" +#endif +#endif +""; + fprintf(target, "redis-cli %s\n" "\n" @@ -3012,26 +3035,7 @@ static void usage(int err) { " -D <delimiter> Delimiter between responses for raw formatting (default: \\n).\n" " -c Enable cluster mode (follow -ASK and -MOVED redirections).\n" " -e Return exit error code when command execution fails.\n" -#ifdef USE_OPENSSL -" --tls Establish a secure TLS connection.\n" -" --sni <host> Server name indication for TLS.\n" -" --cacert <file> CA Certificate file to verify with.\n" -" --cacertdir <dir> Directory where trusted CA certificates are stored.\n" -" If neither cacert nor cacertdir are specified, the default\n" -" system-wide trusted root certs configuration will apply.\n" -" --insecure Allow insecure TLS connection by skipping cert validation.\n" -" --cert <file> Client certificate to authenticate with.\n" -" --key <file> Private key file to authenticate with.\n" -" --tls-ciphers <list> Sets the list of preferred ciphers (TLSv1.2 and below)\n" -" in order of preference from highest to lowest separated by colon (\":\").\n" -" See the ciphers(1ssl) manpage for more information about the syntax of this string.\n" -#ifdef TLS1_3_VERSION -" --tls-ciphersuites <list> Sets the list of preferred ciphersuites (TLSv1.3)\n" -" in order of preference from highest to lowest separated by colon (\":\").\n" -" See the ciphers(1ssl) manpage for more information about the syntax of this string,\n" -" and specifically for TLSv1.3 ciphersuites.\n" -#endif -#endif +"%s" " --raw Use raw formatting for replies (default when STDOUT is\n" " not a tty).\n" " --no-raw Force formatted output even when STDOUT is not a tty.\n" @@ -3041,7 +3045,8 @@ static void usage(int err) { " --quoted-json Same as --json, but produce ASCII-safe quoted strings, not Unicode.\n" " --show-pushes <yn> Whether to print RESP3 PUSH messages. Enabled by default when\n" " STDOUT is a tty but can be overridden with --show-pushes no.\n" -" --stat Print rolling stats about server: mem, clients, ...\n",version); +" --stat Print rolling stats about server: mem, clients, ...\n", +version,tls_usage); fprintf(target, " --latency Enter a special mode continuously sampling latency.\n" |