Обсуждение: Compile warnings in dbcommands.c building with meson

When building current head on debian bullseye I get this compile warning:

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


I only get this when building with meson, not when building with
autotools. AFAICT, I have the same config:

./configure --enable-debug --enable-depend --with-python
--enable-cassert --with-openssl --enable-tap-tests --with-icu

vs

meson setup build -Ddebug=true -Dpython=true -Dcassert=true
-Dssl=openssl -Dtap-test=true -Dicu=enabled -Dnls=disabled


in both cases the compiler is:
gcc (Debian 10.2.1-6) 10.2.1 20210110

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

Hi,

> When building current head on debian bullseye I get this compile warning:
>
> In file included from ../src/backend/commands/dbcommands.c:20:
> ../src/backend/commands/dbcommands.c: In function ‘createdb’:
> ../src/include/postgres.h:104:9: warning: ‘src_hasloginevt’ may be
> used uninitialized in this function [-Wmaybe-uninitialized]
>   104 |  return (Datum) (X ? 1 : 0);
>       |         ^~~~~~~~~~~~~~~~~~~
> ../src/backend/commands/dbcommands.c:683:8: note: ‘src_hasloginevt’
> was declared here
>   683 |  bool  src_hasloginevt;
>       |        ^~~~~~~~~~~~~~~
>
>
> I only get this when building with meson, not when building with
> autotools. AFAICT, I have the same config:
>
> ./configure --enable-debug --enable-depend --with-python
> --enable-cassert --with-openssl --enable-tap-tests --with-icu
>
> vs
>
> meson setup build -Ddebug=true -Dpython=true -Dcassert=true
> -Dssl=openssl -Dtap-test=true -Dicu=enabled -Dnls=disabled
>
>
> in both cases the compiler is:
> gcc (Debian 10.2.1-6) 10.2.1 20210110

Seems to me that the compiler is not smart enough to process:

```
    if (!get_db_info(dbtemplate, ShareLock,
                     &src_dboid, &src_owner, &src_encoding,
                     &src_istemplate, &src_allowconn, &src_hasloginevt,
                     &src_frozenxid, &src_minmxid, &src_deftablespace,
                     &src_collate, &src_ctype, &src_iculocale,
&src_icurules, &src_locprovider,
                     &src_collversion))
        ereport(ERROR, ...
```

Should we just silence the warning like this - see attachment? I don't
think createdb() is called that often to worry about slight
performance change, if there is any.

--
Best regards,
Aleksander Alekseev

On Wed, Jan 10, 2024 at 1:16 PM Aleksander Alekseev
<aleksander@timescale.com> wrote:
>
> Hi,
>
> > When building current head on debian bullseye I get this compile warning:
> >
> > In file included from ../src/backend/commands/dbcommands.c:20:
> > ../src/backend/commands/dbcommands.c: In function ‘createdb’:
> > ../src/include/postgres.h:104:9: warning: ‘src_hasloginevt’ may be
> > used uninitialized in this function [-Wmaybe-uninitialized]
> >   104 |  return (Datum) (X ? 1 : 0);
> >       |         ^~~~~~~~~~~~~~~~~~~
> > ../src/backend/commands/dbcommands.c:683:8: note: ‘src_hasloginevt’
> > was declared here
> >   683 |  bool  src_hasloginevt;
> >       |        ^~~~~~~~~~~~~~~
> >
> >
> > I only get this when building with meson, not when building with
> > autotools. AFAICT, I have the same config:
> >
> > ./configure --enable-debug --enable-depend --with-python
> > --enable-cassert --with-openssl --enable-tap-tests --with-icu
> >
> > vs
> >
> > meson setup build -Ddebug=true -Dpython=true -Dcassert=true
> > -Dssl=openssl -Dtap-test=true -Dicu=enabled -Dnls=disabled
> >
> >
> > in both cases the compiler is:
> > gcc (Debian 10.2.1-6) 10.2.1 20210110
>
> Seems to me that the compiler is not smart enough to process:
>
> ```
>     if (!get_db_info(dbtemplate, ShareLock,
>                      &src_dboid, &src_owner, &src_encoding,
>                      &src_istemplate, &src_allowconn, &src_hasloginevt,
>                      &src_frozenxid, &src_minmxid, &src_deftablespace,
>                      &src_collate, &src_ctype, &src_iculocale,
> &src_icurules, &src_locprovider,
>                      &src_collversion))
>         ereport(ERROR, ...
> ```
>
> Should we just silence the warning like this - see attachment? I don't
> think createdb() is called that often to worry about slight
> performance change, if there is any.

Certainly looks that way, but I'm curious as to why nobody else has seen this..

That said, it appears to be gone in current master. Even though
nothing changed in that file.  Must've been some transient effect,
that somehow didn't get blown away by doing a clean....

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

