From bef5b759f1fa511e3c08380a4e8635832d5d39fd Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 28 Sep 2023 06:52:04 -0700 Subject: [PATCH] Fix bug in reflection based Swap of map fields. It was swapping the arena pointers too when they differed. In that case a full copy must be made instead. The instances can't change arenas. PiperOrigin-RevId: 569166797 --- src/google/protobuf/map.h | 6 ++++-- src/google/protobuf/map_field_inl.h | 2 +- src/google/protobuf/map_test.inc | 7 ++++++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/google/protobuf/map.h b/src/google/protobuf/map.h index 404fd5d925..30481dbd64 100644 --- a/src/google/protobuf/map.h +++ b/src/google/protobuf/map.h @@ -536,7 +536,7 @@ class PROTOBUF_EXPORT UntypedMapBase { public: Arena* arena() const { return this->alloc_.arena(); } - void Swap(UntypedMapBase* other) { + void InternalSwap(UntypedMapBase* other) { std::swap(num_elements_, other->num_elements_); std::swap(num_buckets_, other->num_buckets_); std::swap(seed_, other->seed_); @@ -1501,7 +1501,9 @@ class Map : private internal::KeyMapBase> { } } - void InternalSwap(Map* other) { this->Swap(other); } + void InternalSwap(Map* other) { + internal::UntypedMapBase::InternalSwap(other); + } hasher hash_function() const { return {}; } diff --git a/src/google/protobuf/map_field_inl.h b/src/google/protobuf/map_field_inl.h index 2d8dec910d..6a1fc8a6fe 100644 --- a/src/google/protobuf/map_field_inl.h +++ b/src/google/protobuf/map_field_inl.h @@ -129,7 +129,7 @@ template void TypeDefinedMapFieldBase::Swap(MapFieldBase* other) { MapFieldBase::Swap(other); auto* other_field = DownCast(other); - map_.Swap(&other_field->map_); + map_.swap(other_field->map_); } template diff --git a/src/google/protobuf/map_test.inc b/src/google/protobuf/map_test.inc index 9630a13963..1f19181318 100644 --- a/src/google/protobuf/map_test.inc +++ b/src/google/protobuf/map_test.inc @@ -1270,7 +1270,6 @@ TEST_F(MapImplTest, SwapArena) { TEST_F(MapImplTest, SwapFieldArenaReflection) { MapReflectionTester reflection_tester(UNITTEST::TestMap::descriptor()); - Arena arena; { // Tests filled lfs and empty rhs. @@ -1289,9 +1288,15 @@ TEST_F(MapImplTest, SwapFieldArenaReflection) { reflection->SwapFields(lhs, &rhs, fields); reflection_tester.ExpectClearViaReflection(*lhs); + + // Add an entry to make sure it is using the right arena. + (*lhs->mutable_map_int32_int32())[1234] = 1234; } reflection_tester.ExpectMapFieldsSetViaReflection(rhs); + + // Add an entry to make sure it is using the right arena. + (*rhs.mutable_map_int32_int32())[1234] = 1234; } }