Re: [lmi] Unexpected gcc diagnostic

[Greg Chicares]

Thanks for your help. TL;DR: it's presumably a defect in the
20200525 gcc version that has been fixed in the 20210110 version,
so we should upgrade gcc on the server; and the assertion doesn't
actually afford the hoped-for protection, but I'll fix that.

On 4/6/21 4:32 PM, Vadim Zeitlin wrote:
> On Tue, 6 Apr 2021 15:41:21 +0000 Greg Chicares <gchicares@sbcglobal.net> 
> wrote:
> 
> GC> Vadim--What do you make of this? Building for i686-w64-mingw32
> GC> with `i686-w64-mingw32-gcc -dumpfullversion` = "10-win32"
> 
>  Could you please try running "i686-win32-mingw32-gcc -v" too? It shows
> more information than "-dumpfullversion", e.g. here it gives:
> 
>       % i686-w64-mingw32-g++ -v |& tail -n1
>       gcc version 10-win32 20210110 (GCC)

My own PC reports the same as yours:

$i686-w64-mingw32-gcc -v
[...snippage: same as below, but I also have
   --disable-sjlj-exceptions --with-dwarf2
at the end, which the server lacks...]
gcc version 10-win32 20210110 (GCC)

but the corporate server is older:

$i686-w64-mingw32-gcc -v
Using built-in specs.
COLLECT_GCC=i686-w64-mingw32-gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/i686-w64-mingw32/10-win32/lto-wrapper
Target: i686-w64-mingw32
Configured with: ../../src/configure --build=x86_64-linux-gnu --prefix=/usr 
--includedir='/usr/include' --mandir='/usr/share/man' 
--infodir='/usr/share/info' --sysconfdir=/etc --localstatedir=/var 
--disable-option-checking --disable-silent-rules 
--libdir='/usr/lib/x86_64-linux-gnu' --libexecdir='/usr/lib/x86_64-linux-gnu' 
--disable-maintainer-mode --disable-dependency-tracking --prefix=/usr 
--enable-shared --enable-static --disable-multilib --with-system-zlib 
--libexecdir=/usr/lib --without-included-gettext --libdir=/usr/lib 
--enable-libstdcxx-time=yes --with-tune=generic 
--with-headers=/usr/i686-w64-mingw32/include 
--enable-version-specific-runtime-libs --enable-fully-dynamic-string 
--enable-libgomp --enable-languages=c,c++,fortran,objc,obj-c++,ada --enable-lto 
--enable-threads=win32 --program-suffix=-win32 
--program-prefix=i686-w64-mingw32- --target=i686-w64-mingw32 
--with-as=/usr/bin/i686-w64-mingw32-as --with-ld=/usr/bin/i686-w64-mingw32-ld 
--enable-libatomic --enable-libstdcxx-filesystem-ts=yes 
--enable-dependency-tracking
Thread model: win32
Supported LTO compression algorithms: zlib
gcc version 10-win32 20200525 (GCC)

> If the server has the same version, could you please also show the full
> compiler command line it uses? Thanks in advance!

Though the versions differ, I'll paste the full command line just
because I have it handy right now, but it's just the standard lmi
command for production ("-O2", and nothing exotic like libstdc++
debug build):

i686-w64-mingw32-g++ -MMD -MP -MT ihs_irc7702a.o -MF ihs_irc7702a.d  -c -I 
/opt/lmi/src/lmi -I /opt/lmi/src/lmi/tools/pete-2.1.1 -isystem 
/opt/lmi/local/gcc_i686-w64-mingw32/lib/wx/include/i686-w64-mingw32-msw-unicode-3.1
 -isystem /opt/lmi/local/include/wx-3.1 -isystem /opt/lmi/third_party/include 
