Add a comment that `is_cleared` is always false for repeated fields.

Also added a test to verify that no uninitialized access happens in NumExtensions.

Creation `Extension()` does zero initialization. Illustration: https://gcc.godbolt.org/z/37j8hrb9T

PiperOrigin-RevId: 698094874
pull/19314/head
Vitaly Goldshteyn 1 week ago committed by Copybara-Service
parent ea0ab2d9a5
commit a8c4159734
  1. 17
      csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.g.cs
  2. 1
      src/google/protobuf/extension_set.cc
  3. 7
      src/google/protobuf/extension_set.h
  4. 13
      src/google/protobuf/extension_set_unittest.cc

@ -1,17 +0,0 @@
#region Copyright notice and license
// Protocol Buffers - Google's data interchange format
// Copyright 2008 Google Inc. All rights reserved.
//
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file or at
// https://developers.google.com/open-source/licenses/bsd
#endregion
namespace Google.Protobuf.Reflection;
internal sealed partial class FeatureSetDescriptor
{
// Canonical serialized form of the edition defaults, generated by embed_edition_defaults.
private const string DefaultsBase64 =
"ChMYhAciACoMCAEQAhgCIAMoATACChMY5wciACoMCAIQARgBIAIoATABChMY6AciDAgBEAEYASACKAEwASoAIOYHKOgH";
}

@ -385,6 +385,7 @@ void* ExtensionSet::MutableRawRepeatedField(int number, FieldType field_type,
extension->is_pointer = true; extension->is_pointer = true;
extension->type = field_type; extension->type = field_type;
extension->is_packed = packed; extension->is_packed = packed;
ABSL_DCHECK(!extension->is_cleared);
switch (WireFormatLite::FieldTypeToCppType( switch (WireFormatLite::FieldTypeToCppType(
static_cast<WireFormatLite::FieldType>(field_type))) { static_cast<WireFormatLite::FieldType>(field_type))) {

@ -762,9 +762,10 @@ class PROTOBUF_EXPORT ExtensionSet {
// For singular types, indicates if the extension is "cleared". This // For singular types, indicates if the extension is "cleared". This
// happens when an extension is set and then later cleared by the caller. // happens when an extension is set and then later cleared by the caller.
// We want to keep the Extension object around for reuse, so instead of // We want to keep the Extension object around for reuse, so instead of
// removing it from the map, we just set is_cleared = true. This has no // removing it from the map, we just set is_cleared = true.
// meaning for repeated types; for those, the size of the RepeatedField //
// simply becomes zero when cleared. // This is always set to false for repeated types.
// The size of the RepeatedField simply becomes zero when cleared.
bool is_cleared : 1; bool is_cleared : 1;
// For singular message types, indicates whether lazy parsing is enabled // For singular message types, indicates whether lazy parsing is enabled

@ -1422,6 +1422,19 @@ TEST(ExtensionSetTest, ConstInit) {
EXPECT_EQ(set.NumExtensions(), 0); EXPECT_EQ(set.NumExtensions(), 0);
} }
// Make sure that is_cleared is set correctly for repeated fields.
TEST(ExtensionSetTest, NumExtensionsWithRepeatedFields) {
unittest::TestAllExtensions msg;
ExtensionSet set;
const auto* desc =
unittest::TestAllExtensions::descriptor()->file()->FindExtensionByName(
"repeated_int32_extension");
ASSERT_NE(desc, nullptr);
set.MutableRawRepeatedField(desc->number(), WireFormatLite::TYPE_INT32, false,
desc);
EXPECT_EQ(set.NumExtensions(), 1);
}
TEST(ExtensionSetTest, ExtensionSetSpaceUsed) { TEST(ExtensionSetTest, ExtensionSetSpaceUsed) {
unittest::TestAllExtensions msg; unittest::TestAllExtensions msg;
size_t l = msg.SpaceUsedLong(); size_t l = msg.SpaceUsedLong();

Loading…
Cancel
Save