From 02cf7aaa1d97bc5268639e56f735d2ec30e7c4ed Mon Sep 17 00:00:00 2001 From: Eric Salo Date: Sun, 18 Dec 2022 13:58:04 -0800 Subject: [PATCH] upb_Array_Resize() now correctly clears new values PiperOrigin-RevId: 496255949 --- BUILD | 10 ++++++ upb/collections/array.c | 33 ++++++++++-------- upb/collections/array.h | 2 +- upb/collections/array_internal.h | 5 +-- upb/collections/test.cc | 60 ++++++++++++++++++++++++++++++++ upb/wire/decode_fast.c | 2 +- 6 files changed, 94 insertions(+), 18 deletions(-) create mode 100644 upb/collections/test.cc diff --git a/BUILD b/BUILD index 262a877c3c..fba37786e9 100644 --- a/BUILD +++ b/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"], diff --git a/upb/collections/array.c b/upb/collections/array.c index adc3c4387a..a957f26ab8 100644 --- a/upb/collections/array.c +++ b/upb/collections/array.c @@ -79,8 +79,8 @@ 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); } @@ -88,7 +88,7 @@ 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,7 +101,7 @@ 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); @@ -109,7 +109,17 @@ void upb_Array_Delete(upb_Array* arr, size_t i, size_t 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); diff --git a/upb/collections/array.h b/upb/collections/array.h index 399f7f6d34..2e73e7ab6a 100644 --- a/upb/collections/array.h +++ b/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); diff --git a/upb/collections/array_internal.h b/upb/collections/array_internal.h index 51964e9b23..e1a4cefde2 100644 --- a/upb/collections/array_internal.h +++ b/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; diff --git a/upb/collections/test.cc b/upb/collections/test.cc new file mode 100644 index 0000000000..d82c7f8a2c --- /dev/null +++ b/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); +} diff --git a/upb/wire/decode_fast.c b/upb/wire/decode_fast.c index b0755c5749..690f364fdd 100644 --- a/upb/wire/decode_fast.c +++ b/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); \