On Fri, Jan 12, 2024 at 1:05 AM Magnus Hagander <magnus@hagander.net> wrote:
>
> On Wed, Jan 10, 2024 at 1:16 PM Aleksander Alekseev
> <aleksander@timescale.com> wrote:
> >
> > Hi,
> >
> > > When building current head on debian bullseye I get this compile warning:
> > >
> > > In file included from ../src/backend/commands/dbcommands.c:20:
> > > ../src/backend/commands/dbcommands.c: In function ‘createdb’:
> > > ../src/include/postgres.h:104:9: warning: ‘src_hasloginevt’ may be
> > > used uninitialized in this function [-Wmaybe-uninitialized]
> > >   104 |  return (Datum) (X ? 1 : 0);
> > >       |         ^~~~~~~~~~~~~~~~~~~
> > > ../src/backend/commands/dbcommands.c:683:8: note: ‘src_hasloginevt’
> > > was declared here
> > >   683 |  bool  src_hasloginevt;
> > >       |        ^~~~~~~~~~~~~~~
> > >
> > >
> > > I only get this when building with meson, not when building with
> > > autotools. AFAICT, I have the same config:
> > >
> > > ./configure --enable-debug --enable-depend --with-python
> > > --enable-cassert --with-openssl --enable-tap-tests --with-icu
> > >
> > > vs
> > >
> > > meson setup build -Ddebug=true -Dpython=true -Dcassert=true
> > > -Dssl=openssl -Dtap-test=true -Dicu=enabled -Dnls=disabled
> > >
> > >
> > > in both cases the compiler is:
> > > gcc (Debian 10.2.1-6) 10.2.1 20210110
> >
> > Seems to me that the compiler is not smart enough to process:
> >
> > ```
> >     if (!get_db_info(dbtemplate, ShareLock,
> >                      &src_dboid, &src_owner, &src_encoding,
> >                      &src_istemplate, &src_allowconn, &src_hasloginevt,
> >                      &src_frozenxid, &src_minmxid, &src_deftablespace,
> >                      &src_collate, &src_ctype, &src_iculocale,
> > &src_icurules, &src_locprovider,
> >                      &src_collversion))
> >         ereport(ERROR, ...
> > ```
> >
> > Should we just silence the warning like this - see attachment? I don't
> > think createdb() is called that often to worry about slight
> > performance change, if there is any.
>
> Certainly looks that way, but I'm curious as to why nobody else has seen this..
>

I saw it sometimes, sometimes not.
Now I think the reason is:
it will appear when you do `-Dbuildtype=release`.

but it will not occur when I do:
`-Dbuildtype=debug`

my current meson version is 1.3.1, my ninja version is 1.10.1.

On 2024-Jan-12, jian he wrote:

> I saw it sometimes, sometimes not.
> Now I think the reason is:
> it will appear when you do `-Dbuildtype=release`.
> 
> but it will not occur when I do:
> `-Dbuildtype=debug`
> 
> my current meson version is 1.3.1, my ninja version is 1.10.1.

Hmm, but why doesn't it happen for other arguments of get_db_info that
have pretty much identical code, say src_istemplate?

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"But static content is just dynamic content that isn't moving!"
                http://smylers.hates-software.com/2007/08/15/fe244d0c.html

On Fri, Jan 12, 2024 at 8:03 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2024-Jan-12, jian he wrote:
>
> > I saw it sometimes, sometimes not.
> > Now I think the reason is:
> > it will appear when you do `-Dbuildtype=release`.
> >
> > but it will not occur when I do:
> > `-Dbuildtype=debug`
> >
> > my current meson version is 1.3.1, my ninja version is 1.10.1.
>
> Hmm, but why doesn't it happen for other arguments of get_db_info that
> have pretty much identical code, say src_istemplate?
>

git at commit 6780b79d5c580586ae6feb37b9c8b8bf33367886 (HEAD ->
master, origin/master, origin/HEAD)
the minimum setup that will generate the warning:

meson setup  --reconfigure ${BUILD} \
-Dprefix=${PG_PREFIX} \
-Dpgport=5462 \
-Dbuildtype=release

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.

Hi. one more feedback.

I tested the original repo setup, but it does not generate a warning
on my local setup.
meson setup  --reconfigure ${BUILD} \
-Dprefix=${PG_PREFIX} \
-Dpgport=5463 \
-Dplpython=enabled \
-Dcassert=true \
-Dtap_tests=enabled \
-Dicu=enabled  \
-Ddebug=true  \
-Dnls=disabled

 it generate warning, when I add -Dbuildtype=release :

meson setup  --reconfigure ${BUILD} \
-Dprefix=${PG_PREFIX} \
-Dpgport=5463 \
-Dplpython=enabled \
-Dcassert=true \
-Dtap_tests=enabled \
-Dicu=enabled  \
-Dbuildtype=release \
-Ddebug=true  \
-Dnls=disabled

After applying the patch, the warning disappeared.
so it fixed the problem.