public inbox for [email protected]  
help / color / mirror / Atom feed
From: 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 00:43:12 +0530
Message-ID: <CAJTYsWXjhJnm-CQYve=qeMWrr+adwvFDLX1W-qyTVq90_p7WCA@mail.gmail.com> (raw)
In-Reply-To: <3nuudxv365kjnmwjhnygdakhxuktpdjvf26rt26eb44esgqdrj@y2x3vomkrfoo>
References: <3nuudxv365kjnmwjhnygdakhxuktpdjvf26rt26eb44esgqdrj@y2x3vomkrfoo>

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


view thread (3+ messages)  latest in thread

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: <CAJTYsWXjhJnm-CQYve=qeMWrr+adwvFDLX1W-qyTVq90_p7WCA@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