ned14 / outcome

Provides very lightweight outcome<T> and result<T> (non-Boost edition)
https://ned14.github.io/outcome
Other
676 stars 62 forks source link

[experimental] runtime error: reference binding to null pointer of type 'const struct domain_type' #220

Closed Tradias closed 3 years ago

Tradias commented 4 years ago

Using experimental outcome. When compiling the following program:

#include <boost/outcome/experimental/status_result.hpp>

namespace outcome = boost::outcome_v2::experimental;

template <class T, class E = outcome::error>
using Result = outcome::status_result<T, E>;

template <class T>
using PosixResult = outcome::status_result<T, outcome::posix_code>;

Result<int> convert(const PosixResult<int> &posix_result) {
  return Result<int>(posix_result);
}

int main() { return convert(PosixResult<int>(0)).value(); }`

with:

g++ -Ioutcome-master/include -fsanitize=address,undefined -g main.cpp -o a.out

and then running it, outputs:

outcome-master/include/boost/outcome/experimental/status-code/status_code.hpp:244:114: runtime error: reference binding to null pointer of type 'const struct domain_type'

On a side note: The experimental version is great I really like using it. Can nicely be adopted to the use cases that I am facing and makes error handling so much more pleasant. 👍

ned14 commented 4 years ago

Can you confirm the version of Outcome for me please? I had thought that particular bug fixed a while back. You can find the version in the version.hpp header.

Tradias commented 4 years ago

Yes sorry. It is a clone of master from yesterday. version.hpp says 2.2.0.0. Compiler is g++ (Ubuntu 9.1.0-2ubuntu2~19.04) 9.1.0

ned14 commented 4 years ago

Well that's not good :( I'm currently fixing other bugs on the backlog, shall hopefully get to this later this week. Thanks for reporting this.

ned14 commented 4 years ago

Fixed! Thanks for the bug report!

Tradias commented 4 years ago

Confirmed working.

I have been using this as a workaround:

template <class T, class E, class P>
Result<T> to_result(outcome::status_result<T, E, P>&& custom_result)
{
    if (custom_result)
    {
        if constexpr (std::is_same_v<void, T>)
        {
            return outcome::success();
        }
        else
        {
            return outcome::success(std::move(custom_result).value());
        }
    }
    return std::move(custom_result).error();
}

Do you think I should switch (back) to return Result<T>(std::move(custom_result)); instead now?

ned14 commented 4 years ago

The report was always benign by the way. I simply worked around the report by adding a new pointer returning accessor used internally. I'd certainly remove any workarounds, then you'll spot any actually unsafe UB.

Tradias commented 3 years ago

This issue starts occuring again in Boost 1.74. Looks like this commit https://github.com/ned14/status-code/commit/36a8cd77926b2c9590209f5b803f5b032dd1fa65 undid your fix. I understand that the report is benign and will rework my code to consistently use one Result<> type instead of separate results like PosixResult<>. However until then I believe adding a UBSAN suppression would veil actual problems and I'd rather not have to use one.

ned14 commented 3 years ago

Well that's weird that I undid that fix. Thanks for reporting that I did.

ned14 commented 3 years ago

Fixed once again, thanks for the report!