Created attachment 37676 [details]
compressed compilation temp file

This works in GCC 6 on aarch64 as far as I can tell.

After further analysis we've discovered that the optimizer does not remove the tests, it alters the comparison of (ec == enumerated value).   If one replaces the "ec" with "ec.value()" in the expressions, such that you are explicitly comparing the interger error value the code works correctly when optimized:

{{{
     :

    // Retry operation if interrupted by signal.
    if (ec.value() == boost::asio::error::interrupted)
      continue;

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

     :
}}}

Alternatively, adding an inline operator:

{{{
      inline friend bool operator==( const error_code & lhs,
                                     const int & rhs ) BOOST_SYSTEM_NOEXCEPT
      {
        return lhs.m_val == rhs;
      }
}}}

to error_code class also avoids the issue.

This issue also manifests itself on fully up-to-date Ubuntu 15.10, which has the following version g++ (Ubuntu 5.2.1-22ubuntu2) 5.2.1 20151010.

Here's also a bug report that confirms this issue on Fedora 23:

https://lists.isc.org/pipermail/kea-users/2016-February/000232.html, 
https://lists.isc.org/pipermail/kea-users/2016-February/000234.html

The reported version there is:

g++ (GCC) 5.3.1 20151207 (Red Hat 5.3.1-2)

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.

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 issue manifests itself also on g++ 5.4.0, the default compiler in Ubuntu 16.04. Exact version: (Ubuntu 5.4.0-6ubuntu1~16.04.2) 5.4.0 20160609.

(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.

(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.