MIME-Version: 1.0
References: <3nuudxv365kjnmwjhnygdakhxuktpdjvf26rt26eb44esgqdrj@y2x3vomkrfoo>
 <CAJTYsWXjhJnm-CQYve=qeMWrr+adwvFDLX1W-qyTVq90_p7WCA@mail.gmail.com>
In-Reply-To: 
 <CAJTYsWXjhJnm-CQYve=qeMWrr+adwvFDLX1W-qyTVq90_p7WCA@mail.gmail.com>
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Thu, 30 Apr 2026 19:59:38 +0530
Message-ID: 
 <CAJTYsWWkNO0fKZit+_hRUUgrP-Gyu6pGDT5w0w+4dYOM7HMF1Q@mail.gmail.com>
Subject: Re: Spurious warnings in crypto-des.c when building with gcc-16 -O3
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org
Content-Type: multipart/mixed; boundary="000000000000016a140650ae4c69"
Archived-At: 
 <https://www.postgresql.org/message-id/CAJTYsWWkNO0fKZit%2B_hRUUgrP-Gyu6pGDT5w0w%2B4dYOM7HMF1Q%40mail.gmail.com>
Precedence: bulk

--000000000000016a140650ae4c69
Content-Type: multipart/alternative; boundary="000000000000016a130650ae4c67"

--000000000000016a130650ae4c67
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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 =E2=80=98px_crypt_des=E2=80=99:
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
75:22:
>> warning: writing 8 bytes into a region of size 7 [-Wstringop-overflow=3D=
]
>>   675 |                 *q++ =3D *key << 1;
>>       |                 ~~~~~^~~~~~~~~~~
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [1, 2] into destination object =E2=80=98keybuf=E2=80=99 =
of size 8
>>   659 |                                 keybuf[2];
>>       |                                 ^~~~~~
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [2, 3] into destination object =E2=80=98keybuf=E2=80=99 =
of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [3, 4] into destination object =E2=80=98keybuf=E2=80=99 =
of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [4, 5] into destination object =E2=80=98keybuf=E2=80=99 =
of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [5, 6] into destination object =E2=80=98keybuf=E2=80=99 =
of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [6, 7] into destination object =E2=80=98keybuf=E2=80=99 =
of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [7, 8] into destination object =E2=80=98keybuf=E2=80=99 =
of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
75:22:
>> warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=3D]
>>   675 |                 *q++ =3D *key << 1;
>>       |                 ~~~~~^~~~~~~~~~~
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset 8 into destination object =E2=80=98keybuf=E2=80=99 of si=
ze 8
>>   659 |                                 keybuf[2];
>>       |                                 ^~~~~~
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [9, 10] into destination object =E2=80=98keybuf=E2=80=99=
 of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [10, 11] into destination object =E2=80=98keybuf=E2=80=
=99 of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [11, 12] into destination object =E2=80=98keybuf=E2=80=
=99 of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [12, 13] into destination object =E2=80=98keybuf=E2=80=
=99 of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [13, 14] into destination object =E2=80=98keybuf=E2=80=
=99 of size 8
>> ../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:6=
59:33:
>> note: at offset [14, 15] into destination object =E2=80=98keybuf=E2=80=
=99 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 w=
as
>> 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 =3D (uint8 *) keybuf;
>>     while (q - (uint8 *) keybuf - 8)
>>     {
>>         *q++ =3D *key << 1;
>>         if (*key !=3D '\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 eve=
n
>> 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 =3D 0; byteno < sizeof(keybuf); byteno++)
>>     {
>>         *q++ =3D *key << 1;
>>         if (*key !=3D '\0')
>>             key++;
>>     }
>>
>> would do the same thing, except be trivially understandable for humans a=
nd
>> 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

--000000000000016a130650ae4c67
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi,=C2=A0</div><div class=3D"gmail_quote =
gmail_quote_container"><blockquote class=3D"gmail_quote" style=3D"margin:0p=
x 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><d=
iv dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=
=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">On Wed, 29 Apr 2026 =
at 23:17, Andres Freund &lt;<a href=3D"mailto:andres@anarazel.de" target=3D=
"_blank">andres@anarazel.de</a>&gt; wrote:<br></div><blockquote class=3D"gm=
ail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,=
204,204);padding-left:1ex">Hi,<br>
<br>
Recently I got a bit of a shock building postgres with gcc-16:<br>
<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c: In =
function =E2=80=98px_crypt_des=E2=80=99:<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:675:=
22: warning: writing 8 bytes into a region of size 7 [-Wstringop-overflow=
=3D]<br>
=C2=A0 675 |=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*=
q++ =3D *key &lt;&lt; 1;<br>
=C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0~~~~~^~~~~~~~~~~<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [1, 2] into destination object =E2=80=98keybuf=E2=80=99=
 of size 8<br>
