[PATCH] Support: ensure proper state in ErrorOr copy ctors before calling 'get'
Michael Spencer
bigcheesegs at gmail.com
Mon Feb 4 12:00:09 PST 2013
On Mon, Feb 4, 2013 at 10:48 AM, Meador Inge <meadori at codesourcery.com> wrote:
> Some paths through the copy constructors for 'ErrorOr' were calling
> 'get' when 'HasError' and 'IsValid' were not properly initialized.
> Depending on what happened to be in memory for those member variables
> the asserts in 'get' might incorrectly fire. Fixed by ensuring that
> the member variables in question are always initialized before calling
> 'get'.
>
> I found this when running the LLD test suite on GNU/Linux. OK?
>
> ---
> include/llvm/Support/ErrorOr.h | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/include/llvm/Support/ErrorOr.h b/include/llvm/Support/ErrorOr.h
> index 452d85c..828d77b 100644
> --- a/include/llvm/Support/ErrorOr.h
> +++ b/include/llvm/Support/ErrorOr.h
> @@ -202,18 +202,17 @@ public:
> // Construct an invalid ErrorOr if other is invalid.
> if (!Other.IsValid)
> return;
> + IsValid = true;
> if (!Other.HasError) {
> // Get the other value.
> - new (get()) storage_type(*Other.get());
> HasError = false;
> + new (get()) storage_type(*Other.get());
> } else {
> // Get other's error.
> Error = Other.Error;
> HasError = true;
> Error->aquire();
> }
> -
> - IsValid = true;
> }
>
> ErrorOr &operator =(const ErrorOr &Other) {
> @@ -231,11 +230,11 @@ public:
> // Construct an invalid ErrorOr if other is invalid.
> if (!Other.IsValid)
> return;
> + IsValid = true;
> if (!Other.HasError) {
> // Get the other value.
> - IsValid = true;
> - new (get()) storage_type(std::move(*Other.get()));
> HasError = false;
> + new (get()) storage_type(std::move(*Other.get()));
> // Tell other not to do any destruction.
> Other.IsValid = false;
> } else {
> @@ -245,8 +244,6 @@ public:
> // Tell other not to do any destruction.
> Other.IsValid = false;
> }
> -
> - IsValid = true;
> }
>
> ErrorOr &operator =(ErrorOr &&Other) {
> --
> 1.8.1
>
LGTM.
- Michael Spencer
More information about the llvm-commits
mailing list