Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3 - Mailing list pgsql-hackers
| From | Ayush Tiwari |
|---|---|
| Subject | Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3 |
| Date | |
| Msg-id | CAJTYsWWkNO0fKZit+_hRUUgrP-Gyu6pGDT5w0w+4dYOM7HMF1Q@mail.gmail.com Whole thread |
| In response to | Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3 (Ayush Tiwari <ayushtiwari.slg01@gmail.com>) |
| List | pgsql-hackers |
Hi,
On Wed, 29 Apr 2026 at 23:17, Andres Freund <andres@anarazel.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
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
Attachment
pgsql-hackers by date: