Add ABSL_ATTRIBUTE_LIFETIME_BOUND attribute on generated oneof acce…

…ssors. This allows the compiler to statically detect use-after-free bugs. PiperOrigin-RevId: 569504371
protocolbuffers · Sep 29, 2023 · e33e0d2 · e33e0d2
1 parent 8f831e9
commit e33e0d2
Show file tree

Hide file tree

Showing 8 changed files with 32 additions and 7 deletions.
diff --git a/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java b/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java
@@ -920,10 +920,10 @@ public void testOneofDescriptor() throws Exception {
     assertThat(messageType.getOneofs().get(0)).isSameInstanceAs(oneofDescriptor);
     assertThat(oneofDescriptor.getName()).isEqualTo("oneof_field");
 
-    assertThat(oneofDescriptor.getFieldCount()).isEqualTo(4);
+    assertThat(oneofDescriptor.getFieldCount()).isEqualTo(7);
     assertThat(field).isSameInstanceAs(oneofDescriptor.getField(1));
 
-    assertThat(oneofDescriptor.getFields()).hasSize(4);
+    assertThat(oneofDescriptor.getFields()).hasSize(7);
     assertThat(field).isEqualTo(oneofDescriptor.getFields().get(1));
   }
 

diff --git a/python/google/protobuf/internal/field_mask_test.py b/python/google/protobuf/internal/field_mask_test.py
@@ -53,7 +53,7 @@ def testDescriptorToFieldMask(self):
     mask = field_mask_pb2.FieldMask()
     msg_descriptor = unittest_pb2.TestAllTypes.DESCRIPTOR
     mask.AllFieldsFromDescriptor(msg_descriptor)
-    self.assertEqual(76, len(mask.paths))
+    self.assertEqual(79, len(mask.paths))
     self.assertTrue(mask.IsValidForDescriptor(msg_descriptor))
     for field in msg_descriptor.fields:
       self.assertTrue(field.name in mask.paths)

diff --git a/python/google/protobuf/internal/generator_test.py b/python/google/protobuf/internal/generator_test.py
@@ -344,8 +344,15 @@ def testOneof(self):
     self.assertEqual(0, desc.oneofs[0].index)
     self.assertIs(desc, desc.oneofs[0].containing_type)
     self.assertIs(desc.oneofs[0], desc.oneofs_by_name['oneof_field'])
-    nested_names = set(['oneof_uint32', 'oneof_nested_message',
-                        'oneof_string', 'oneof_bytes'])
+    nested_names = set([
+        'oneof_uint32',
+        'oneof_nested_message',
+        'oneof_string',
+        'oneof_bytes',
+        'oneof_cord',
+        'oneof_string_piece',
+        'oneof_lazy_nested_message',
+    ])
     self.assertEqual(
         nested_names,
         set([field.name for field in desc.oneofs[0].fields]))

diff --git a/src/google/protobuf/compiler/cpp/field_generators/cord_field.cc b/src/google/protobuf/compiler/cpp/field_generators/cord_field.cc
@@ -353,7 +353,8 @@ void CordOneofFieldGenerator::GenerateInlineAccessorDefinitions(
     }
   )cc");
   printer->Emit(R"cc(
-    inline const ::absl::Cord& $classname$::$name$() const {
+    inline const ::absl::Cord& $classname$::$name$() const
+        ABSL_ATTRIBUTE_LIFETIME_BOUND {
       $annotate_get$;
       // @@protoc_insertion_point(field_get:$full_name$)
       return _internal_$name$();

diff --git a/src/google/protobuf/generated_message_tctable_gen.cc b/src/google/protobuf/generated_message_tctable_gen.cc
@@ -14,6 +14,7 @@
 #include <utility>
 #include <vector>
 
+#include "absl/log/absl_check.h"
 #include "absl/strings/str_cat.h"
 #include "google/protobuf/descriptor.h"
 #include "google/protobuf/descriptor.pb.h"
@@ -731,6 +732,10 @@ TailCallTableInfo::TailCallTableInfo(
     const OptionProvider& option_provider,
     const std::vector<int>& has_bit_indices,
     const std::vector<int>& inlined_string_indices) {
+  ABSL_DCHECK(std::is_sorted(ordered_fields.begin(), ordered_fields.end(),
+                             [](const auto* lhs, const auto* rhs) {
+                               return lhs->number() < rhs->number();
+                             }));
   // If this message has any inlined string fields, store the donation state
   // offset in the first auxiliary entry, which is kInlinedStringAuxIdx.
   if (!inlined_string_indices.empty()) {

diff --git a/src/google/protobuf/test_util.inc b/src/google/protobuf/test_util.inc
@@ -295,7 +295,16 @@ inline void TestUtil::ModifyRepeatedFields(UNITTEST::TestAllTypes* message) {
 inline void TestUtil::SetOneofFields(UNITTEST::TestAllTypes* message) {
   message->set_oneof_uint32(601);
   message->mutable_oneof_nested_message()->set_bb(602);
+  message->mutable_oneof_lazy_nested_message()->set_bb(605);
   message->set_oneof_string("603");
+#ifndef PROTOBUF_TEST_NO_DESCRIPTORS
+  message->GetReflection()->SetString(
+      message, message->GetDescriptor()->FindFieldByName("oneof_cord"), "606");
+  message->GetReflection()->SetString(
+      message, message->GetDescriptor()->FindFieldByName("oneof_string_piece"),
+      "607");
+#endif  // !PROTOBUF_TEST_NO_DESCRIPTORS
+  // Must be last because tests are expecting it.
   message->set_oneof_bytes("604");
 }
 

diff --git a/src/google/protobuf/unittest.proto b/src/google/protobuf/unittest.proto
@@ -157,6 +157,9 @@ message TestAllTypes {
     NestedMessage oneof_nested_message = 112;
     string oneof_string = 113;
     bytes oneof_bytes = 114;
+    string oneof_cord = 115 [ctype=CORD];
+    string oneof_string_piece = 116 [ctype=STRING_PIECE];
+    NestedMessage oneof_lazy_nested_message = 117 [lazy=true];
   }
 }
 

diff --git a/src/google/protobuf/util/field_mask_util_test.cc b/src/google/protobuf/util/field_mask_util_test.cc
@@ -202,7 +202,7 @@ TEST(FieldMaskUtilTest, TestGetFieldMaskForAllFields) {
   EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("bb", mask));
 
   mask = FieldMaskUtil::GetFieldMaskForAllFields<TestAllTypes>();
-  EXPECT_EQ(76, mask.paths_size());
+  EXPECT_EQ(79, mask.paths_size());
   EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_int32", mask));
   EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_int64", mask));
   EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_uint32", mask));