some gcc 15.2.1 warnings

[ZerBea]

As requested, some (harmless) gcc (15.2.1) warnings:

unrar.c: In function 'read_tables':
unrar.c:500:56: warning: writing 16 bytes into a region of size 4 [-Wstringop-overflow=]
  500 |                                         bit_length[i++]=0;
      |                                                        ^
unrar.c:450:23: note: at offset [16, 20] into destination object 'bit_length' of size 20
  450 |         unsigned char bit_length[BC];
      |                       ^
options.c: In function 'opt_init':
options.c:1030:27: warning: 'strchr' reading 1 or more bytes from a region of size 0 [-Wstringop-overread]
 1030 |                 char *e = strchr(s + 1, ':');
      |                           ^
In function 'check_huffman',
    inlined from 'check_rar' at rar_common.h:654:10:
rar_common.h:526:57: warning: writing 16 bytes into a region of size 4 [-Wstringop-overflow=]
  526 |                                         bit_length[i++] = 0;
      |                                                         ^
rar_common.h: In function 'check_rar':
rar_common.h:501:23: note: at offset [16, 20] into destination object 'bit_length' of size 20
  501 |         unsigned char bit_length[20];
      |                       ^
In function 'check_huffman',
    inlined from 'check_rar' at rar_common.h:654:10:
rar_common.h:526:57: warning: writing 16 bytes into a region of size 4 [-Wstringop-overflow=]
  526 |                                         bit_length[i++] = 0;
      |                                                         ^
rar_common.h: In function 'check_rar':
rar_common.h:501:23: note: at offset [16, 20] into destination object 'bit_length' of size 20
  501 |         unsigned char bit_length[20];
      |                       ^

[solardiz]

In the RAR code, these are false positives because the array accesses are right after checks of i < sizeof(bit_length) / sizeof(bit_length[0]). I don't know what's the best way to silence these warnings, I defer to @magnumripper who worked on this code.

In options.c, I can see how the logic is wrong in case the string is empty, which it probably can't be in this place, but we'd better make the code more robust anyway. Also the if that follows looks like UB to me in case the strchr returns NULL (somehow this isn't flagged by the compiler yet):

        /*
         * The format should never have been a parameter to --regen-lost-salts but now that we have to live with it:
         * If --regen-lost-salts=TYPE:hash_sz:mask and no --format option was given, infer --format=TYPE.
         * If on the other hand --format=TYPE *was* given, require that they actually match.
         */
        if (options.regen_lost_salts) {
                char *s = str_alloc_copy(regen_salts_options);
                char *e = strchr(s + 1, ':');

                if (e > s + 8) {

We probably need e && e > s + 8 there, as I think comparing NULL for "greater than" is UB. But also s + 8 may be beyond the end of object, so we have another instance of UB there, and probably need to write this in the form of e && e - s > 8.

As to the warning we did get, perhaps just drop the + 1 from the strchr call? Alternatively, if we somehow need to allow for and skip a leading colon on purpose (why?), we can write *s ? strchr(s + 1, ':') : NULL, but I'm unsure the compiler will get it and not warn anymore.

[magnumripper]

I don't get any warnings using Homebrew GCC 15.2.0. Not sure why? I can't believe the .1 patch level would make a difference. I even run with -Werror all the time.

[magnumripper]

ChatGPT describes the unrar case well: "GCC 15.2 has taken the “false positive inflation” crown. It’s basically punishing low-level code for being clearer than its analyzer can handle."

[ZerBea]

@magnumripper I agree, gcc is extremely restrictive, but anyway Wstringop-overread is requested. It doesn't make sense to set this warning option but to ignore the warning.

clang (version 21.1.4) is less restrictive and it shows only one warning:

solarwinds_common_plug.c:85:53: warning: destination buffer will always be overflown by source [-Wuser-defined-warnings]
   85 |                 strncat(cs.salt, SALT_PADDING, 8 - strlen(cs.salt));
      |                                                                   ^
/usr/include/bits/string_fortified.h:147:6: note: from 'diagnose_if' attribute on 'strncat':
  147 |      __fortify_clang_warn_if_src_too_large (__dest, __src)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/cdefs.h:298:3: note: expanded from macro '__fortify_clang_warn_if_src_too_large'
  298 |   __fortify_clang_warning (__fortify_clang_size_too_small (__glibc_objsize, \
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  299 |                                                            __dest, \
      |                                                            ~~~~~~~~~
  300 |                                                            __builtin_strlen (__src) + 1), \
      |                                                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  301 |                            "destination buffer will always be overflown by source")
      |                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/cdefs.h:259:19: note: expanded from macro '__fortify_clang_warning'
  259 |   __attribute__ ((__diagnose_if__ ((__c), (__msg), "warning")))
      |                   ^                ~~~~~
1 warning generated.