public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ayush Tiwari <[email protected]>
To: Andres Freund <[email protected]>
Cc: [email protected]
Subject: Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3
Date: Thu, 30 Apr 2026 19:59:38 +0530
Message-ID: <CAJTYsWWkNO0fKZit+_hRUUgrP-Gyu6pGDT5w0w+4dYOM7HMF1Q@mail.gmail.com> (raw)
In-Reply-To: <CAJTYsWXjhJnm-CQYve=qeMWrr+adwvFDLX1W-qyTVq90_p7WCA@mail.gmail.com>
References: <3nuudxv365kjnmwjhnygdakhxuktpdjvf26rt26eb44esgqdrj@y2x3vomkrfoo>
<CAJTYsWXjhJnm-CQYve=qeMWrr+adwvFDLX1W-qyTVq90_p7WCA@mail.gmail.com>
Hi,
>
>
> On Wed, 29 Apr 2026 at 23:17, Andres Freund <[email protected]> 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
Attachments:
[application/octet-stream] v1-0001-Avoid-obscure-DES-key-buffer-loop-bounds.patch (1.7K, 3-v1-0001-Avoid-obscure-DES-key-buffer-loop-bounds.patch)
download | inline diff:
From 88d7fbd07b2094c101fdd2e8810becfb88db3ec1 Mon Sep 17 00:00:00 2001
From: Ayush Tiwari <[email protected]>
Date: Thu, 30 Apr 2026 02:14:53 +0000
Subject: [PATCH] Avoid obscure DES key buffer loop bounds
px_crypt_des() used pointer subtraction expressions such as
"q - (uint8 *) keybuf - 8" as loop conditions when filling or
updating the eight-byte key buffer. While correct, that form is hard
to read and confuses gcc 16 at -O3 into emitting spurious
-Wstringop-overflow warnings.
Use explicit sizeof(keybuf)-bounded for loops instead. This preserves
the number of bytes processed, makes the bound visible to the
compiler, and applies the clearer form to the similar extended-DES
loop as well.
---
contrib/pgcrypto/crypt-des.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/contrib/pgcrypto/crypt-des.c b/contrib/pgcrypto/crypt-des.c
index 98c30ea122e..f556849cbfc 100644
--- a/contrib/pgcrypto/crypt-des.c
+++ b/contrib/pgcrypto/crypt-des.c
@@ -651,6 +651,7 @@ char *
px_crypt_des(const char *key, const char *setting)
{
int i;
+ size_t bytenum;
uint32 count,
salt,
l,
@@ -670,7 +671,7 @@ px_crypt_des(const char *key, const char *setting)
* zeros.
*/
q = (uint8 *) keybuf;
- while (q - (uint8 *) keybuf - 8)
+ for (bytenum = 0; bytenum < sizeof(keybuf); bytenum++)
{
*q++ = *key << 1;
if (*key != '\0')
@@ -714,7 +715,9 @@ px_crypt_des(const char *key, const char *setting)
* And XOR with the next 8 characters of the key.
*/
q = (uint8 *) keybuf;
- while (q - (uint8 *) keybuf - 8 && *key)
+ for (bytenum = 0;
+ bytenum < sizeof(keybuf) && *key;
+ bytenum++)
*q++ ^= *key++ << 1;
if (des_setkey((char *) keybuf))
--
2.43.0
view thread (3+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3
In-Reply-To: <CAJTYsWWkNO0fKZit+_hRUUgrP-Gyu6pGDT5w0w+4dYOM7HMF1Q@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox