Re: Warning in the RecordTransactionAbort routine during compilationwith 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