Beast: unreachable code in asio/buffer.hpp with optimizations

[vinniefalco]

MSVC generates this warning:

1>c:usersvinniesrcboostboostasiobuffer.hpp(244): warning C4702: unreachable code

We need to first determine if it is a beast problem.

[stale[bot]]

This issue has been open for a while with no activity, has it been resolved?

[Gibson85]

Same problem here:

D:\Sources\boost\boost\asio\buffer.hpp(244): error C2220: the following warning is treated as an error
D:\Sources\boost\boost\asio\buffer.hpp(244): error C2220: the following warning is treated as an error
D:\Sources\boost\boost\asio\buffer.hpp(244): warning C4702: unreachable code
D:\Sources\boost\boost\asio\buffer.hpp(244): warning C4702: unreachable code
D:\Sources\boost\boost\asio\buffer.hpp(244): warning C4702: unreachable code
D:\Sources\boost\boost\asio\buffer.hpp(244): warning C4702: unreachable code
D:\Sources\boost\boost\asio\buffer.hpp(244): warning C4702: unreachable code
D:\Sources\boost\boost\asio\buffer.hpp(244): warning C4702: unreachable code
D:\Sources\boost\boost\asio\buffer.hpp(244): warning C4702: unreachable code
LINK : fatal error LNK1257: code generation failed

[madmongo1]

@Gibson85 Which version of boost are you running?

[Gibson85]

I am using version 1.72 (C++17)

[madmongo1]

@Gibson85

Can you quickly check that line to be sure I'm looking the correct code?

My source file is:

  /// Construct a non-modifiable buffer from a modifiable one.
  const_buffer(const mutable_buffer& b) BOOST_ASIO_NOEXCEPT
    : data_(b.data()),        <<<--- LINE 244
      size_(b.size())
#if defined(BOOST_ASIO_ENABLE_BUFFER_DEBUGGING)
      , debug_check_(b.get_debug_check())
#endif // BOOST_ASIO_ENABLE_BUFFER_DEBUGGING
  {
  }

Does that match yours?

[vinniefalco]

That's consistent with what I see.

[Gibson85]

Yes, that's the line.

[Gibson85]

The problem is most probably caused by "whole program optimization". Linking e.g. a static lib (using boost internally) against a program causes this warning for me.

[madmongo1]

Thanks for letting us know.

If course this would place the problem firmly outside the domain of beast.

Would you be happy to close the issue?

[vinniefalco]

We have to do something about it, even if it is just to make a minimally reproducible test program that we can show Chris and get it fixed.

[maddinat0r]

Disabling "Whole Program Optimization" did not fix the warning for me, I had to completely disable optimizations with /Od.
Disabling inline function expansion (/Ob0) moves the warning from asio to beast:

C:\libs\boost_1_72_0\boost\beast\core\impl\buffers_cat.hpp(198): warning C4702: unreachable code

    struct dereference
    {
        const_iterator const& self;

        reference
        operator()(mp11::mp_size_t<0>)
        {
            BOOST_BEAST_LOGIC_ERROR_RETURN({},
                "Dereferencing a default-constructed iterator");
        }

        template<class I>
        reference operator()(I)
        {
            return *self.it_.template get<I::value>(); // <--- line 198
        }
    };

Using Boost 1.72.0.

[vinniefalco]

Thanks for narrowing this down. We need to figure out how to modify the beast code to make this warning stop.

[laudrup]

For me at least, this also causes a compiler crash (internal compiler error) using MSVC 2017. Although that might not related to the potential bug in boost/asio, it definitely makes it slightly more serious since it's not possible to simply disable the warning.

I've currently worked around it by disabling optimizations for the specific translation unit, but although I do not care too much about performance in my use case, it seems quite wrong, especially considering this is code intended for release.

Is there any other workaround (eg. using another buffer type somehow) or any update on a fix for this?

Thanks.

[vinniefalco]

Is there any other workaround (eg. using another buffer type somehow) or any update on a fix for this?

No update, no workaround - and it is annoying the shit out of me. If we could put together a small reproducible main function that exhibits the problem, then it might be tractable.

[madmongo1]

@laudrup if you could share a complete 1-function program that exhibits the crash I'll take a look.

[laudrup]

@madmongo1

To be honest, I've only experienced the ICE problem in our build pipeline and haven't been able to reproduce the crash on my development machine, but I assume the actual issue is the "unreachable code" warning.

