From 448e326200a69afd83ae86e44dcddb612f007763 Mon Sep 17 00:00:00 2001
From: Protobuf Team Bot <protobuf-github-bot@google.com>
Date: Tue, 14 May 2024 10:36:48 -0700
Subject: [PATCH] Use bool HasHasbits(const FieldDescriptor*) instead of manual
 checks.

It is not sufficient to check schema_.HasHasbits() followed by naively skipping
repeated and oneof fields as that can miss proto3 messages with message fields
and other scalar fields without "optional" keyword.

Use internal::cpp::HasHasbits(const FieldDescriptor*) instead.

PiperOrigin-RevId: 633633184
---
 .../protobuf/generated_message_reflection.cc  |  5 +-
 .../generated_message_reflection_unittest.cc  |  8 ++
 src/google/protobuf/unittest_proto3.proto     | 76 +++++++++++++++++++
 3 files changed, 86 insertions(+), 3 deletions(-)

diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc
index 4d7d7481057b..92045dabecb5 100644
--- a/src/google/protobuf/generated_message_reflection.cc
+++ b/src/google/protobuf/generated_message_reflection.cc
@@ -1265,10 +1265,9 @@ void Reflection::InternalSwap(Message* lhs, Message* rhs) const {
     int fields_with_has_bits = 0;
     for (int i = 0; i < descriptor_->field_count(); i++) {
       const FieldDescriptor* field = descriptor_->field(i);
-      if (field->is_repeated() || schema_.InRealOneof(field)) {
-        continue;
+      if (internal::cpp::HasHasbit(field)) {
+        ++fields_with_has_bits;
       }
-      fields_with_has_bits++;
     }
 
     int has_bits_size = (fields_with_has_bits + 31) / 32;
diff --git a/src/google/protobuf/generated_message_reflection_unittest.cc b/src/google/protobuf/generated_message_reflection_unittest.cc
index 94a7c15c3587..a432d74d6733 100644
--- a/src/google/protobuf/generated_message_reflection_unittest.cc
+++ b/src/google/protobuf/generated_message_reflection_unittest.cc
@@ -41,6 +41,7 @@
 #include "google/protobuf/unittest.pb.h"
 #include "google/protobuf/unittest_mset.pb.h"
 #include "google/protobuf/unittest_mset_wire_format.pb.h"
+#include "google/protobuf/unittest_proto3.pb.h"
 
 // Must be included last.
 #include "google/protobuf/port_def.inc"
@@ -1756,6 +1757,13 @@ TEST(GeneratedMessageReflection, ListFieldsSorted) {
                           Pointee(Property(&FieldDescriptor::number, 101))));
 }
 
+TEST(GeneratedMessageReflection, SwapImplicitPresenceShouldWork) {
+  proto3_unittest::TestHasbits lhs, rhs;
+  rhs.mutable_child()->set_optional_int32(-1);
+  lhs.GetReflection()->Swap(&lhs, &rhs);
+  EXPECT_EQ(lhs.child().optional_int32(), -1);
+}
+
 }  // namespace
 }  // namespace protobuf
 }  // namespace google
diff --git a/src/google/protobuf/unittest_proto3.proto b/src/google/protobuf/unittest_proto3.proto
index 9067728ae206..d88521afea5b 100644
--- a/src/google/protobuf/unittest_proto3.proto
+++ b/src/google/protobuf/unittest_proto3.proto
@@ -205,3 +205,79 @@ message TestOneof2 {
     BAZ = 3;
   }
 }
+
+// If bool fields are incorrectly assumed to have hasbits, InternalSwap would
+// result in swapping N more 32bit hasbits incorrectly. Considering padding, we
+// need many bool fields to stress this.
+message TestHasbits {
+  bool b1 = 1;
+  bool b2 = 2;
+  bool b3 = 3;
+  bool b4 = 4;
+  bool b5 = 5;
+  bool b6 = 6;
+  bool b7 = 7;
+  bool b8 = 8;
+  bool b9 = 9;
+  bool b10 = 10;
+  bool b11 = 11;
+  bool b12 = 12;
+  bool b13 = 13;
+  bool b14 = 14;
+  bool b15 = 15;
+  bool b16 = 16;
+  bool b17 = 17;
+  bool b18 = 18;
+  bool b19 = 19;
+  bool b20 = 20;
+  bool b21 = 21;
+  bool b22 = 22;
+  bool b23 = 23;
+  bool b24 = 24;
+  bool b25 = 25;
+  bool b26 = 26;
+  bool b27 = 27;
+  bool b28 = 28;
+  bool b29 = 29;
+  bool b30 = 30;
+  bool b31 = 31;
+  bool b32 = 32;
+  bool b33 = 33;
+  bool b34 = 34;
+  bool b35 = 35;
+  bool b36 = 36;
+  bool b37 = 37;
+  bool b38 = 38;
+  bool b39 = 39;
+  bool b40 = 40;
+  bool b41 = 41;
+  bool b42 = 42;
+  bool b43 = 43;
+  bool b44 = 44;
+  bool b45 = 45;
+  bool b46 = 46;
+  bool b47 = 47;
+  bool b48 = 48;
+  bool b49 = 49;
+  bool b50 = 50;
+  bool b51 = 51;
+  bool b52 = 52;
+  bool b53 = 53;
+  bool b54 = 54;
+  bool b55 = 55;
+  bool b56 = 56;
+  bool b57 = 57;
+  bool b58 = 58;
+  bool b59 = 59;
+  bool b60 = 60;
+  bool b61 = 61;
+  bool b62 = 62;
+  bool b63 = 63;
+  bool b64 = 64;
+  bool b65 = 65;
+  bool b66 = 66;
+  bool b67 = 67;
+  bool b68 = 68;
+  bool b69 = 69;
+  TestAllTypes child = 100;
+}