• sundb's avatar
    Fix some compile warnings and errors when building with gcc-12 or clang (#12035) · 42c8c618
    sundb authored
    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.
    42c8c618
redis-cli.c 371 KB