The warning issue can be reproduced by simply compiling the http_client_async example in release mode using either MSVC 2017 or 2019 (haven't tried with earlier versions) using boost 1.72. Not sure if that can be boiled down more than that.

Not really sure how much more I can help since I have limited knowledge of boost asio/beast internals, but of course I'll be happy to provide any help I can.

[korzhp]

@madmongo1 https://developercommunity.visualstudio.com/content/problem/880290/internal-compiler-error-with-msvc-2017-15918-boost.html

[laudrup]

It seems like this has been fixed in later versions of MSVC 2019 from around version 16.6.

To be honest I haven't tested exactly which version fixes the issue (switching compiler version on Windows is a bit of a hassle), but I can compile the http_client_async example just fine with version 16.6.1 at least.

Not sure if any actions should be taken, but I wanted to share that information at least.

[madmongo1]

@laudrup Thanks for letting us know. To be honest, I'm at a loss as to how we should proceed with this, if at all.

[maddinat0r]

Just tried it with VS 2019 version 16.6.0 and 16.6.2, and I'm still getting the same error (tried Boost 1.72.0 and 1.73.0) while building my project. I've also tried compiling the examples (Boost 1.73.0, MSVC v16.6.2, x86 and x64), without much luck either.

[laudrup]

@maddinat0r My bad. I had forgotten that it is only an issue in release builds (ie. with optimizations enabled).

Sorry about the confusion. This is still an issue :-(

[laudrup]

I do seem to be able to suppress the warning (ie. -wd4702) and have the code build and run without any issues though, so the ICE is at least no longer an issue (for me at least).

[maddinat0r]

I did some digging and I think I found the culprit of those warnings:
https://github.com/boostorg/beast/blob/6f57e5934c1f307e4510ee11dd09594526a3f5d0/include/boost/beast/core/impl/buffers_cat.hpp#L55
Changing that macro to return v for example gets rid of those warnings.
I think it's safe to say that ignoring that warning is a viable solution, as the compiler is warning about something we intentionally want to happen.

[vinniefalco]

Beautiful! So, I think a good fix would be:

#ifdef _MSC_VER
# define BOOST_BEAST_UNREACHABLE_RETURN(v) return v
#else
# define BOOST_BEAST_UNREACHABLE_RETURN(v) __assume(false)
#endif

[TheStormN]

I did some digging and I think I found the culprit of those warnings:
https://github.com/boostorg/beast/blob/6f57e5934c1f307e4510ee11dd09594526a3f5d0/include/boost/beast/core/impl/buffers_cat.hpp#L55

Changing that macro to return v for example gets rid of those warnings.
I think it's safe to say that ignoring that warning is a viable solution, as the compiler is warning about something we intentionally want to happen.

This does work. Although just ignoring the warning is not an option at all for codebases that are threating warnings as errors.

[madmongo1]

There are 2 uses of this macro that I could see:

Here's one:

        net::mutable_buffer
        operator*() const
        {
            BOOST_BEAST_LOGIC_ERROR_RETURN({},
                "Dereferencing a one-past-the-end iterator");
        }

It seems to me that the use of __assume(false) is an un-necessary optimisation, since in a well-defined program the code will never be called in any case.

__assume(false) is a useful optimisation when (for example) one knows that a certain case will not be taken in a switch statement that is executed often.

In this case the code path will be taken exactly once in a program's life, followed by a break into the debugger.

[TheStormN]

@madmongo1 Yeah, that's true. Btw you can see similar issue here: https://stackoverflow.com/questions/5512560/is-there-a-workaround-for-this-c4702-link-time-warning

It's the very same problem. The person there is trying to catch an invalid usage of template operator() and it is getting that error.

Anyway, I guess that you can just remove __assume(false) as proposed to get the compiler happy. :)

[madmongo1]

Yup, I'll go ahead and do that in the morning.

On Thu, 18 Jun 2020 at 20:28, Nikolay Baklicharov [e-mail]
wrote:

@madmongo1 https://github.com/madmongo1 Yeah, that's true. Btw you can
see similar issue here:
https://stackoverflow.com/questions/5512560/is-there-a-workaround-for-this-c4702-link-time-warning

It's the very same problem. The person there is trying to catch an invalid
usage of template operator() and it is getting that error.

Anyway, I guess that you can just remove __assume(false) as proposed to
get the compiler happy. :)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/boostorg/beast/issues/1582#issuecomment-646233621,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABHOZSISSLKTUQBGUSFNBATRXJMFVANCNFSM4HHJ2NLQ
.

--
Richard Hodges
hodges.[email protected]
office: [phone]
home: [phone]
mobile: [phone]

[TheStormN]

@madmongo1 Just a remainder in case you forgot to change that. :)

[madmongo1]

Thanks :) The release has been pretty hectic.

[TheStormN]

@madmongo1 Sorry but I still don't see changes in that regard. Forgot again? :)

[madmongo1]

Not forgotten. Just dealing with all the other stuff :)

[madmongo1]

Hmm. doesn't seem to cure it for me. What have I missed?

[TheStormN]

Don't know, it does fix it for me when I try it...