Closed dkrejsa closed 1 year ago
Yes, the erasure casting of status_code<erased<...>>
seems to have been written with only LittleEndian in mind:
https://github.com/ned14/status-code/blob/833a0a6d9d8290fdf5d00705e7b0f0bf75b2c029/include/status-code/config.hpp#L337-L342
Removing these static cast overloads should fix the bug https://github.com/ned14/status-code/blob/833a0a6d9d8290fdf5d00705e7b0f0bf75b2c029/include/status-code/config.hpp#L374-L378
as the ones working with explicit padding would do the right:tm: thing in any case, though I don't know why they were added in the first place (enhanced C++14/17 compat?).
li r9,67
addi r4,r1,112
std r9,120(r1)
in .main
vs the load in generic_code::_do_failure
lwz r3,8(r4)
That code was donated, though as I accepted it, it's on me.
I generally call #error
when I write endian specific code and I detect big endian. Not because I don't support it, but because I have zero way of ever testing big endian code.
I've just arrived at the WG21 Varna meeting so there is an excellent chance I'll get this fixed this week. Thanks for reporting this issue, I'll be honest in saying I never expected any of my code to ever run on big endian.
Thank you both! I'll keep an eye on this.
Try that fix and let me know how you get on.
At least the optimized assembly looks like it's correct now:
lis r9,67 ; r9 = 0x0000'0000'0043'0000
rldicr r9,r9,16,47 ; r9 = 0x0000'0043'0000'0000
addi r4,r1,112 ; save address of the "code" parameter for `_do_failure` in the second argument register
std r9,120(r1)
Hi, I modified the header boost/outcome/experimental/status-code/config.hpp based upon the changes to include/status-code/config.hpp in the commits
https://github.com/ned14/status-code/commit/52692befeb5556b725312f88fc0c3bab249dec25 https://github.com/ned14/status-code/commit/1be0aa2c0b08349fc968454846fd4ce547a2f931
I rebuilt and re-ran on the LP64 Big Endian P5020ds target all the Boost outcome tests from Boost 1.81, including the two tests experimental-core-outcome-status.test and experimental-core-result-status.test that previously failed with a call to std::terminate() on this target. All those tests passed. Thanks very much!
Am I correct that the changes in single-header/system_error2-nowindows.hpp, single-header/system_error2.hpp, and test/main.cpp would apply to the stand-alone status-code or outcome/experimental setup and not to a vanilla Boost installation?
Now you've confirmed this fixes things for you, I'll cycle the Outcome release to include this fix, so it should turn up in Standalone Outcome by tomorrow.
There is no reason it should not appear in Boost.Outcome in the next Boost release.
Thanks for reporting this issue, and for testing that the fix works!
The following code causes a big-endian 64-bit target (LP64 memory model) to terminate the program using std::terminate():
The problem appears to occur because the non-zero
errc::no_link
value is stored as an 8-byte, big-endianintptr_t
value after the domain pointer in theerror
object, so that the non-zero part of the error code value is stored in the last 4 bytes. However, when the time comes to check that the value represents an error rather than a success case, the _generic_code_domain's_do_failure()
method casts theconst status_code<void> & code
argument as aconst generic_code &
, andgeneric_code
uses a 4-byteerrc
value_type member immediately following the domain pointer.So when the
value()
is computed, it takes the 4 bytes immediately following the domain pointer that were the most-significant 4 bytes of theintptr_t
value representingerrc::no_link
in theerror
object, rather than the least-significant 4 bytes. Since the most-significant 4 bytes are zero, the value looks like a success value rather than an error code, and the errored_status_code_check()
function terminates the process.Seen using Boost 1.81 on VxWorks, built for LP64 on a Freescale P5020DS board.