public inbox for [email protected]
help / color / mirror / Atom feedSpurious warnings in crypto-des.c when building with gcc-16 -O3
3+ messages / 2 participants
[nested] [flat]
* Spurious warnings in crypto-des.c when building with gcc-16 -O3
@ 2026-04-29 17:47 Andres Freund <[email protected]>
2026-04-29 19:13 ` Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3 Ayush Tiwari <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Andres Freund @ 2026-04-29 17:47 UTC (permalink / raw)
To: pgsql-hackers
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.
Am I missing something or is what I suggest equivalent? Any reason to not
change it that way, both to clarify the code and to work around the spurious
warning?
Greetings,
Andres Freund
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113664
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3
2026-04-29 17:47 Spurious warnings in crypto-des.c when building with gcc-16 -O3 Andres Freund <[email protected]>
@ 2026-04-29 19:13 ` Ayush Tiwari <[email protected]>
2026-04-30 14:29 ` Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3 Ayush Tiwari <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: Ayush Tiwari @ 2026-04-29 19:13 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: pgsql-hackers
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.
>
>
> Am I missing something or is what I suggest equivalent? Any reason to not
> change it that way, both to clarify the code and to work around the
> spurious
> warning?
>
I got the warning with gcc-16 that you are pointing out.
Your proposed rewrite looks equivalent to me. The current condition:
q - (uint8 *) keybuf - 8
is a rather obscure way to say that the loop should stop once eight bytes
have been written, and it seems plausible that gcc is getting confused by
that form.
There is one similar loop a little further down in the extended-DES path:
while (q - (uint8 *) keybuf - 8 && *key)
*q++ ^= *key++ << 1;
That one did not warn in my build, probably because of the additional
data-dependent "*key" condition, but it uses the same idiom. It might be
worth rewriting both loops to use an explicit sizeof(keybuf)-bounded for
loop, for readability and consistency.
Regards,
Ayush
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3
2026-04-29 17:47 Spurious warnings in crypto-des.c when building with gcc-16 -O3 Andres Freund <[email protected]>
2026-04-29 19:13 ` Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3 Ayush Tiwari <[email protected]>
@ 2026-04-30 14:29 ` Ayush Tiwari <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Ayush Tiwari @ 2026-04-30 14:29 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-04-30 14:29 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-29 17:47 Spurious warnings in crypto-des.c when building with gcc-16 -O3 Andres Freund <[email protected]>
2026-04-29 19:13 ` Ayush Tiwari <[email protected]>
2026-04-30 14:29 ` Ayush Tiwari <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox