Skip to content

Commit 5c183bd

Browse files
Clear oneof message fields even on arena on non-OPT builds.
PiperOrigin-RevId: 616461373
1 parent 7ef5207 commit 5c183bd

File tree

3 files changed

+49
-1
lines changed

3 files changed

+49
-1
lines changed

src/google/protobuf/compiler/cpp/field_generators/message_field.cc

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include "absl/log/absl_check.h"
1717
#include "absl/memory/memory.h"
18+
#include "absl/strings/str_cat.h"
1819
#include "absl/strings/string_view.h"
1920
#include "absl/strings/substitute.h"
2021
#include "google/protobuf/compiler/cpp/field.h"
@@ -595,7 +596,12 @@ void OneofMessage::GenerateClearingCode(io::Printer* p) const {
595596
p->Emit(R"cc(
596597
if (GetArena() == nullptr) {
597598
delete $field_$;
598-
})cc");
599+
} else if ($pbi$::DebugHardenClearOneofMessageOnArena()) {
600+
if ($field_$ != nullptr) {
601+
$field_$->Clear();
602+
}
603+
}
604+
)cc");
599605
}
600606

601607
void OneofMessage::GenerateMessageClearingCode(io::Printer* p) const {

src/google/protobuf/port.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,15 @@ inline constexpr bool DebugHardenStringValues() {
238238
#endif
239239
}
240240

241+
// Returns true if force clearing oneof message on arena is enabled.
242+
inline constexpr bool DebugHardenClearOneofMessageOnArena() {
243+
#ifdef NDEBUG
244+
return false;
245+
#else
246+
return true;
247+
#endif
248+
}
249+
241250
// Prefetch 5 64-byte cache line starting from 7 cache-lines ahead.
242251
// Constants are somewhat arbitrary and pretty aggressive, but were
243252
// chosen to give a better benchmark results. E.g. this is ~20%

src/google/protobuf/proto3_arena_unittest.cc

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "absl/strings/match.h"
1515
#include "google/protobuf/arena.h"
1616
#include "google/protobuf/descriptor.h"
17+
#include "google/protobuf/port.h"
1718
#include "google/protobuf/text_format.h"
1819
#include "google/protobuf/unittest.pb.h"
1920
#include "google/protobuf/unittest_proto3_arena.pb.h"
@@ -256,6 +257,38 @@ TEST(Proto3OptionalTest, OptionalFields) {
256257
EXPECT_EQ(serialized.size(), 0);
257258
}
258259

260+
TEST(Proto3ArenaTest, CheckMessageFieldIsCleared) {
261+
Arena arena;
262+
auto msg = Arena::Create<TestAllTypes>(&arena);
263+
264+
// Referring to a saved pointer to a child message is never guaranteed to
265+
// work. IOW, protobufs do not guarantee pointer stability. This test only
266+
// does this to replicate (unsupported) user behaviors.
267+
auto child = msg->mutable_optional_foreign_message();
268+
child->set_c(100);
269+
msg->Clear();
270+
271+
EXPECT_EQ(child->c(), 0);
272+
}
273+
274+
TEST(Proto3ArenaTest, CheckOneofMessageFieldIsCleared) {
275+
if (!internal::DebugHardenClearOneofMessageOnArena()) {
276+
GTEST_SKIP() << "arena allocated oneof message fields are not cleared.";
277+
}
278+
279+
Arena arena;
280+
auto msg = Arena::Create<TestAllTypes>(&arena);
281+
282+
// Referring to a saved pointer to a child message is never guaranteed to
283+
// work. IOW, protobufs do not guarantee pointer stability. This test only
284+
// does this to replicate (unsupported) user behaviors.
285+
auto child = msg->mutable_oneof_nested_message();
286+
child->set_bb(100);
287+
msg->Clear();
288+
289+
EXPECT_EQ(child->bb(), 0);
290+
}
291+
259292
TEST(Proto3OptionalTest, OptionalFieldDescriptor) {
260293
const Descriptor* d = protobuf_unittest::TestProto3Optional::descriptor();
261294

0 commit comments

Comments
 (0)