Re: Warning in the RecordTransactionAbort routine during compilation with O3 flag

09.12.2019 13:03, Michael Paquier пишет:
> On Mon, Dec 09, 2019 at 08:49:26AM +0500, Andrey Lepikhov wrote:
>> xact.c: In function ‘RecordTransactionAbort’:
>> xact.c:5709:55: warning: argument 1 null where non-null expected [-Wnonnull]
>> XLogRegisterData(unconstify(char *, twophase_gid), strlen(twophase_gid)
>> + 1);
>
> Assert(twophase_gid != NULL);
> -
> - if (XLogLogicalInfoActive())
> - xl_xinfo.xinfo |= XACT_XINFO_HAS_GID;
>
> xlinfo is set in the first part logging the transaction commit and the
> record data is registered in the second, so I think that the original
> coding makes more sense than what you are suggesting. Perhaps it
> would help to just add an assertion on twophase_gid to make sure that
> it is not NULL in the part registering the data? After that we really
> have no bugs here, so it does not really help much..

We already have assertion on the twophase_gid variable. But compiler is
not so smart and can't find a link between the XACT_XINFO_HAS_GID flag
and state of twophase_gid pointer.

>
>> formatting.c: In function ‘parse_datetime’:
>> formatting.c:4229:13: warning: ‘flags’ may be used uninitialized in this
>> function [-Wmaybe-uninitialized]
>> if (flags & DCH_ZONED)
>
> - uint32 flags;
> + uint32 flags = 0;
>
> do_to_timestamp(date_txt, fmt, strict, &tm, &fsec, &fprec, &flags, have_error);
>
> For this one, OK. Wouldn't it be better to initialize flags, fprec
> and have_error directly in do_to_timestamp if they are not NULL? This
> way future callers of the routine, if any, won't miss the
> initialization.

Ok. In accordance with your review, I have prepared a new version of the
patch.

>
> By the way, are you using more specific CFLAGS to see that? With -O3
> and -Wnonnull I cannot spot both issues with GCC 9.2.1.

My compilation procedure:

export CFLAGS="-O3"
./configure --prefix=`pwd`/tmp_install --enable-tap-tests
--enable-depend && make clean
make > /dev/null
make install > /dev/null

My compiler:

gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/7/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu
7.4.0-1ubuntu1~18.04.1'
--with-bugurl=file:///usr/share/doc/gcc-7/README.Bugs
--enable-languages=c,ada,c++,go,brig,d,fortran,objc,obj-c++
--prefix=/usr --with-gcc-major-version-only --program-suffix=-7
--program-prefix=x86_64-linux-gnu- --enable-shared
--enable-linker-build-id --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix --libdir=/usr/lib
--enable-nls --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-vtable-verify --enable-libmpx --enable-plugin
--enable-default-pie --with-system-zlib --with-target-system-zlib
--enable-objc-gc=auto --enable-multiarch --disable-werror
--with-arch-32=i686 --with-abi=m64 --with-multilib-list=m32,m64,mx32
--enable-multilib --with-tune=generic
--enable-offload-targets=nvptx-none --without-cuda-driver
--enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 7.4.0 (Ubuntu 7.4.0-1ubuntu1~18.04.1)

--
Andrey Lepikhov
Postgres Professional
https://postgrespro.com
The Russian Postgres Company