diff --git a/src/google/protobuf/repeated_field.h b/src/google/protobuf/repeated_field.h index 06b3353cc7..e5cd3af5c2 100644 --- a/src/google/protobuf/repeated_field.h +++ b/src/google/protobuf/repeated_field.h @@ -333,6 +333,16 @@ class RepeatedField final { // RepeatedField class to avoid costly cache misses due to the indirection. int current_size_; int total_size_; + + // Replaces current_size_ with new_size and returns the previous value of + // current_size_. This function is indended to be the only place where + // current_size_ is modified. + inline int ExchangeCurrentSize(int new_size) { + int prev_size = current_size_; + current_size_ = new_size; + return prev_size; + } + // Pad the Rep after arena allow for power-of-two byte sizes when // sizeof(Element) > sizeof(Arena*). eg for 16-byte objects. static PROTOBUF_CONSTEXPR const size_t kRepHeaderSize = @@ -457,11 +467,11 @@ class RepeatedField final { } FastAdderImpl(const FastAdderImpl&) = delete; FastAdderImpl& operator=(const FastAdderImpl&) = delete; - ~FastAdderImpl() { repeated_field_->current_size_ = index_; } + ~FastAdderImpl() { repeated_field_->ExchangeCurrentSize(index_); } void Add(Element val) { if (index_ == capacity_) { - repeated_field_->current_size_ = index_; + repeated_field_->ExchangeCurrentSize(index_); repeated_field_->Reserve(index_ + 1); capacity_ = repeated_field_->total_size_; buffer_ = repeated_field_->unsafe_elements(); @@ -611,13 +621,13 @@ inline int RepeatedField::Capacity() const { template inline void RepeatedField::AddAlreadyReserved(const Element& value) { GOOGLE_DCHECK_LT(current_size_, total_size_); - elements()[current_size_++] = value; + elements()[ExchangeCurrentSize(current_size_ + 1)] = value; } template inline Element* RepeatedField::AddAlreadyReserved() { GOOGLE_DCHECK_LT(current_size_, total_size_); - return &elements()[current_size_++]; + return &elements()[ExchangeCurrentSize(current_size_ + 1)]; } template @@ -628,9 +638,7 @@ inline Element* RepeatedField::AddNAlreadyReserved(int elements) { // total_size_ == 0. In this case the return pointer points to a zero size // array (n == 0). Hence we can just use unsafe_elements(), because the user // cannot dereference the pointer anyway. - Element* ret = unsafe_elements() + current_size_; - current_size_ += elements; - return ret; + return unsafe_elements() + ExchangeCurrentSize(current_size_ + elements); } template @@ -638,9 +646,11 @@ inline void RepeatedField::Resize(int new_size, const Element& value) { GOOGLE_DCHECK_GE(new_size, 0); if (new_size > current_size_) { Reserve(new_size); - std::fill(&elements()[current_size_], &elements()[new_size], value); + std::fill(&elements()[ExchangeCurrentSize(new_size)], &elements()[new_size], + value); + } else { + ExchangeCurrentSize(new_size); } - current_size_ = new_size; } template @@ -680,26 +690,21 @@ inline void RepeatedField::Set(int index, const Element& value) { template inline void RepeatedField::Add(const Element& value) { - uint32_t size = current_size_; - if (static_cast(size) == total_size_) { + if (current_size_ == total_size_) { // value could reference an element of the array. Reserving new space will // invalidate the reference. So we must make a copy first. auto tmp = value; Reserve(total_size_ + 1); - elements()[size] = std::move(tmp); + elements()[ExchangeCurrentSize(current_size_ + 1)] = std::move(tmp); } else { - elements()[size] = value; + elements()[ExchangeCurrentSize(current_size_ + 1)] = value; } - current_size_ = size + 1; } template inline Element* RepeatedField::Add() { - uint32_t size = current_size_; - if (static_cast(size) == total_size_) Reserve(total_size_ + 1); - auto ptr = &elements()[size]; - current_size_ = size + 1; - return ptr; + if (current_size_ == total_size_) Reserve(total_size_ + 1); + return &elements()[ExchangeCurrentSize(current_size_ + 1)]; } template @@ -711,15 +716,15 @@ inline void RepeatedField::Add(Iter begin, Iter end) { int additional = std::distance(begin, end); if (additional == 0) return; - Reserve(size() + additional); + int new_size = current_size_ + additional; + Reserve(new_size); // TODO(ckennelly): The compiler loses track of the buffer freshly // allocated by Reserve() by the time we call elements, so it cannot // guarantee that elements does not alias [begin(), end()). // // If restrict is available, annotating the pointer obtained from elements() // causes this to lower to memcpy instead of memmove. - std::copy(begin, end, elements() + size()); - current_size_ = size() + additional; + std::copy(begin, end, elements() + ExchangeCurrentSize(new_size)); } else { FastAdder fast_adder(this); for (; begin != end; ++begin) fast_adder.Add(*begin); @@ -729,7 +734,7 @@ inline void RepeatedField::Add(Iter begin, Iter end) { template inline void RepeatedField::RemoveLast() { GOOGLE_DCHECK_GT(current_size_, 0); - current_size_--; + ExchangeCurrentSize(current_size_ - 1); } template @@ -754,7 +759,7 @@ void RepeatedField::ExtractSubrange(int start, int num, template inline void RepeatedField::Clear() { - current_size_ = 0; + ExchangeCurrentSize(0); } template @@ -975,7 +980,7 @@ template inline void RepeatedField::Truncate(int new_size) { GOOGLE_DCHECK_LE(new_size, current_size_); if (current_size_ > 0) { - current_size_ = new_size; + ExchangeCurrentSize(new_size); } } diff --git a/src/google/protobuf/repeated_ptr_field.cc b/src/google/protobuf/repeated_ptr_field.cc index 8e86727144..c63643dc17 100644 --- a/src/google/protobuf/repeated_ptr_field.cc +++ b/src/google/protobuf/repeated_ptr_field.cc @@ -116,7 +116,7 @@ void* RepeatedPtrFieldBase::AddOutOfLineHelper(void* obj) { InternalExtend(1); // Equivalent to "Reserve(total_size_ + 1)" } ++rep_->allocated_size; - rep_->elements[current_size_++] = obj; + rep_->elements[ExchangeCurrentSize(current_size_ + 1)] = obj; return obj; } @@ -125,13 +125,14 @@ void RepeatedPtrFieldBase::CloseGap(int start, int num) { // Close up a gap of "num" elements starting at offset "start". for (int i = start + num; i < rep_->allocated_size; ++i) rep_->elements[i - num] = rep_->elements[i]; - current_size_ -= num; + ExchangeCurrentSize(current_size_ - num); rep_->allocated_size -= num; } MessageLite* RepeatedPtrFieldBase::AddWeak(const MessageLite* prototype) { if (rep_ != nullptr && current_size_ < rep_->allocated_size) { - return reinterpret_cast(rep_->elements[current_size_++]); + return reinterpret_cast( + rep_->elements[ExchangeCurrentSize(current_size_ + 1)]); } if (!rep_ || rep_->allocated_size == total_size_) { Reserve(total_size_ + 1); @@ -140,7 +141,7 @@ MessageLite* RepeatedPtrFieldBase::AddWeak(const MessageLite* prototype) { MessageLite* result = prototype ? prototype->New(arena_) : Arena::CreateMessage(arena_); - rep_->elements[current_size_++] = result; + rep_->elements[ExchangeCurrentSize(current_size_ + 1)] = result; return result; } diff --git a/src/google/protobuf/repeated_ptr_field.h b/src/google/protobuf/repeated_ptr_field.h index 2fca2d6206..b68845a90c 100644 --- a/src/google/protobuf/repeated_ptr_field.h +++ b/src/google/protobuf/repeated_ptr_field.h @@ -210,7 +210,8 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { typename TypeHandler::Type* Add( const typename TypeHandler::Type* prototype = nullptr) { if (rep_ != nullptr && current_size_ < rep_->allocated_size) { - return cast(rep_->elements[current_size_++]); + return cast( + rep_->elements[ExchangeCurrentSize(current_size_ + 1)]); } typename TypeHandler::Type* result = TypeHandler::NewFromPrototype(prototype, arena_); @@ -223,7 +224,9 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { typename std::enable_if::type* = nullptr> inline void Add(typename TypeHandler::Type&& value) { if (rep_ != nullptr && current_size_ < rep_->allocated_size) { - *cast(rep_->elements[current_size_++]) = std::move(value); + *cast( + rep_->elements[ExchangeCurrentSize(current_size_ + 1)]) = + std::move(value); return; } if (!rep_ || rep_->allocated_size == total_size_) { @@ -232,7 +235,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { ++rep_->allocated_size; typename TypeHandler::Type* result = TypeHandler::New(arena_, std::move(value)); - rep_->elements[current_size_++] = result; + rep_->elements[ExchangeCurrentSize(current_size_ + 1)] = result; } template @@ -288,7 +291,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { do { TypeHandler::Clear(cast(elements[i++])); } while (i < n); - current_size_ = 0; + ExchangeCurrentSize(0); } } @@ -321,7 +324,8 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template void RemoveLast() { GOOGLE_DCHECK_GT(current_size_, 0); - TypeHandler::Clear(cast(rep_->elements[--current_size_])); + ExchangeCurrentSize(current_size_ - 1); + TypeHandler::Clear(cast(rep_->elements[current_size_])); } template @@ -403,7 +407,8 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template typename TypeHandler::Type* AddFromCleared() { if (rep_ != nullptr && current_size_ < rep_->allocated_size) { - return cast(rep_->elements[current_size_++]); + return cast( + rep_->elements[ExchangeCurrentSize(current_size_ + 1)]); } else { return nullptr; } @@ -439,7 +444,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { ++rep_->allocated_size; } - rep_->elements[current_size_++] = value; + rep_->elements[ExchangeCurrentSize(current_size_ + 1)] = value; } template @@ -454,8 +459,9 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { template typename TypeHandler::Type* UnsafeArenaReleaseLast() { GOOGLE_DCHECK_GT(current_size_, 0); + ExchangeCurrentSize(current_size_ - 1); typename TypeHandler::Type* result = - cast(rep_->elements[--current_size_]); + cast(rep_->elements[current_size_]); --rep_->allocated_size; if (current_size_ < rep_->allocated_size) { // There are cleared elements on the end; replace the removed element @@ -508,8 +514,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { // allocated list. elems[rep_->allocated_size] = elems[current_size_]; } - elems[current_size_] = value; - current_size_ = current_size_ + 1; + elems[ExchangeCurrentSize(current_size_ + 1)] = value; rep_->allocated_size = rep_->allocated_size + 1; } else { AddAllocatedSlowWithCopy(value, element_arena, arena); @@ -531,8 +536,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { // allocated list. elems[rep_->allocated_size] = elems[current_size_]; } - elems[current_size_] = value; - current_size_ = current_size_ + 1; + elems[ExchangeCurrentSize(current_size_ + 1)] = value; ++rep_->allocated_size; } else { UnsafeArenaAddAllocated(value); @@ -644,6 +648,16 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { Arena* arena_; int current_size_; int total_size_; + + // Replaces current_size_ with new_size and returns the previous value of + // current_size_. This function is indended to be the only place where + // current_size_ is modified. + inline int ExchangeCurrentSize(int new_size) { + int prev_size = current_size_; + current_size_ = new_size; + return prev_size; + } + struct Rep { int allocated_size; // Here we declare a huge array as a way of approximating C's "flexible @@ -676,7 +690,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase { int allocated_elems = rep_->allocated_size - current_size_; (this->*inner_loop)(new_elements, other_elements, other_size, allocated_elems); - current_size_ += other_size; + ExchangeCurrentSize(current_size_ + other_size); if (rep_->allocated_size < current_size_) { rep_->allocated_size = current_size_; } diff --git a/src/google/protobuf/util/json_util_test.cc b/src/google/protobuf/util/json_util_test.cc index d4e1b1eb9f..698fa05883 100644 --- a/src/google/protobuf/util/json_util_test.cc +++ b/src/google/protobuf/util/json_util_test.cc @@ -848,6 +848,18 @@ TEST_F(TypeResolverTest, ParseWrappers) { R"({"boolValue":true,"int32Value":42,"stringValue":"ieieo"})")); } +TEST_F(TypeResolverTest, RejectDuplicateOneof) { + StringPiece input = R"json( + { + "oneofInt32Value": 42, + "oneofStringValue": "bad", + } + )json"; + + EXPECT_THAT(Json2Proto(input, "proto3.TestOneof"), + StatusIs(util::StatusCode::kInvalidArgument)); +} + TEST_F(TypeResolverTest, TestWrongJsonInput) { EXPECT_THAT(Json2Proto(R"json({"unknown_field": "some_value"})json", "proto3.TestMessage"),