Re: gcc 15.1 warnings - jsonb_util.c - Mailing list pgsql-hackers

From Tom Lane
Subject Re: gcc 15.1 warnings - jsonb_util.c
Date
Msg-id 1982482.1745601384@sss.pgh.pa.us
Whole thread Raw
In response to gcc 15.1 warnings - jsonb_util.c  (Erik Rijkers <er@xs4all.nl>)
List pgsql-hackers
Erik Rijkers <er@xs4all.nl> writes:
> Compiling gcc 15.1 utters these protests (at least, with
> --enable-cassert) that I don't think I saw with gcc 14:

Hmm, odd.  I tried with gcc 15.0.1 from a fresh Fedora 42 install,
and I can reproduce your result:

jsonb_util.c: In function 'compareJsonbContainers':
jsonb_util.c:301:34: warning: 'va.type' may be used uninitialized [-Wmaybe-uninitialized]
  301 |                         res = (va.type > vb.type) ? 1 : -1;
      |                                ~~^~~~~
jsonb_util.c:202:33: note: 'va' declared here
  202 |                 JsonbValue      va,
      |                                 ^~
jsonb_util.c:301:44: warning: 'vb.type' may be used uninitialized [-Wmaybe-uninitialized]
  301 |                         res = (va.type > vb.type) ? 1 : -1;
      |                                          ~~^~~~~
jsonb_util.c:203:41: note: 'vb' declared here
  203 |                                         vb;
      |                                         ^~

but for me, it only happens with asserts *disabled*.
With --enable-cassert, I think that the preceding bit
about

            Assert(ra != WJB_END_ARRAY && ra != WJB_END_OBJECT);
            Assert(rb != WJB_END_ARRAY && rb != WJB_END_OBJECT);

is sufficient to persuade the compiler that va.type and vb.type
must have been set by JsonbIteratorNext?  Although AFAICS it
should not have been, in view of the first "return" in
JsonbIteratorNext:

JsonbIteratorNext(JsonbIterator **it, JsonbValue *val, bool skipNested)
{
    if (*it == NULL)
        return WJB_DONE;

TBH I think compareJsonbContainers is too cute for its own good.
The comment at line 280ff explains why it's safe, but no compiler or
static analyzer in the world is going to duplicate that reasoning,
especially in a non-assert build.  And it just seems fragile.
(I notice we've had to fix up this logic once already.)

I'm inclined to add manual initializations of va.type and vb.type
just to silence such warnings.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Summarizing indexes allowing single-phase VACUUM?
Next
From: Antonin Houska
Date:
Subject: Re: Conflicting updates of command progress