Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3

Hi,

>
>
> On Wed, 29 Apr 2026 at 23:17, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
>> Hi,
>>
>> Recently I got a bit of a shock building postgres with gcc-16:
>>
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:
>> In function ‘px_crypt_des’:
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:675:22:
>> warning: writing 8 bytes into a region of size 7 [-Wstringop-overflow=]
>> 675 | *q++ = *key << 1;
>> | ~~~~~^~~~~~~~~~~
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [1, 2] into destination object ‘keybuf’ of size 8
>> 659 | keybuf[2];
>> | ^~~~~~
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [2, 3] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [3, 4] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [4, 5] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [5, 6] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [6, 7] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [7, 8] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:675:22:
>> warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
>> 675 | *q++ = *key << 1;
>> | ~~~~~^~~~~~~~~~~
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset 8 into destination object ‘keybuf’ of size 8
>> 659 | keybuf[2];
>> | ^~~~~~
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [9, 10] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [10, 11] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [11, 12] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [12, 13] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [13, 14] into destination object ‘keybuf’ of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:33:
>> note: at offset [14, 15] into destination object ‘keybuf’ of size 8
>>
>>
>> Luckily it turns out that that the warning is spurious, due to a bug in
>> gcc
>> [1].
>>
>> However, it took me quite a while to figure out what the hell the code was
>> doing:
>>
>> char *
>> px_crypt_des(const char *key, const char *setting)
>> {
>> uint32 keybuf[2];
>> ...
>> uint8 *q;
>> ...
>>
>> /*
>> * Copy the key, shifting each character up by one bit and padding
>> with
>> * zeros.
>> */
>> q = (uint8 *) keybuf;
>> while (q - (uint8 *) keybuf - 8)
>> {
>> *q++ = *key << 1;
>> if (*key != '\0')
>> key++;
>> }
>>
>> Like, it's far from immediately obvious where the 8 is coming from (it's
>> the
>> size of keybuf), whether there are precedence issues or what this is even
>> trying to achieve.
>>
>> And it's still not clear to me why on earth it makes sense to write it
>> that
>> complicated, when it seems something like
>>
>> for (int byteno = 0; byteno < sizeof(keybuf); byteno++)
>> {
>> *q++ = *key << 1;
>> if (*key != '\0')
>> key++;
>> }
>>
>> would do the same thing, except be trivially understandable for humans and
>> compilers.
>>
>>
I've created a patch for it.

It replaces the obscure "while (q - (uint8 *) keybuf - 8)" loop
conditions in px_crypt_des() with for-loops bounded by
sizeof(keybuf). To avoid introducing a new -Wsign-compare warning
against sizeof, I used the new loop counter (bytenum) as size_t.

The logic remains equivalent, it preserves the exact
iteration counts and the *key short-circuit in the extended-DES
loop, but makes the bounds obvious to both readers and the compiler.

Please find the patch attached. Thoughts?

Regards,
Ayush