Converting extended file_io_error to status_code

[gix]

When file_io_error contains extra info (i.e. LLFIO_DISABLE_PATHS_IN_FAILURE_INFO == 1), how does one convert it to a normal status_code? It contains the correct domain, and file_io_error_value_type contains the error code, so this ought to be possible. But the following doesn't compile:

status_result<void> foo() {
    auto result = llfio::file({}, "foo");
    if (!result)
        return std::move(result).error(); // <---
}

Using MSVC 19.31 if that makes a difference.

[BurningEnlightenment]

Well, the implicit conversion from llfio::file_io_error (which is an domain type erased status_code with 24b / 16b storage depending on sizeof(size_t) and alignof(size_t)) to a status_code<erased<intptr_t>> (8b / 4b storage) is rightfully disabled as that will loose information which the file_io_error_domain might access in the future.

However, currently there exists an explicit conversion from status_code<void> const & which does what you want (although I'm very doubtful whether this should be allowed to work in the general case, haven't checked though). You can allow outcome to use the explicit conversions by wrapping your error type in a failure_type<>. In practice this is what OUTCOME_TRY or the as_failure() method does. So all in all the following variants do work currently:

1. variant

   status_result<void> foo() {
   auto result = llfio::file({}, "foo");
   if (!result)
       return std::move(result).as_failure(); // <---
   }

2. variant

   status_result<void> foo() {
   OUTCOME_TRY(auto &&f, llfio::file({}, "foo"));
   }

[BurningEnlightenment]

there exists an explicit conversion from status_code<void> const & [...] although I'm very doubtful whether this should be allowed to work

So after thinking about it a bit more and checking the sources, I came to the conclusion that this is likely to work on platforms where sizeof(std::intptr_t) >= 8 holds (it involves a bit of undefined behaviour). On all other platforms performing system_code(someFileIoError).message() can crash due to accessing uninitialized memory: https://github.com/ned14/llfio/blob/ae7f9c5a92879285ad5100c89efc47ce1cb0031b/include/llfio/v2.0/status_code.hpp#L122 Note that this also applies to conversions to all custom domain type erased status_codes with a value_type smaller than 8b which are allowed due to the fact that the common base domain report a payload size of 4b.

@ned14 at the very least file_io_error_domain should override payload_info() and _do_erased_copy() to require at least 4b of additional storage and force _thread_id to some invalid thread id.

[gix]

Indeed, .as_failure() is the way I usually use to forward errors, which didn't work either. But it seems to have been fixed with current master, so from my side this can be closed.

[ned14]

The void status code copy constructor perhaps should be more "opt in", but it should be safe. What it does is a runtime attempt to copy whatever the source domain thinks ought to be copied if and only if the destination status code is big enough. If it isn't big enough, you get a default constructed status code. Almost without doubt WG21 will want that to throw an exception instead, and this is a rare situation where I'm very comfortable with that.

If you're all thinking that perhaps a tag type ought to activate that specific constructor, and it ought to throw if it fails to copy construct, I'm happy to make that change. Do vote!

Re: the OP's original question, you can either make your destination status code big enough to transport the LLFIO status code (this is what I generally do), or you can drop the additional LLFIO failure info state using .as_failure(). It looks like you already figured that out.

@ned14 at the very least file_io_error_domain should override payload_info() and _do_erased_copy() to require at least 4b of additional storage and force _thread_id to some invalid thread id.

Perhaps I am missing something here? The default erased copy just copies bits, and the LLFIO wrapper of status code just forwards to the domain it wraps so whatever it does. The intention is to drop the added LLFIO payload on request, is this not preserved?

[BurningEnlightenment]

Perhaps I am missing something here? The default erased copy just copies bits, and the LLFIO wrapper of status code just forwards to the domain it wraps so whatever it does. The intention is to drop the added LLFIO payload on request, is this not preserved?

The thing is, that the system_code still references a file_io_error_domain<posix_domain> and not a posix_domain. If you call .message() on that system_code, the file_io_error_domain::_do_message() method will try to access _thread_id which will not be contained within the system_code's storage if sizeof(intptr_t) < 8.

EDIT: I created a reprex/MCVE for my issue:

#include <llfio/llfio.hpp>

int main()
{
    using namespace LLFIO_V2_NAMESPACE;
    using namespace SYSTEM_ERROR2_NAMESPACE;

    utils::page_allocator<std::byte> alloc{};
    auto const pageSize = utils::page_size();
    auto page = alloc.allocate(pageSize);

    file_io_error ioError
            = LLFIO_V2_NAMESPACE::generic_error(errc::state_not_recoverable);

    // (I know this is very likely to be misaligned on 64Bit systems, but
    //  otherwise the problem/segfault is only triggered on 32Bit systems)
    auto erasedError
            = new (page + pageSize - sizeof(status_code_domain *) - sizeof(int))
                    errored_status_code<erased<int>>(ioError);

    // the following line seg faults
    erasedError->message();
}

[ned14]

Yes you are completely right. I'll fix it shortly.

[BurningEnlightenment]

yeah 05141b444da3ce26740fa0a22baac7f13704609e should do the trick.

[ned14]

Many thanks for the bug report and repro!