upb_Array_Resize() now correctly clears new values

PiperOrigin-RevId: 496255949

Author: ericsalo

BUILD

@@ -681,6 +681,16 @@ cc_test(
     ],
 )
 
+cc_test(
+    name = "collections_test",
+    srcs = ["upb/collections/test.cc"],
+    deps = [
+        ":collections",
+        ":upb",
+        "@com_google_googletest//:gtest_main",
+    ],
+)
+
 cc_test(
     name = "message_test",
     srcs = ["upb/message/test.cc"],

upb/collections/array.c

@@ -79,16 +79,16 @@ bool upb_Array_Append(upb_Array* arr, upb_MessageValue val, upb_Arena* arena) {
 
 void upb_Array_Move(upb_Array* arr, size_t dst_idx, size_t src_idx,
                     size_t count) {
+  const int lg2 = arr->data & 7;
   char* data = _upb_array_ptr(arr);
-  int lg2 = arr->data & 7;
   memmove(&data[dst_idx << lg2], &data[src_idx << lg2], count << lg2);
 }
 
 bool upb_Array_Insert(upb_Array* arr, size_t i, size_t count,
                       upb_Arena* arena) {
   UPB_ASSERT(i <= arr->size);
   UPB_ASSERT(count + arr->size >= count);
-  size_t oldsize = arr->size;
+  const size_t oldsize = arr->size;
   if (!upb_Array_Resize(arr, arr->size + count, arena)) {
     return false;
   }
@@ -101,15 +101,25 @@ bool upb_Array_Insert(upb_Array* arr, size_t i, size_t count,
  * |------------|XXXXXXXX|--------|
  */
 void upb_Array_Delete(upb_Array* arr, size_t i, size_t count) {
-  size_t end = i + count;
+  const size_t end = i + count;
   UPB_ASSERT(i <= end);
   UPB_ASSERT(end <= arr->size);
   upb_Array_Move(arr, i, end, arr->size - end);
   arr->size -= count;
 }
 
 bool upb_Array_Resize(upb_Array* arr, size_t size, upb_Arena* arena) {
-  return _upb_Array_Resize(arr, size, arena);
+  const size_t oldsize = arr->size;
+  if (UPB_UNLIKELY(!_upb_Array_ResizeUninitialized(arr, size, arena))) {
+    return false;
+  }
+  const size_t newsize = arr->size;
+  if (newsize > oldsize) {
+    const int lg2 = arr->data & 7;
+    char* data = _upb_array_ptr(arr);
+    memset(data + (oldsize << lg2), 0, (newsize - oldsize) << lg2);
+  }
+  return true;
 }
 
 // EVERYTHING BELOW THIS LINE IS INTERNAL - DO NOT USE /////////////////////////
@@ -126,10 +136,7 @@ bool _upb_array_realloc(upb_Array* arr, size_t min_capacity, upb_Arena* arena) {
 
   new_bytes = new_capacity << elem_size_lg2;
   ptr = upb_Arena_Realloc(arena, ptr, old_bytes, new_bytes);
-
-  if (!ptr) {
-    return false;
-  }
+  if (!ptr) return false;
 
   arr->data = _upb_tag_arrptr(ptr, elem_size_lg2);
   arr->capacity = new_capacity;
@@ -150,8 +157,9 @@ static upb_Array* getorcreate_array(upb_Array** arr_ptr, int elem_size_lg2,
 void* _upb_Array_Resize_fallback(upb_Array** arr_ptr, size_t size,
                                  int elem_size_lg2, upb_Arena* arena) {
   upb_Array* arr = getorcreate_array(arr_ptr, elem_size_lg2, arena);
-  return arr && _upb_Array_Resize(arr, size, arena) ? _upb_array_ptr(arr)
-                                                    : NULL;
+  return arr && _upb_Array_ResizeUninitialized(arr, size, arena)
+             ? _upb_array_ptr(arr)
+             : NULL;
 }
 
 bool _upb_Array_Append_fallback(upb_Array** arr_ptr, const void* value,
@@ -160,10 +168,7 @@ bool _upb_Array_Append_fallback(upb_Array** arr_ptr, const void* value,
   if (!arr) return false;
 
   size_t elems = arr->size;
-
-  if (!_upb_Array_Resize(arr, elems + 1, arena)) {
-    return false;
-  }
+  if (!_upb_Array_ResizeUninitialized(arr, elems + 1, arena)) return false;
 
   char* data = _upb_array_ptr(arr);
   memcpy(data + (elems << elem_size_lg2), value, 1 << elem_size_lg2);

upb/collections/array.h

@@ -72,7 +72,7 @@ UPB_API bool upb_Array_Insert(upb_Array* array, size_t i, size_t count,
 // REQUIRES: `i + count <= upb_Array_Size(arr)`
 UPB_API void upb_Array_Delete(upb_Array* array, size_t i, size_t count);
 
-// Changes the size of a vector. New elements are initialized to empty/0.
+// Changes the size of a vector. New elements are initialized to NULL/0.
 // Returns false on allocation failure.
 UPB_API bool upb_Array_Resize(upb_Array* array, size_t size, upb_Arena* arena);
 

upb/collections/array_internal.h

@@ -95,8 +95,9 @@ UPB_INLINE bool _upb_array_reserve(upb_Array* arr, size_t size,
   return true;
 }
 
-UPB_INLINE bool _upb_Array_Resize(upb_Array* arr, size_t size,
-                                  upb_Arena* arena) {
+// Resize without initializing new elements.
+UPB_INLINE bool _upb_Array_ResizeUninitialized(upb_Array* arr, size_t size,
+                                               upb_Arena* arena) {
   if (!_upb_array_reserve(arr, size, arena)) return false;
   arr->size = size;
   return true;

upb/collections/test.cc

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2009-2021, Google LLC
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Google LLC nor the
+ *       names of its contributors may be used to endorse or promote products
+ *       derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL Google LLC BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "gtest/gtest.h"
+#include "upb/base/descriptor_constants.h"
+#include "upb/collections/array.h"
+#include "upb/upb.hpp"
+
+TEST(CollectionsTest, Arrays) {
+  upb::Arena arena;
+  upb::Status status;
+
+  upb_Array* array = upb_Array_New(arena.ptr(), kUpb_CType_Int32);
+  EXPECT_TRUE(array);
+
+  for (int i = 0; i < 10; i++) {
+    upb_MessageValue mv = {.int32_val = 3 * i};
+    upb_Array_Append(array, mv, arena.ptr());
+    EXPECT_EQ(upb_Array_Size(array), i + 1);
+    EXPECT_EQ(upb_Array_Get(array, i).int32_val, 3 * i);
+  }
+
+  upb_Array_Resize(array, 12, arena.ptr());
+  EXPECT_EQ(upb_Array_Get(array, 10).int32_val, 0);
+  EXPECT_EQ(upb_Array_Get(array, 11).int32_val, 0);
+
+  upb_Array_Resize(array, 4, arena.ptr());
+  EXPECT_EQ(upb_Array_Size(array), 4);
+
+  upb_Array_Resize(array, 6, arena.ptr());
+  EXPECT_EQ(upb_Array_Size(array), 6);
+
+  EXPECT_EQ(upb_Array_Get(array, 3).int32_val, 9);
+  EXPECT_EQ(upb_Array_Get(array, 4).int32_val, 0);
+  EXPECT_EQ(upb_Array_Get(array, 5).int32_val, 0);
+}

upb/wire/decode_fast.c

@@ -563,7 +563,7 @@ TAGBYTES(p)
       _upb_FastDecoder_ErrorJmp(d, kUpb_DecodeStatus_Malformed);            \
     }                                                                       \
   } else {                                                                  \
-    _upb_Array_Resize(arr, elems, &d->arena);                               \
+    _upb_Array_ResizeUninitialized(arr, elems, &d->arena);                  \
   }                                                                         \
                                                                             \
   char* dst = _upb_array_ptr(arr);                                          \