Re: [lmi] Unexpected gcc diagnostic

[Vadim Zeitlin]

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)

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

GC> on our corporate server, we observe:
GC> 
GC> In file included from /opt/lmi/src/lmi/any_member.hpp:65,
GC>                  from /opt/lmi/src/lmi/mec_state.hpp:27,
GC>                  from /opt/lmi/src/lmi/ihs_irc7702a.hpp:28,
GC>                  from /opt/lmi/src/lmi/ihs_irc7702a.cpp:30:
GC> /opt/lmi/src/lmi/ihs_irc7702a.cpp: In member function ‘double 
Irc7702A::DetermineLowestBft() const’:
GC> /opt/lmi/src/lmi/ihs_irc7702a.cpp:1306:32: error: zero as null pointer 
constant [-Werror=zero-as-null-pointer-constant]
GC>  1306 |     LMI_ASSERT(Bfts.begin() <= last_bft_in_test_period);
GC>       |                                ^~~~~~~~~~~~~~~~~~~~~~~
GC> /opt/lmi/src/lmi/assert_lmi.hpp:56:12: note: in definition of macro 
‘LMI_ASSERT’
GC>    56 |         if(condition)                                           \
GC>       |            ^~~~~~~~~
GC> 
GC> But the build succeeds with no warnings on my own PC,
GC> using "10-win32" (possibly a slightly later version
GC> than the same-versioned compiler on the server).

 I also don't see it here with the version above.

GC> And I imagine we'd already know if github CI got this warning.

 Yes, indeed.

GC> The code is:
GC> 
GC> double Irc7702A::DetermineLowestBft() const
GC> {
GC>     LMI_ASSERT(0 <= TestPeriodLen && 0 <= TestPeriodDur);
GC>     std::vector<double>::const_iterator last_bft_in_test_period = std::min
GC>         (Bfts.end()
GC>         ,Bfts.begin() + std::min(TestPeriodLen, TestPeriodDur)
GC>         );
GC>     LMI_ASSERT(Bfts.begin() <= last_bft_in_test_period);
GC> // TAXATION !! This is harmful for inforce if inforce history is unreliable:
GC>     LowestBft = *std::min_element(Bfts.begin(), last_bft_in_test_period);
GC>     return LowestBft;
GC> }
GC> 
GC> I reason that the assertion is superfluous and can never fire:
GC>   - TestPeriodLen and TestPeriodDur are both 'int', and
GC>       they've already been asserted to be nonnegative;
GC>   - begin() plus the sum of two nonnegative ints cannot be
GC>       less than begin();

 Moreover, if it were, this would already be undefined behaviour.

GC>   - end() cannot be less than begin();
GC>   - the min() of two things cannot be less than begin() when
GC>       both things are not less than begin().

 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.

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.

GC> I imagine std::vector::const_iterator is of unsigned type,

 In most implementations it's a pointer.

 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.

 Regards,
VZ

pgp7nfSjE_S6E.pgp
Description: PGP signature