diff --git a/src/google/protobuf/repeated_field_unittest.cc b/src/google/protobuf/repeated_field_unittest.cc index f5529ed784..529d872284 100644 --- a/src/google/protobuf/repeated_field_unittest.cc +++ b/src/google/protobuf/repeated_field_unittest.cc @@ -1677,6 +1677,14 @@ TEST(RepeatedPtrField, AddAllocated) { EXPECT_EQ(moo, &field.Get(index)); } +TEST(RepeatedPtrField, AddMethodsDontAcceptNull) { +#if !defined(NDEBUG) + RepeatedPtrField field; + EXPECT_DEATH(field.AddAllocated(nullptr), "nullptr"); + EXPECT_DEATH(field.UnsafeArenaAddAllocated(nullptr), "nullptr"); +#endif +} + TEST(RepeatedPtrField, AddAllocatedDifferentArena) { RepeatedPtrField field; Arena arena; diff --git a/src/google/protobuf/repeated_ptr_field.h b/src/google/protobuf/repeated_ptr_field.h index 08642b56c0..72beb1c2f5 100644 --- a/src/google/protobuf/repeated_ptr_field.h +++ b/src/google/protobuf/repeated_ptr_field.h @@ -443,11 +443,13 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template void AddAllocated(Value* value) { typename TypeImplementsMergeBehavior>::type t; + ABSL_DCHECK_NE(value, nullptr); AddAllocatedInternal(value, t); } template void UnsafeArenaAddAllocated(Value* value) { + ABSL_DCHECK_NE(value, nullptr); // Make room for the new pointer. if (current_size_ == total_size_) { // The array is completely full with no cleared objects, so grow it. @@ -1179,6 +1181,7 @@ class RepeatedPtrField final : private internal::RepeatedPtrFieldBase { // (ii) if this field holds strings, the passed-in string *must* be // heap-allocated, not arena-allocated. There is no way to dynamically check // this at runtime, so User Beware. + // Requires: value != nullptr void AddAllocated(Element* value); // Removes and returns the last element, passing ownership to the caller. @@ -1201,6 +1204,7 @@ class RepeatedPtrField final : private internal::RepeatedPtrFieldBase { // If you put temp_field on the arena this fails, because the ownership // transfers to the arena at the "AddAllocated" call and is not released // anymore, causing a double delete. UnsafeArenaAddAllocated prevents this. + // Requires: value != nullptr void UnsafeArenaAddAllocated(Element* value); // Removes and returns the last element. Unlike ReleaseLast, the returned