-isystem /opt/lmi/local/include -isystem /opt/lmi/local/include/libxml2 
-DLMI_WX_NEW_USE_SO  -DXMLWRAPP_USE_DLL -DXSLTWRAPP_USE_DLL -DSTRICT    
-D_FILE_OFFSET_BITS=64 -DWXUSINGDLL -D__WXMSW__ -D_FILE_OFFSET_BITS=64 
-DBOOST_NO_AUTO_PTR -DBOOST_NO_STD_ALLOCATOR -DBOOST_STRICT_CONFIG 
-DBOOST_STATIC_ASSERT_HPP   -fno-ms-extensions -frounding-math -std=c++20 
-pedantic-errors -Werror -Wall -Walloc-zero -Walloca -Wcast-align 
-Wcast-function-type -Wconversion -Wdangling-else -Wdeprecated-declarations 
-Wdisabled-optimization -Wdouble-promotion -Wduplicated-branches 
-Wduplicated-cond -Wextra -Wformat-nonliteral -Wformat-security 
-Wformat-signedness -Wformat-y2k -Wimport -Winvalid-pch -Wlogical-op 
-Wmissing-include-dirs -Wmultichar -Wnull-dereference -Wpacked -Wpointer-arith 
-Wredundant-decls -Wrestrict -Wshadow -Wsign-compare -Wstack-protector 
-Wswitch-enum -Wtrampolines -Wundef -Wunreachable-code -Wunused-macros 
-Wvector-operation-performance -Wno-parentheses  -Wc++11-compat -Wc++14-compat 
-Wc++1z-compat -Wcatch-value=3 -Wconditionally-supported -Wctor-dtor-privacy 
-Wdelete-non-virtual-dtor -Wdeprecated -Wextra-semi -Wnoexcept -Wnoexcept-type 
-Wnon-template-friend -Wnon-virtual-dtor -Wold-style-cast -Woverloaded-virtual 
-Wplacement-new=2 -Wpmf-conversions -Wregister -Wreorder -Wstrict-null-sentinel 
-Wsuggest-override -Wsynth -Wuseless-cast -Wzero-as-null-pointer-constant  
-Wcast-qual    -D'BOOST_STATIC_ASSERT(A)=static_assert((A))'   -O2 
-fno-omit-frame-pointer -ggdb    /opt/lmi/src/lmi/ihs_irc7702a.cpp 
-oihs_irc7702a.o

> GC>     LMI_ASSERT(Bfts.begin() <= last_bft_in_test_period);
> GC>     LowestBft = *std::min_element(Bfts.begin(), last_bft_in_test_period);

[...snip my rigorous proof of something that isn't quite what we want...]

>  All this is correct and holds even if Bfts vector is empty. However
> dereferencing the iterator returned by std::min_element() wouldn't be in
> this case, as it would return non-dereferencable Bfts.end() iterator. I'm
> not at all sure if it's worth checking for this, but I wanted to at least
> mention this for completeness.

Thanks: "<=" above isn't good enough; "<" would truly guard
the "*std::min_element" expression.

> GC> But that's dynamic analysis; is the compiler is performing
> GC> static analysis here?
> 
>  No, I don't believe this at all, it just seems to be awfully confused
> here. The only surprising thing is that the corporate server somehow uses a
> version with this bug in it, while we don't see it and I'm almost certain
> that I must have tried building lmi with all the versions of MinGW-w64 from
> Sid, as I update my chroot regularly and always rebuild lmi if the compiler
> version changes, but I had never seen this before neither. But perhaps it
> happens in optimized build only, while I usually build in debug.

BTW, does the github CI similarly do continual gcc updates?

> GC> I imagine std::vector::const_iterator is of unsigned type,
> 
>  In most implementations it's a pointer.

Good point. I did enough asm work, back in the day, that I
still think of pointers as nothing more than hex numbers
that are naturally unsigned. But ptrdiff_t is signed, so
unsignedness cannot be the explanation of any legitimate
complaint gcc has here (but its complaint seems invalid).

>  To summarize, I think the only explanation for this warning is a compiler
> bug and I do agree that the assertion is useless anyhow and so could be
> removed without losing anything.

I think I'll retain it anyway, but change '<=' to '<'.
It's like guarding a division with assert(0!=denominator).
If I see that assertion, I know at a glance that the
division is protected. Otherwise, when I review the code
years later, I have a question that requires a nonzero
amount of thought.