[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