[Bug target/69789] g++ -O2 is removing tests against a variable that can be changed

["tmark at isc dot org" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69789

--- Comment #8 from Thomas Markwalder <tmark at isc dot org> ---
(In reply to Jonathan Wakely from comment #7)
> (In reply to Thomas Markwalder from comment #5)
> > A bit more digging reveals that in the logic expression which fails:
> > 
> > {{{
> >     // Check if we need to run the operation again.
> >     if (ec == boost::asio::error::would_block
> >         || ec == boost::asio::error::try_again)
> >       return false;
> > }}}
> > 
> > The value for error_category:m_cat differs between the implicitly created
> > instances for would_block and try_again.
> 
> Do you mean that error_code(boost::asio::error::would_block) !=
> error_code(boost::asio::error::try_again) ?
> 
> I don't see how that's possible, since in the optimized GIMPLE dump they
> both use the same constructor, which isn't inlined. The two enums have the
> same value, and the constructor will call the same asio::make_error_code()
> overload that sets the category to boost::system_category().
> 
>              
> boost::system::error_code::error_code<boost::asio::error::basic_errors>
> (&D.94931, 11, 0B);
>               cleanup.88 = 1;
>               D.165192 = boost::system::operator== (ec, &D.94931);
>               if (D.165192 != 0) goto <D.165186>; else goto <D.165193>;
>               <D.165193>:
>              
> boost::system::error_code::error_code<boost::asio::error::basic_errors>
> (&D.94932, 11, 0B);
>               cleanup.89 = 1;
>               D.165197 = boost::system::operator== (ec, &D.94932);
>               if (D.165197 != 0) goto <D.165186>; else goto <D.165187>;
>               <D.165186>:
>               iftmp.87 = 1;
>               goto <D.165188>;
>               <D.165187>:
>               iftmp.87 = 0;
>               <D.165188>:
>               retval.86 = iftmp.87;
> 
> 
> So what exactly are you seeing? What digging have you done? Stepped through
> with the debugger?
> 
> The most likely explanation seems to be that ec.category() !=
> boost::system::system_category() and so the comparisons are always false.
> 
> 
> > If one splits the if into two separate if statements,  the values for m_cat
> > are the same.  This looks a like the behavior described under "Static
> > Variables in Inline Functions" described here:
> > 
> > http://processors.wiki.ti.com/index.php/C++_Inlining_Issues
> 
> This function has nothing to do with static variables.

Hello Jonathan:

It has been some months since I looked at this so let me try to recap. 
I did do a great deal of stepping through with the debugger as well as
instrumented the Boost code with couts in various places.  The basic issue
seems to be that when you build Boost such that BOOST_ERROR_CODE_HEADER_ONLY is 
defined, then the following function in <boost>/system/detail/error_code.ipp
is declared as inline:

  : 
# ifdef BOOST_ERROR_CODE_HEADER_ONLY
#   define BOOST_SYSTEM_LINKAGE inline
# else
#   define BOOST_SYSTEM_LINKAGE BOOST_SYSTEM_DECL
# endif

    BOOST_SYSTEM_LINKAGE const error_category & system_category()
BOOST_SYSTEM_NOEXCEPT
    {
      static const system_error_category  system_category_const;
      return system_category_const;
    }
  :

Apparently some combination of boost includes along with our own, and building
with optimization, we end up with more than once instance of this function
each with its own value for the static variable, system_error_category.  Thus
the value returned by invoking system_category() depends upon which instance
is invoked. The result is the aforementioned if-clause:

    // Check if we need to run the operation again.
    if (ec == boost::asio::error::would_block
        || ec == boost::asio::error::try_again)
      return false;

not evaluating to true even though it should. I confirmed with the debugger
that  ec.m_cat does not reliably equal would_block.m_cat or try_again.m_cat.
If you split the clause into two separate statements it works reliably:

    // Check if we need to run the operation again.
    if (ec == boost::asio::error::would_block)
      return false;

    // Check if we need to run the operation again.
    if (ec == boost::asio::error::try_again)
      return false;

Not necessarily meaningful, but noteworthy, and of course, I can't use that
as a solution as that's within the boost code.

If one compiles without optimization or one builds with Boost system library 
rather than with BOOST_ERROR_CODE_HEADER_ONLY defined, the issue disappears.
I did try to replicate it with only boost code and could not do so.  I think
there is some corner case we are hitting in permits there to be more than
one static instance of system_error_category.