From 5c183bda866e4410a66cda550d6afa2428408879 Mon Sep 17 00:00:00 2001
From: Protobuf Team Bot <protobuf-github-bot@google.com>
Date: Sat, 16 Mar 2024 14:09:39 -0700
Subject: [PATCH] Clear oneof message fields even on arena on non-OPT builds.

PiperOrigin-RevId: 616461373
---
 .../cpp/field_generators/message_field.cc     |  8 ++++-
 src/google/protobuf/port.h                    |  9 +++++
 src/google/protobuf/proto3_arena_unittest.cc  | 33 +++++++++++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/src/google/protobuf/compiler/cpp/field_generators/message_field.cc b/src/google/protobuf/compiler/cpp/field_generators/message_field.cc
index 8fe22a849f76..e473681c5caf 100644
--- a/src/google/protobuf/compiler/cpp/field_generators/message_field.cc
+++ b/src/google/protobuf/compiler/cpp/field_generators/message_field.cc
@@ -15,6 +15,7 @@
 
 #include "absl/log/absl_check.h"
 #include "absl/memory/memory.h"
+#include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
 #include "absl/strings/substitute.h"
 #include "google/protobuf/compiler/cpp/field.h"
@@ -595,7 +596,12 @@ void OneofMessage::GenerateClearingCode(io::Printer* p) const {
   p->Emit(R"cc(
     if (GetArena() == nullptr) {
       delete $field_$;
-    })cc");
+    } else if ($pbi$::DebugHardenClearOneofMessageOnArena()) {
+      if ($field_$ != nullptr) {
+        $field_$->Clear();
+      }
+    }
+  )cc");
 }
 
 void OneofMessage::GenerateMessageClearingCode(io::Printer* p) const {
diff --git a/src/google/protobuf/port.h b/src/google/protobuf/port.h
index 91968ae657e0..73e08a21afcc 100644
--- a/src/google/protobuf/port.h
+++ b/src/google/protobuf/port.h
@@ -238,6 +238,15 @@ inline constexpr bool DebugHardenStringValues() {
 #endif
 }
 
+// Returns true if force clearing oneof message on arena is enabled.
+inline constexpr bool DebugHardenClearOneofMessageOnArena() {
+#ifdef NDEBUG
+  return false;
+#else
+  return true;
+#endif
+}
+
 // Prefetch 5 64-byte cache line starting from 7 cache-lines ahead.
 // Constants are somewhat arbitrary and pretty aggressive, but were
 // chosen to give a better benchmark results. E.g. this is ~20%
diff --git a/src/google/protobuf/proto3_arena_unittest.cc b/src/google/protobuf/proto3_arena_unittest.cc
index 38f4df098efc..318f0f287374 100644
--- a/src/google/protobuf/proto3_arena_unittest.cc
+++ b/src/google/protobuf/proto3_arena_unittest.cc
@@ -14,6 +14,7 @@
 #include "absl/strings/match.h"
 #include "google/protobuf/arena.h"
 #include "google/protobuf/descriptor.h"
+#include "google/protobuf/port.h"
 #include "google/protobuf/text_format.h"
 #include "google/protobuf/unittest.pb.h"
 #include "google/protobuf/unittest_proto3_arena.pb.h"
@@ -256,6 +257,38 @@ TEST(Proto3OptionalTest, OptionalFields) {
   EXPECT_EQ(serialized.size(), 0);
 }
 
+TEST(Proto3ArenaTest, CheckMessageFieldIsCleared) {
+  Arena arena;
+  auto msg = Arena::Create<TestAllTypes>(&arena);
+
+  // Referring to a saved pointer to a child message is never guaranteed to
+  // work. IOW, protobufs do not guarantee pointer stability. This test only
+  // does this to replicate (unsupported) user behaviors.
+  auto child = msg->mutable_optional_foreign_message();
+  child->set_c(100);
+  msg->Clear();
+
+  EXPECT_EQ(child->c(), 0);
+}
+
+TEST(Proto3ArenaTest, CheckOneofMessageFieldIsCleared) {
+  if (!internal::DebugHardenClearOneofMessageOnArena()) {
+    GTEST_SKIP() << "arena allocated oneof message fields are not cleared.";
+  }
+
+  Arena arena;
+  auto msg = Arena::Create<TestAllTypes>(&arena);
+
+  // Referring to a saved pointer to a child message is never guaranteed to
+  // work. IOW, protobufs do not guarantee pointer stability. This test only
+  // does this to replicate (unsupported) user behaviors.
+  auto child = msg->mutable_oneof_nested_message();
+  child->set_bb(100);
+  msg->Clear();
+
+  EXPECT_EQ(child->bb(), 0);
+}
+
 TEST(Proto3OptionalTest, OptionalFieldDescriptor) {
   const Descriptor* d = protobuf_unittest::TestProto3Optional::descriptor();