public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v9 19/21] Almost cosmetic fixes
9+ messages / 4 participants
[nested] [flat]
* [PATCH v9 19/21] Almost cosmetic fixes
@ 2024-03-27 21:44 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 9+ messages in thread
From: Heikki Linnakangas @ 2024-03-27 21:44 UTC (permalink / raw)
---
src/backend/access/heap/pruneheap.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 337331901ab..2bd2e858bcd 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -38,7 +38,10 @@ typedef struct
TransactionId visibility_cutoff_xid;
bool all_visible_except_removable;
- TransactionId new_prune_xid; /* new prune hint value for page */
+ /*
+ * Fields describing what to do to the page
+ */
+ TransactionId new_prune_xid; /* new prune hint value */
TransactionId latest_xid_removed;
int nredirected; /* numbers of entries in arrays below */
int ndead;
@@ -61,7 +64,7 @@ typedef struct
/*
* Tuple visibility is only computed once for each tuple, for correctness
* and efficiency reasons; see comment in heap_page_prune_and_freeze() for
- * details. This is of type int8[], instead of HTSV_Result[], so we can
+ * details. This is of type int8[], instead of HTSV_Result[], so we can
* use -1 to indicate no visibility has been computed, e.g. for LP_DEAD
* items.
*
--
2.40.1
--caj67xgx3lukmr5f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0020-Move-frz_conflict_horizon-to-tighter-scope.patch"
^ permalink raw reply [nested|flat] 9+ messages in thread
* gcc 15 "array subscript 0" warning at level -O3
@ 2025-04-25 17:37 Tom Lane <[email protected]>
2025-04-25 18:31 ` Re: gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
2025-04-25 19:58 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
0 siblings, 2 replies; 9+ messages in thread
From: Tom Lane @ 2025-04-25 17:37 UTC (permalink / raw)
To: [email protected]
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: gcc 15 "array subscript 0" warning at level -O3
2025-04-25 17:37 gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
@ 2025-04-25 18:31 ` Tom Lane <[email protected]>
1 sibling, 0 replies; 9+ messages in thread
From: Tom Lane @ 2025-04-25 18:31 UTC (permalink / raw)
To: [email protected]
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: gcc 15 "array subscript 0" warning at level -O3
2025-04-25 17:37 gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
@ 2025-04-25 19:58 ` Andres Freund <[email protected]>
2025-06-04 19:00 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
1 sibling, 1 reply; 9+ messages in thread
From: Andres Freund @ 2025-04-25 19:58 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: gcc 15 "array subscript 0" warning at level -O3
2025-04-25 17:37 gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
2025-04-25 19:58 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
@ 2025-06-04 19:00 ` Andres Freund <[email protected]>
2025-06-05 19:50 ` Re: gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
2025-07-02 14:13 ` Re: gcc 15 "array subscript 0" warning at level -O3 jian he <[email protected]>
0 siblings, 2 replies; 9+ messages in thread
From: Andres Freund @ 2025-06-04 19:00 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]
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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: gcc 15 "array subscript 0" warning at level -O3
2025-04-25 17:37 gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
2025-04-25 19:58 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
2025-06-04 19:00 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
@ 2025-06-05 19:50 ` Tom Lane <[email protected]>
2025-07-09 23:20 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
1 sibling, 1 reply; 9+ messages in thread
From: Tom Lane @ 2025-06-05 19:50 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: [email protected]
Andres Freund <[email protected]> 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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: gcc 15 "array subscript 0" warning at level -O3
2025-04-25 17:37 gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
2025-04-25 19:58 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
2025-06-04 19:00 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
2025-06-05 19:50 ` Re: gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
@ 2025-07-09 23:20 ` Andres Freund <[email protected]>
0 siblings, 0 replies; 9+ messages in thread
From: Andres Freund @ 2025-07-09 23:20 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]
Hi,
On 2025-06-05 15:50:48 -0400, Tom Lane wrote:
> Andres Freund <[email protected]> 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
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: gcc 15 "array subscript 0" warning at level -O3
2025-04-25 17:37 gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
2025-04-25 19:58 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
2025-06-04 19:00 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
@ 2025-07-02 14:13 ` jian he <[email protected]>
2025-07-09 23:21 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
1 sibling, 1 reply; 9+ messages in thread
From: jian he @ 2025-07-02 14:13 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Tom Lane <[email protected]>; [email protected]
On Thu, Jun 5, 2025 at 3:00 AM Andres Freund <[email protected]> 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/
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: gcc 15 "array subscript 0" warning at level -O3
2025-04-25 17:37 gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
2025-04-25 19:58 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
2025-06-04 19:00 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
2025-07-02 14:13 ` Re: gcc 15 "array subscript 0" warning at level -O3 jian he <[email protected]>
@ 2025-07-09 23:21 ` Andres Freund <[email protected]>
0 siblings, 0 replies; 9+ messages in thread
From: Andres Freund @ 2025-07-09 23:21 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: Tom Lane <[email protected]>; [email protected]
Hi,
On 2025-07-02 22:13:17 +0800, jian he wrote:
> On Thu, Jun 5, 2025 at 3:00 AM Andres Freund <[email protected]> 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
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2025-07-09 23:21 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-03-27 21:44 [PATCH v9 19/21] Almost cosmetic fixes Heikki Linnakangas <[email protected]>
2025-04-25 17:37 gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
2025-04-25 18:31 ` Re: gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
2025-04-25 19:58 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
2025-06-04 19:00 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
2025-06-05 19:50 ` Re: gcc 15 "array subscript 0" warning at level -O3 Tom Lane <[email protected]>
2025-07-09 23:20 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[email protected]>
2025-07-02 14:13 ` Re: gcc 15 "array subscript 0" warning at level -O3 jian he <[email protected]>
2025-07-09 23:21 ` Re: gcc 15 "array subscript 0" warning at level -O3 Andres Freund <[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