Fix double-free in absl::StatusOr<T>::emplace when value constructor throws

When `emplace()` is called on an `ok()` `StatusOr<T>`, it calls
`Clear()` to destroy the existing object. If the new value's
constructor throws an exception, `status_` was previously left as
`OkStatus`. This caused `~StatusOrData()` to erroneously destroy the
uninitialized memory again during stack unwinding.

This CL fixes the issue by temporarily setting `status_` to an
internal non-ok state (using the zero-allocation inline
representation) before calling `MakeValue()`, restoring it to
`OkStatus` only if construction succeeds.

PiperOrigin-RevId: 945290296
Change-Id: I17111c161f9d3d9f44ae014c49ab76960c7efc7b

absl/status/internal/status_internal.h

diff --git a/absl/status/internal/status_internal.h b/absl/status/internal/status_internal.h
index d2892c5..59ce36a 100644
--- a/absl/status/internal/status_internal.h
+++ b/absl/status/internal/status_internal.h
@@ -63,6 +63,10 @@
 enum class StatusCode : int;
 enum class StatusToStringMode : int;
 
+// Forward declaration of StatusOr for Status friendship.
+template <typename T>
+class StatusOr;
+
 namespace status_internal {
 #ifndef SWIG
 class StatusPrivateAccessor;

absl/status/status.h

diff --git a/absl/status/status.h b/absl/status/status.h
index faed05e..0b39b9b 100644
--- a/absl/status/status.h
+++ b/absl/status/status.h
@@ -708,6 +708,8 @@
 
   friend class absl::status_internal::StatusPrivateAccessor;
   friend class absl::status_internal::StatusPrivateAccessorForStatusBuilder;
+  template <typename T>
+  friend class absl::StatusOr;
 #endif  // !SWIG
 
   // Creates a status in the canonical error space with the specified

absl/status/statusor.h

diff --git a/absl/status/statusor.h b/absl/status/statusor.h
index ed34bb8..cabfd66 100644
--- a/absl/status/statusor.h
+++ b/absl/status/statusor.h
@@ -614,11 +614,14 @@
   T& emplace(Args&&... args) ABSL_ATTRIBUTE_LIFETIME_BOUND {
     if (ok()) {
       this->Clear();
-      this->MakeValue(std::forward<Args>(args)...);
-    } else {
-      this->MakeValue(std::forward<Args>(args)...);
-      this->status_ = absl::OkStatus();
+      // Temporarily transition to a non-ok status (using the zero-allocation
+      // inlined representation) so that if MakeValue() throws an exception,
+      // ok() returns false during stack unwinding and ~StatusOrData() does not
+      // attempt to destroy uninitialized memory.
+      this->status_ = absl::Status(absl::StatusCode::kInternal);
     }
+    this->MakeValue(std::forward<Args>(args)...);
+    this->status_ = absl::OkStatus();
     return this->data_;
   }
 
@@ -630,11 +633,14 @@
              Args&&... args) ABSL_ATTRIBUTE_LIFETIME_BOUND {
     if (ok()) {
       this->Clear();
-      this->MakeValue(ilist, std::forward<Args>(args)...);
-    } else {
-      this->MakeValue(ilist, std::forward<Args>(args)...);
-      this->status_ = absl::OkStatus();
+      // Temporarily transition to a non-ok status (using the zero-allocation
+      // inlined representation) so that if MakeValue() throws an exception,
+      // ok() returns false during stack unwinding and ~StatusOrData() does not
+      // attempt to destroy uninitialized memory.
+      this->status_ = absl::Status(absl::StatusCode::kInternal);
     }
+    this->MakeValue(ilist, std::forward<Args>(args)...);
+    this->status_ = absl::OkStatus();
     return this->data_;
   }
 

absl/status/statusor_test.cc

diff --git a/absl/status/statusor_test.cc b/absl/status/statusor_test.cc
index 2205acf..d03e8be 100644
--- a/absl/status/statusor_test.cc
+++ b/absl/status/statusor_test.cc
@@ -406,6 +406,53 @@
                                  Field(&InPlaceHelper::y, Pointee(4)))));
 }
 
+#ifdef ABSL_HAVE_EXCEPTIONS
+class ThrowOnEmplace {
+ public:
+  explicit ThrowOnEmplace(int* counter, int val) : destructor_calls_(counter) {
+    if (val < 0) {
+      throw std::runtime_error("expected");
+    }
+    // While destructor_calls tracks the logic, ptr_ ensures that a double
+    // destruction actually results in a reliable crash. Performing a real heap
+    // allocation and deallocation (new/delete) guarantees that AddressSanitizer
+    // (ASAN) or the heap allocator will instantly catch the double-free if the
+    // bug regresses, rather than relying solely on the integer check.
+    ptr_ = new int(val);
+  }
+
+  ThrowOnEmplace(const ThrowOnEmplace&) = delete;
+  ThrowOnEmplace& operator=(const ThrowOnEmplace&) = delete;
+
+  ~ThrowOnEmplace() {
+    if (destructor_calls_) {
+      ++(*destructor_calls_);
+    }
+    delete ptr_;
+  }
+
+ private:
+  int* destructor_calls_ = nullptr;
+  int* ptr_ = nullptr;
+};
+
+TEST(StatusOr, EmplaceThrowsExceptionSafety) {
+  int destructor_calls = 0;
+  {
+    absl::StatusOr<ThrowOnEmplace> status_or(std::in_place, &destructor_calls,
+                                             1);
+    EXPECT_TRUE(status_or.ok());
+    EXPECT_THROW(status_or.emplace(&destructor_calls, -1), std::runtime_error);
+    EXPECT_FALSE(status_or.ok());
+    EXPECT_EQ(status_or.status().code(), absl::StatusCode::kInternal);
+  }
+  // Verifies that the initial object is properly destroyed by Clear() (count is
+  // 1), and that the exception thrown during replacement does not cause a
+  // second destruction (double-free) during stack unwinding.
+  EXPECT_EQ(destructor_calls, 1);
+}
+#endif  // ABSL_HAVE_EXCEPTIONS
+
 TEST(StatusOr, TestCopyCtorStatusOk) {
   const int kI = 4;
   const absl::StatusOr<int> original(kI);