=C2=A0 659 |=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0keybuf[2];<br>
=C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0^~~~~~<br=
>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [2, 3] into destination object =E2=80=98keybuf=E2=80=99=
 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [3, 4] into destination object =E2=80=98keybuf=E2=80=99=
 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [4, 5] into destination object =E2=80=98keybuf=E2=80=99=
 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [5, 6] into destination object =E2=80=98keybuf=E2=80=99=
 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [6, 7] into destination object =E2=80=98keybuf=E2=80=99=
 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [7, 8] into destination object =E2=80=98keybuf=E2=80=99=
 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:675:=
22: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=3D=
]<br>
=C2=A0 675 |=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*=
q++ =3D *key &lt;&lt; 1;<br>
=C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0~~~~~^~~~~~~~~~~<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset 8 into destination object =E2=80=98keybuf=E2=80=99 of s=
ize 8<br>
=C2=A0 659 |=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0keybuf[2];<br>
=C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0^~~~~~<br=
>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [9, 10] into destination object =E2=80=98keybuf=E2=80=
=99 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [10, 11] into destination object =E2=80=98keybuf=E2=80=
=99 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [11, 12] into destination object =E2=80=98keybuf=E2=80=
=99 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [12, 13] into destination object =E2=80=98keybuf=E2=80=
=99 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [13, 14] into destination object =E2=80=98keybuf=E2=80=
=99 of size 8<br>
../../../../../home/andres/src/postgresql/contrib/pgcrypto/crypt-des.c:659:=
33: note: at offset [14, 15] into destination object =E2=80=98keybuf=E2=80=
=99 of size 8<br>
<br>
<br>
Luckily it turns out that that the warning is spurious, due to a bug in gcc=
<br>
[1].<br>
<br>
However, it took me quite a while to figure out what the hell the code was<=
br>
doing:<br>
<br>
char *<br>
px_crypt_des(const char *key, const char *setting)<br>
{<br>
=C2=A0 =C2=A0 uint32 keybuf[2];<br>
...<br>
=C2=A0 =C2=A0 uint8=C2=A0 =C2=A0 =C2=A0 *q;<br>
...<br>
<br>
=C2=A0 =C2=A0 /*<br>
=C2=A0 =C2=A0 =C2=A0* Copy the key, shifting each character up by one bit a=
nd padding with<br>
=C2=A0 =C2=A0 =C2=A0* zeros.<br>
=C2=A0 =C2=A0 =C2=A0*/<br>
=C2=A0 =C2=A0 q =3D (uint8 *) keybuf;<br>
=C2=A0 =C2=A0 while (q - (uint8 *) keybuf - 8)<br>
=C2=A0 =C2=A0 {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 *q++ =3D *key &lt;&lt; 1;<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (*key !=3D &#39;\0&#39;)<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 key++;<br>
=C2=A0 =C2=A0 }<br>
<br>
Like, it&#39;s far from immediately obvious where the 8 is coming from (it&=
#39;s the<br>
size of keybuf), whether there are precedence issues or what this is even<b=
r>
trying to achieve.<br>
<br>
And it&#39;s still not clear to me why on earth it makes sense to write it =
that<br>
complicated, when it seems something like<br>
<br>
=C2=A0 =C2=A0 for (int byteno =3D 0; byteno &lt; sizeof(keybuf); byteno++)<=
br>
=C2=A0 =C2=A0 {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 *q++ =3D *key &lt;&lt; 1;<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (*key !=3D &#39;\0&#39;)<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 key++;<br>
=C2=A0 =C2=A0 }<br>
<br>
would do the same thing, except be trivially understandable for humans and<=
br>
compilers.<br><br></blockquote></div></div></div></blockquote><div><br></di=
v><div>I&#39;ve created a patch for it.<br><br>It replaces the obscure &quo=
t;while (q - (uint8 *) keybuf - 8)&quot; loop<br>conditions in px_crypt_des=
() with for-loops bounded by<br>sizeof(keybuf). To avoid introducing a new =
-Wsign-compare warning<br>against sizeof, I used the new loop counter (byte=
num) as size_t.<br><br>The logic remains equivalent, it preserves the exact=
<br>iteration counts and the *key short-circuit in the extended-DES<br>loop=
, but makes the bounds obvious to both readers and the compiler.<br><br>Ple=
ase find the patch attached. Thoughts?<br><br>Regards,<br>Ayush</div></div>=
</div>

--000000000000016a130650ae4c67--

--000000000000016a140650ae4c69
Content-Type: application/octet-stream;
	name="v1-0001-Avoid-obscure-DES-key-buffer-loop-bounds.patch"
Content-Disposition: attachment;
	filename="v1-0001-Avoid-obscure-DES-key-buffer-loop-bounds.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_molkwf8c0>
X-Attachment-Id: f_molkwf8c0

RnJvbSA4OGQ3ZmJkMDdiMjA5NGMxMDFmZGQyZTg4MTBiZWNmYjg4ZGIzZWMxIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBBeXVzaCBUaXdhcmkgPGF5dXNodGl3YXJpLnNsZzAxQGdtYWls
LmNvbT4KRGF0ZTogVGh1LCAzMCBBcHIgMjAyNiAwMjoxNDo1MyArMDAwMApTdWJqZWN0OiBbUEFU
Q0hdIEF2b2lkIG9ic2N1cmUgREVTIGtleSBidWZmZXIgbG9vcCBib3VuZHMKCnB4X2NyeXB0X2Rl
cygpIHVzZWQgcG9pbnRlciBzdWJ0cmFjdGlvbiBleHByZXNzaW9ucyBzdWNoIGFzCiJxIC0gKHVp
bnQ4ICopIGtleWJ1ZiAtIDgiIGFzIGxvb3AgY29uZGl0aW9ucyB3aGVuIGZpbGxpbmcgb3IKdXBk
YXRpbmcgdGhlIGVpZ2h0LWJ5dGUga2V5IGJ1ZmZlci4gIFdoaWxlIGNvcnJlY3QsIHRoYXQgZm9y
bSBpcyBoYXJkCnRvIHJlYWQgYW5kIGNvbmZ1c2VzIGdjYyAxNiBhdCAtTzMgaW50byBlbWl0dGlu
ZyBzcHVyaW91cwotV3N0cmluZ29wLW92ZXJmbG93IHdhcm5pbmdzLgoKVXNlIGV4cGxpY2l0IHNp
emVvZihrZXlidWYpLWJvdW5kZWQgZm9yIGxvb3BzIGluc3RlYWQuICBUaGlzIHByZXNlcnZlcwp0
aGUgbnVtYmVyIG9mIGJ5dGVzIHByb2Nlc3NlZCwgbWFrZXMgdGhlIGJvdW5kIHZpc2libGUgdG8g
dGhlCmNvbXBpbGVyLCBhbmQgYXBwbGllcyB0aGUgY2xlYXJlciBmb3JtIHRvIHRoZSBzaW1pbGFy
IGV4dGVuZGVkLURFUwpsb29wIGFzIHdlbGwuCi0tLQogY29udHJpYi9wZ2NyeXB0by9jcnlwdC1k
ZXMuYyB8IDcgKysrKystLQogMSBmaWxlIGNoYW5nZWQsIDUgaW5zZXJ0aW9ucygrKSwgMiBkZWxl
dGlvbnMoLSkKCmRpZmYgLS1naXQgYS9jb250cmliL3BnY3J5cHRvL2NyeXB0LWRlcy5jIGIvY29u
dHJpYi9wZ2NyeXB0by9jcnlwdC1kZXMuYwppbmRleCA5OGMzMGVhMTIyZS4uZjU1Njg0OWNiZmMg
MTAwNjQ0Ci0tLSBhL2NvbnRyaWIvcGdjcnlwdG8vY3J5cHQtZGVzLmMKKysrIGIvY29udHJpYi9w
Z2NyeXB0by9jcnlwdC1kZXMuYwpAQCAtNjUxLDYgKzY1MSw3IEBAIGNoYXIgKgogcHhfY3J5cHRf
ZGVzKGNvbnN0IGNoYXIgKmtleSwgY29uc3QgY2hhciAqc2V0dGluZykKIHsKIAlpbnQJCQlpOwor
CXNpemVfdAkJYnl0ZW51bTsKIAl1aW50MzIJCWNvdW50LAogCQkJCXNhbHQsCiAJCQkJbCwKQEAg
LTY3MCw3ICs2NzEsNyBAQCBweF9jcnlwdF9kZXMoY29uc3QgY2hhciAqa2V5LCBjb25zdCBjaGFy
ICpzZXR0aW5nKQogCSAqIHplcm9zLgogCSAqLwogCXEgPSAodWludDggKikga2V5YnVmOwotCXdo
aWxlIChxIC0gKHVpbnQ4ICopIGtleWJ1ZiAtIDgpCisJZm9yIChieXRlbnVtID0gMDsgYnl0ZW51
bSA8IHNpemVvZihrZXlidWYpOyBieXRlbnVtKyspCiAJewogCQkqcSsrID0gKmtleSA8PCAxOwog
CQlpZiAoKmtleSAhPSAnXDAnKQpAQCAtNzE0LDcgKzcxNSw5IEBAIHB4X2NyeXB0X2Rlcyhjb25z
dCBjaGFyICprZXksIGNvbnN0IGNoYXIgKnNldHRpbmcpCiAJCQkgKiBBbmQgWE9SIHdpdGggdGhl
IG5leHQgOCBjaGFyYWN0ZXJzIG9mIHRoZSBrZXkuCiAJCQkgKi8KIAkJCXEgPSAodWludDggKikg
a2V5YnVmOwotCQkJd2hpbGUgKHEgLSAodWludDggKikga2V5YnVmIC0gOCAmJiAqa2V5KQorCQkJ
Zm9yIChieXRlbnVtID0gMDsKKwkJCQkgYnl0ZW51bSA8IHNpemVvZihrZXlidWYpICYmICprZXk7
CisJCQkJIGJ5dGVudW0rKykKIAkJCQkqcSsrIF49ICprZXkrKyA8PCAxOwogCiAJCQlpZiAoZGVz
X3NldGtleSgoY2hhciAqKSBrZXlidWYpKQotLSAKMi40My4wCgo=
--000000000000016a140650ae4c69--