pgsql: Add support event triggers on authenticated login

On Mon, Oct 16, 2023 at 5:49 AM Alexander Korotkov
<akorotkov@postgresql.org> wrote:

Add support event triggers on authenticated login

Hi, I'm seeing a compiler warning with CFLAGS -O3 but not with -O2.

In file included from dbcommands.c:20:
dbcommands.c: In function ‘createdb’:
../../../src/include/postgres.h:104:16: warning: ‘src_hasloginevt’ may
be used uninitialized in this function [-Wmaybe-uninitialized]
104 | return (Datum) (X ? 1 : 0);
| ^~~~~~~~~~~~~~~~~~~
dbcommands.c:683:25: note: ‘src_hasloginevt’ was declared here
683 | bool src_hasloginevt;
| ^~~~~~~~~~~~~~~

The configure command I used is ./configure --prefix=$PWD/inst/
CFLAGS="-ggdb3 -O3" > install.log && make -j 8 install > install.log
2>&1 &:

CONFIGURE = '--prefix=/home/ubuntu/postgres/inst/' 'CFLAGS=-ggdb3 -O3'
CC = gcc
CPPFLAGS = -D_GNU_SOURCE
CFLAGS = -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Werror=vla -Wendif-labels
-Wmissing-format-attribute -Wimplicit-fallthrough=3
-Wcast-function-type -Wshadow=compatible-local -Wformat-security
-fno-strict-aliasing -fwrapv -fexcess-precision=standard
-Wno-format-truncation -Wno-stringop-truncation -ggdb3 -O3
CFLAGS_SL = -fPIC

The compiler version is:
gcc --version
gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:

Hi, I'm seeing a compiler warning with CFLAGS -O3 but not with -O2.

In file included from dbcommands.c:20:
dbcommands.c: In function ‘createdb’:
../../../src/include/postgres.h:104:16: warning: ‘src_hasloginevt’ may
be used uninitialized in this function [-Wmaybe-uninitialized]

Hmm, I also see that at -O3 (not at -O2) when using Fedora 39's
gcc 13.2.1, but *not* when using RHEL8's gcc 8.5.0.

I'm not sure how excited I am about curing that, though, because gcc
13.2.1 spews several other totally baseless warnings (see attached).
Some of them match up with warnings we're seeing on buildfarm member
serinus, which I seem to recall that Andres had tracked to a known gcc
bug.

regards, tom lane

Hi,

On 2024-01-11 21:55:19 -0500, Tom Lane wrote:

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:

Hi, I'm seeing a compiler warning with CFLAGS -O3 but not with -O2.

In file included from dbcommands.c:20:
dbcommands.c: In function ‘createdb’:
../../../src/include/postgres.h:104:16: warning: ‘src_hasloginevt’ may
be used uninitialized in this function [-Wmaybe-uninitialized]

Hmm, I also see that at -O3 (not at -O2) when using Fedora 39's
gcc 13.2.1, but *not* when using RHEL8's gcc 8.5.0.

It's visible here with gcc >= 10. That's enough versions that I think we
should care. Interestingly enough, it seems to have recently have gotten
fixed in gcc master (14 to be).

Some of them match up with warnings we're seeing on buildfarm member
serinus, which I seem to recall that Andres had tracked to a known gcc bug.

Some, but I don't think all.

In file included from ../../../../src/include/executor/instrument.h:16,
from pgstat_io.c:19:
pgstat_io.c: In function 'pgstat_count_io_op_time':
pgstat_io.c:149:60: warning: array subscript 2 is above array bounds of 'instr_time[2][4][8]' [-Warray-bounds=]
149 | INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op],
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~

Huh, I don't see that with any version of gcc I tried.

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:8349: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:8537:17: note: in expansion of macro 'VARATT_IS_EXTERNAL_NON_EXPANDED'
8537 | VARATT_IS_EXTERNAL_NON_EXPANDED(DatumGetPointer(newvalue)))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function 'exec_set_found':
cc1: note: source object is likely at address zero

This I see. If I hint to the compiler that var->datatype->typlen != 1 when
called from exec_set_found(), the warning vanishes. E.g. with

if (var->datatype->typlen == -1)
__builtin_unreachable();

I see one more warning:

[1390/2375 42 58%] Compiling C object src/backend/postgres_lib.a.p/utils_adt_jsonb_util.c.o
../../../../../home/andres/src/postgresql/src/backend/utils/adt/jsonb_util.c: In function 'compareJsonbContainers':
../../../../../home/andres/src/postgresql/src/backend/utils/adt/jsonb_util.c:296:34: warning: 'va.type' may be used uninitialized [-Wmaybe-uninitialized]
296 | res = (va.type > vb.type) ? 1 : -1;
| ~~^~~~~
../../../../../home/andres/src/postgresql/src/backend/utils/adt/jsonb_util.c:204:33: note: 'va' declared here
204 | JsonbValue va,
| ^~

I can't really blame the compiler here. There's a fairly lengthy comment
explaining that va.type/vb.type are set, and it took me a while to understand:

/*
* It's safe to assume that the types differed, and that the va
* and vb values passed were set.
*
* If the two values were of the same container type, then there'd
* have been a chance to observe the variation in the number of
* elements/pairs (when processing WJB_BEGIN_OBJECT, say). They're
* either two heterogeneously-typed containers, or a container and
* some scalar type.
*
* We don't have to consider the WJB_END_ARRAY and WJB_END_OBJECT
* cases here, because we would have seen the corresponding
* WJB_BEGIN_ARRAY and WJB_BEGIN_OBJECT tokens first, and
* concluded that they don't match.
*/

It's not surprising that the compiler can't understand that you can't get
ra = WJB_DONE, rb = WJB_END_ARRAY or such.

Greetings,

Andres Freund

On Wed, Feb 7, 2024 at 10:31 PM Andres Freund <andres@anarazel.de> wrote:

On 2024-01-11 21:55:19 -0500, Tom Lane wrote:

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:

Hi, I'm seeing a compiler warning with CFLAGS -O3 but not with -O2.

In file included from dbcommands.c:20:
dbcommands.c: In function ‘createdb’:
../../../src/include/postgres.h:104:16: warning: ‘src_hasloginevt’ may
be used uninitialized in this function [-Wmaybe-uninitialized]

Hmm, I also see that at -O3 (not at -O2) when using Fedora 39's
gcc 13.2.1, but *not* when using RHEL8's gcc 8.5.0.

It's visible here with gcc >= 10. That's enough versions that I think we
should care. Interestingly enough, it seems to have recently have gotten
fixed in gcc master (14 to be).

I managed to reproduce this warning locally. Fixed.

------
Regards,
Alexander Korotkov