Add bounds checking to DeleteSubrange, create new helper, RuntimeAssertInBoundsGE, and modify LogIndexOutOfBoundsAndAbort to customize the message being logged.

PiperOrigin-RevId: 853478344

Author: protobuf-github-bot

src/google/protobuf/repeated_field.cc

@@ -34,9 +34,25 @@ void LogIndexOutOfBounds(int index, int size) {
   ABSL_DLOG(FATAL) << "Index " << index << " out of bounds " << size;
 }
 
-void LogIndexOutOfBoundsAndAbort(int64_t index, int64_t size) {
-  ABSL_LOG(FATAL) << "Index (" << index
-                  << ") out of bounds of container with size (" << size << ")";
+void LogIndexOutOfBoundsAndAbort(int64_t index, int64_t size,
+                                 BoundsCheckMessageType type) {
+  switch (type) {
+    case BoundsCheckMessageType::kIndex:
+      ABSL_LOG(FATAL) << "Index (" << index
+                      << ") out of bounds of container with size (" << size
+                      << ")";
+      break;
+    case BoundsCheckMessageType::kGe:
+      ABSL_LOG(FATAL) << "Value (" << index
+                      << ") must be greater than or equal to limit (" << size
+                      << ")";
+      break;
+    case BoundsCheckMessageType::kLe:
+      ABSL_LOG(FATAL) << "Value (" << index
+                      << ") must be less than or equal to limit (" << size
+                      << ")";
+      break;
+  }
 }
 }  // namespace internal
 

src/google/protobuf/repeated_field.h

@@ -1122,16 +1122,7 @@ inline Element* RepeatedField<Element>::AddAlreadyReserved()
 template <typename Element>
 inline Element* RepeatedField<Element>::AddNAlreadyReserved(int n)
     ABSL_ATTRIBUTE_LIFETIME_BOUND {
-  if (ABSL_PREDICT_FALSE(n < 0)) {
-    // Calling with size 0 ensures that internal check (`n < 0 || n >= 0`)
-    // fails for any negative input.
-    internal::RuntimeAssertInBounds(n, 0);
-  }
-  // n = 0 will fail if it reaches RuntimeAssertInBoundsLE.
-  if (n == 0) {
-    return unsafe_elements(is_soo()) + size(is_soo());
-  }
-
+  internal::RuntimeAssertInBoundsGE(n, 0);
   const bool is_soo = this->is_soo();
   const int old_size = size(is_soo);
   [[maybe_unused]] const int capacity = Capacity(is_soo);

src/google/protobuf/repeated_ptr_field.h

@@ -104,6 +104,12 @@ struct InternalMetadataResolverOffsetHelper {
 PROTOBUF_EXPORT MessageLite* CloneSlow(Arena* arena, const MessageLite& value);
 PROTOBUF_EXPORT std::string* CloneSlow(Arena* arena, const std::string& value);
 
+enum class BoundsCheckMessageType {
+  kIndex,
+  kGe,
+  kLe,
+};
+
 // A utility function for logging that doesn't need any template types.
 PROTOBUF_EXPORT void LogIndexOutOfBounds(int index, int size);
 
@@ -113,7 +119,8 @@ PROTOBUF_EXPORT void LogIndexOutOfBounds(int index, int size);
 // TODO: Remove preserve_all and add no_return once experiment is
 // complete.
 PROTOBUF_PRESERVE_ALL PROTOBUF_EXPORT void LogIndexOutOfBoundsAndAbort(
-    int64_t index, int64_t size);
+    int64_t index, int64_t size,
+    BoundsCheckMessageType type = BoundsCheckMessageType::kIndex);
 PROTOBUF_EXPORT inline void RuntimeAssertInBounds(int index, int size) {
   if constexpr (GetBoundsCheckMode() == BoundsCheckMode::kAbort) {
     if (ABSL_PREDICT_FALSE(index < 0 || index >= size)) {
@@ -131,13 +138,25 @@ PROTOBUF_EXPORT inline void RuntimeAssertInBoundsLE(int64_t value,
                                                     int64_t limit) {
   if constexpr (GetBoundsCheckMode() == BoundsCheckMode::kAbort) {
     if (ABSL_PREDICT_FALSE(value > limit)) {
-      PROTOBUF_NO_MERGE LogIndexOutOfBoundsAndAbort(value, limit);
+      PROTOBUF_NO_MERGE LogIndexOutOfBoundsAndAbort(
+          value, limit, BoundsCheckMessageType::kLe);
     }
   }
 
   ABSL_DCHECK_LE(value, limit);
 }
 
+PROTOBUF_EXPORT inline void RuntimeAssertInBoundsGE(int64_t value,
+                                                    int64_t limit) {
+  if constexpr (GetBoundsCheckMode() == BoundsCheckMode::kAbort) {
+    if (ABSL_PREDICT_FALSE(value < limit)) {
+      PROTOBUF_NO_MERGE LogIndexOutOfBoundsAndAbort(
+          value, limit, BoundsCheckMessageType::kGe);
+    }
+  }
+  ABSL_DCHECK_GE(value, limit);
+}
+
 // Defined further below.
 template <typename Type>
 class GenericTypeHandler;
@@ -1764,9 +1783,9 @@ inline void RepeatedPtrField<Element>::RemoveLast() {
 
 template <typename Element>
 inline void RepeatedPtrField<Element>::DeleteSubrange(int start, int num) {
-  ABSL_DCHECK_GE(start, 0);
-  ABSL_DCHECK_GE(num, 0);
-  ABSL_DCHECK_LE(start + num, size());
+  internal::RuntimeAssertInBoundsGE(start, 0);
+  internal::RuntimeAssertInBoundsGE(num, 0);
+  internal::RuntimeAssertInBoundsLE(static_cast<int64_t>(start) + num, size());
   void** subrange = raw_mutable_data() + start;
   Arena* arena = GetArena();
   for (int i = 0; i < num; ++i) {