chromium / subspace

A concept-centered standard library for C++20, enabling safer and more reliable products and a more modern feel for C++ code.; Also home of Subdoc the code-documentation generator.
https://suslib.cc
Apache License 2.0
89 stars 15 forks source link

Prevent clobbering of data in Result's tail padding internally and externally #407

Closed danakj closed 10 months ago

danakj commented 11 months ago

https://github.com/llvm/llvm-project/issues/70494 points out that [[no_unique_address]] on a union member can cause its placement-new construction to clobber data in its tail padding, as ctors can and do write their entire sizeof() size, not just their data-size.

So we do some wild things to prevent this from occurring:

The T and E union are grouped with the state into a struct, which we call Inner. This structure is always constructed together as a group when changing the state to Ok or Err. This ensures that we construct T or E and then set the state (which follows in the struct) thus avoiding clobbering of the state.

Second, the Inner struct needs to be protected from clobbering other things in its tail padding. So it is wrapped in another struct that conditionally applies [[no_unique_address]] to it, which we call MaybeNoUniqueAddress.

We mark Inner as [[no_unique_address]] only when there is no tail padding in T or E. Then the tail padding of Inner which is introduced by the state flag can be used by things external to the Result.

All of this is put into the outermost structure StorageVoid or StorageNonVoid for a void T or not, respectively. Everything is marked [[no_unique_address]] except Inner which is conditionally marked as mentioned above.

In order to do this, we've rewritten the entire implementation of Result's storage. In the process, copy/move operations and destructors are all trivial if the same for both T and E are trivial, which fixes #403.

The Clang 18 and GCC builds are failing due to the Clang-18 apt package not including libclang: https://github.com/llvm/llvm-project/issues/73402