Thread

Commits

  1. Use pg_assume() to avoid compiler warning below exec_set_found()

  2. Add pg_assume(expr) macro

  1. gcc 15 "array subscript 0" warning at level -O3

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-04-25T17:37:15Z

    Whilst poking at Erik Rijkers' nearby report, I found that
    Fedora 42's gcc 15.0.1 will produce this complaint if you
    select optimization level -O3:
    
    
    In file included from ../../../../src/include/access/htup_details.h:22,
                     from pl_exec.c:21:
    In function 'assign_simple_var',
        inlined from 'exec_set_found' at pl_exec.c:8609:2:
    ../../../../src/include/varatt.h:230:36: warning: array subscript 0 is outside array bounds of 'char[0]' [-Warray-bounds=]
      230 |         (((varattrib_1b_e *) (PTR))->va_tag)
          |                                    ^
    ../../../../src/include/varatt.h:94:12: note: in definition of macro 'VARTAG_IS_EXPANDED'
       94 |         (((tag) & ~1) == VARTAG_EXPANDED_RO)
          |            ^~~
    ../../../../src/include/varatt.h:284:57: note: in expansion of macro 'VARTAG_1B_E'
      284 | #define VARTAG_EXTERNAL(PTR)                            VARTAG_1B_E(PTR)
          |                                                         ^~~~~~~~~~~
    ../../../../src/include/varatt.h:301:57: note: in expansion of macro 'VARTAG_EXTERNAL'
      301 |         (VARATT_IS_EXTERNAL(PTR) && !VARTAG_IS_EXPANDED(VARTAG_EXTERNAL(PTR)))
          |                                                         ^~~~~~~~~~~~~~~
    pl_exec.c:8797:17: note: in expansion of macro 'VARATT_IS_EXTERNAL_NON_EXPANDED'
     8797 |                 VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    In function 'exec_set_found':
    cc1: note: source object is likely at address zero
    
    
    Buildfarm member serinus has been producing the identical warning for
    some time.  I'd been ignoring that because it runs "experimental gcc",
    but I guess the experiment has leaked out to production distros.
    
    What seems to be happening here is that after inlining
    assign_simple_var into exec_set_found, the compiler decides that
    "newvalue" might be zero (since it's a BoolGetDatum result),
    and then it warns -- in a rather strange way -- about the
    potential null dereference.  The dereference is not reachable
    because of the preceding "var->datatype->typlen == -1" check,
    but that's not stopping the optimizer from bitching.
    
    I experimented with modifying exec_set_found thus:
    
    	var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
    +	Assert(var->datatype->typlen == 1);
    	assign_simple_var(estate, var, BoolGetDatum(state), false, false);
    
    which should be OK since we're expecting the "found" variable to
    be boolean.  That does silence the warning, but of course only
    in --enable-cassert builds.
    
    Anybody have an idea about how to silence it more effectively?
    There are going to be more people seeing this as gcc 15 propagates.
    
    			regards, tom lane
    
    
    
    
  2. Re: gcc 15 "array subscript 0" warning at level -O3

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-04-25T18:31:28Z

    I wrote:
    > Anybody have an idea about how to silence it more effectively?
    > There are going to be more people seeing this as gcc 15 propagates.
    
    Meh.  I tried this:
    
    diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
    index bb99781c56e..ea489db89c9 100644
    --- a/src/pl/plpgsql/src/pl_exec.c
    +++ b/src/pl/plpgsql/src/pl_exec.c
    @@ -8794,6 +8794,7 @@ assign_simple_var(PLpgSQL_execstate *estate, PLpgSQL_var *var,
             * not a problem since all array entries are always detoasted.)
             */
            if (!estate->atomic && !isnull && var->datatype->typlen == -1 &&
    +           DatumGetPointer(newvalue) != NULL &&
                VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
            {
                MemoryContext oldcxt;
    
    and was rewarded with *two* copies of the warning.  So that makes it
    smell more like a compiler bug than anything else.  (I now vaguely
    recall Andres opining that that's what it was on serinus, though
    I can't find any such email.)
    
    			regards, tom lane
    
    
    
    
  3. Re: gcc 15 "array subscript 0" warning at level -O3

    Andres Freund <andres@anarazel.de> — 2025-04-25T19:58:29Z

    Hi,
    
    On 2025-04-25 13:37:15 -0400, Tom Lane wrote:
    > Whilst poking at Erik Rijkers' nearby report, I found that
    > Fedora 42's gcc 15.0.1 will produce this complaint if you
    > select optimization level -O3:
    > 
    > In file included from ../../../../src/include/access/htup_details.h:22,
    >                  from pl_exec.c:21:
    > In function 'assign_simple_var',
    >     inlined from 'exec_set_found' at pl_exec.c:8609:2:
    > ../../../../src/include/varatt.h:230:36: warning: array subscript 0 is outside array bounds of 'char[0]' [-Warray-bounds=]
    >   230 |         (((varattrib_1b_e *) (PTR))->va_tag)
    >       |                                    ^
    > ../../../../src/include/varatt.h:94:12: note: in definition of macro 'VARTAG_IS_EXPANDED'
    >    94 |         (((tag) & ~1) == VARTAG_EXPANDED_RO)
    >       |            ^~~
    > ../../../../src/include/varatt.h:284:57: note: in expansion of macro 'VARTAG_1B_E'
    >   284 | #define VARTAG_EXTERNAL(PTR)                            VARTAG_1B_E(PTR)
    >       |                                                         ^~~~~~~~~~~
    > ../../../../src/include/varatt.h:301:57: note: in expansion of macro 'VARTAG_EXTERNAL'
    >   301 |         (VARATT_IS_EXTERNAL(PTR) && !VARTAG_IS_EXPANDED(VARTAG_EXTERNAL(PTR)))
    >       |                                                         ^~~~~~~~~~~~~~~
    > pl_exec.c:8797:17: note: in expansion of macro 'VARATT_IS_EXTERNAL_NON_EXPANDED'
    >  8797 |                 VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
    >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    > In function 'exec_set_found':
    > cc1: note: source object is likely at address zero
    
    FWIW, I've seen this even before GCC 15.
    
    
    > Buildfarm member serinus has been producing the identical warning for
    > some time.  I'd been ignoring that because it runs "experimental gcc",
    > but I guess the experiment has leaked out to production distros.
    > 
    > What seems to be happening here is that after inlining
    > assign_simple_var into exec_set_found, the compiler decides that
    > "newvalue" might be zero (since it's a BoolGetDatum result),
    > and then it warns -- in a rather strange way -- about the
    > potential null dereference.
    
    I don't think it actually is complaining about a null dereference - it thinks
    we're interpreting a boolean as a pointer (for which it obviously is not wide
    enough)
    
    
    > The dereference is not reachable
    > because of the preceding "var->datatype->typlen == -1" check,
    > but that's not stopping the optimizer from bitching.
    
    > I experimented with modifying exec_set_found thus:
    > 
    > 	var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
    > +	Assert(var->datatype->typlen == 1);
    > 	assign_simple_var(estate, var, BoolGetDatum(state), false, false);
    > 
    > which should be OK since we're expecting the "found" variable to
    > be boolean.  That does silence the warning, but of course only
    > in --enable-cassert builds.
    
    One way to address this is outlined here:
    
    https://postgr.es/m/20230316172818.x6375uvheom3ibt2%40awork3.anarazel.de
    https://postgr.es/m/20240207203138.sknifhlppdtgtxnk%40awork3.anarazel.de
    
    I've been wondering about adding wrapping something like that in a
    pg_assume(expr) or such.
    
    Greetings,
    
    Andres Freund
    
    
    
    
  4. Re: gcc 15 "array subscript 0" warning at level -O3

    Andres Freund <andres@anarazel.de> — 2025-06-04T19:00:03Z

    Hi,
    
    On 2025-04-25 15:58:29 -0400, Andres Freund wrote:
    > On 2025-04-25 13:37:15 -0400, Tom Lane wrote:
    > > Buildfarm member serinus has been producing the identical warning for
    > > some time.  I'd been ignoring that because it runs "experimental gcc",
    > > but I guess the experiment has leaked out to production distros.
    > > 
    > > What seems to be happening here is that after inlining
    > > assign_simple_var into exec_set_found, the compiler decides that
    > > "newvalue" might be zero (since it's a BoolGetDatum result),
    > > and then it warns -- in a rather strange way -- about the
    > > potential null dereference.
    > 
    > I don't think it actually is complaining about a null dereference - it thinks
    > we're interpreting a boolean as a pointer (for which it obviously is not wide
    > enough)
    > 
    > 
    > > The dereference is not reachable
    > > because of the preceding "var->datatype->typlen == -1" check,
    > > but that's not stopping the optimizer from bitching.
    > 
    > > I experimented with modifying exec_set_found thus:
    > > 
    > > 	var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
    > > +	Assert(var->datatype->typlen == 1);
    > > 	assign_simple_var(estate, var, BoolGetDatum(state), false, false);
    > > 
    > > which should be OK since we're expecting the "found" variable to
    > > be boolean.  That does silence the warning, but of course only
    > > in --enable-cassert builds.
    > 
    > One way to address this is outlined here:
    > 
    > https://postgr.es/m/20230316172818.x6375uvheom3ibt2%40awork3.anarazel.de
    > https://postgr.es/m/20240207203138.sknifhlppdtgtxnk%40awork3.anarazel.de
    > 
    > I've been wondering about adding wrapping something like that in a
    > pg_assume(expr) or such.
    
    I've been once more annoyed by this warning. Here's a prototype for the
    approach outlined above.
    
    Greetings,
    
    Andres Freund
    
  5. Re: gcc 15 "array subscript 0" warning at level -O3

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-06-05T19:50:48Z

    Andres Freund <andres@anarazel.de> writes:
    >> I've been wondering about adding wrapping something like that in a
    >> pg_assume(expr) or such.
    
    > I've been once more annoyed by this warning. Here's a prototype for the
    > approach outlined above.
    
    Looks plausible by eyeball.  I did notice a typo in the comment:
    
    + * pg_assume(expr) stats that we assume `expr` to evaluate to true. In assert
    
    s/stats/states/, I think you meant.
    
    			regards, tom lane
    
    
    
    
  6. Re: gcc 15 "array subscript 0" warning at level -O3

    jian he <jian.universality@gmail.com> — 2025-07-02T14:13:17Z

    On Thu, Jun 5, 2025 at 3:00 AM Andres Freund <andres@anarazel.de> wrote:
    >
    > > > The dereference is not reachable
    > > > because of the preceding "var->datatype->typlen == -1" check,
    > > > but that's not stopping the optimizer from bitching.
    > >
    > > > I experimented with modifying exec_set_found thus:
    > > >
    > > >     var = (PLpgSQL_var *) (estate->datums[estate->found_varno]);
    > > > +   Assert(var->datatype->typlen == 1);
    > > >     assign_simple_var(estate, var, BoolGetDatum(state), false, false);
    > > >
    > > > which should be OK since we're expecting the "found" variable to
    > > > be boolean.  That does silence the warning, but of course only
    > > > in --enable-cassert builds.
    > >
    > > One way to address this is outlined here:
    > >
    > > https://postgr.es/m/20230316172818.x6375uvheom3ibt2%40awork3.anarazel.de
    > > https://postgr.es/m/20240207203138.sknifhlppdtgtxnk%40awork3.anarazel.de
    > >
    > > I've been wondering about adding wrapping something like that in a
    > > pg_assume(expr) or such.
    >
    > I've been once more annoyed by this warning. Here's a prototype for the
    > approach outlined above.
    >
    
    I can confirm the warning disappears when using gcc-14.0 compile
    source code with the attached patch.
    I didn't review it though.
    I didn’t find this in the CommitFest, so I added an entry [1] to make sure it
    doesn’t get forgotten...
    
    [1]: https://commitfest.postgresql.org/patch/5888/
    
    
    
    
  7. Re: gcc 15 "array subscript 0" warning at level -O3

    Andres Freund <andres@anarazel.de> — 2025-07-09T23:20:20Z

    Hi,
    
    On 2025-06-05 15:50:48 -0400, Tom Lane wrote:
    > Andres Freund <andres@anarazel.de> writes:
    > >> I've been wondering about adding wrapping something like that in a
    > >> pg_assume(expr) or such.
    > 
    > > I've been once more annoyed by this warning. Here's a prototype for the
    > > approach outlined above.
    > 
    > Looks plausible by eyeball.  I did notice a typo in the comment:
    > 
    > + * pg_assume(expr) stats that we assume `expr` to evaluate to true. In assert
    > 
    > s/stats/states/, I think you meant.
    
    Thanks.  I pushed it with that tweak and a bit of minor comment burnishing.
    Glad to see the last of that warning.
    
    Greetings,
    
    Andres Freund
    
    
    
    
  8. Re: gcc 15 "array subscript 0" warning at level -O3

    Andres Freund <andres@anarazel.de> — 2025-07-09T23:21:29Z

    Hi,
    
    On 2025-07-02 22:13:17 +0800, jian he wrote:
    > On Thu, Jun 5, 2025 at 3:00 AM Andres Freund <andres@anarazel.de> wrote:
    > > I've been once more annoyed by this warning. Here's a prototype for the
    > > approach outlined above.
    > >
    > 
    > I can confirm the warning disappears when using gcc-14.0 compile
    > source code with the attached patch.
    > I didn't review it though.
    > I didn’t find this in the CommitFest, so I added an entry [1] to make sure it
    > doesn’t get forgotten...
    
    Thanks. Pushed it now and closed the CF entry.
    
    Greetings,
    
    Andres Freund