Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Outcome riddled with UB due to not using std::launder() #185

Open
ned14 opened this issue Apr 13, 2019 · 3 comments

Comments

@ned14
Copy link
Owner

commented Apr 13, 2019

I was informed in the bar during the ACCU 2019 conference that Outcome is riddled with UB. I did not believe them. However, upon reading https://wg21.link/P0532, it turns out that they were correct.

Wherever there is a possibility that a const type could be in a T, EC or EP, one cannot use placement new and discard the return value without introducing UB. To avoid UB, one must do one of either:

  1. Always use the pointer returned by placement new to access the object just placement newed.

  2. Use std::launder on every access of the placement newed object.

Outcome's implementation needs to be audited for all use of placement new where the returned pointer is discarded, and fixups applied as where necessary.

@ned14 ned14 added the bug label Apr 13, 2019

@ned14 ned14 added this to the Boost 1.71 cutoff milestone Jun 17, 2019

ned14 added a commit that referenced this issue Jun 18, 2019

@ned14 ned14 removed this from the Boost 1.71 cutoff milestone Jun 18, 2019

@ned14

This comment has been minimized.

Copy link
Owner Author

commented Jun 19, 2019

Added a FAQ entry describing the current policy and approach to this bug, which is expected to remain open for a long time to come: https://ned14.github.io/outcome/faq/#is-outcome-riddled-with-undefined-behaviour-for-const-const-containing-and-reference-containing-types

@BurningEnlightenment

This comment has been minimized.

Copy link

commented Jun 22, 2019

@ned14 would you accept a patch implementing the second solution or is this deemed infeasible?

@ned14

This comment has been minimized.

Copy link
Owner Author

commented Jun 22, 2019

I didn't want to say anything about internal committee proceedings in outward facing documentation, but there is a reasonable chance that the need for std::launder will go away in C++ 23, as the memory model would be modified to no longer make it needed. As no compiler ever did anything with it anyway, that's no bad thing. But it also means that work done on this right now would be wasted effort. But thanks for volunteering, very kind of you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.