Merge tag 'refs/tags/sync-piper' into sync-stage

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<Element>::Capacity() const {
 template <typename Element>
 inline void RepeatedField<Element>::AddAlreadyReserved(const Element& value) {
   GOOGLE_DCHECK_LT(current_size_, total_size_);
-  elements()[current_size_++] = value;
+  elements()[ExchangeCurrentSize(current_size_ + 1)] = value;
 }
 
 template <typename Element>
 inline Element* RepeatedField<Element>::AddAlreadyReserved() {
   GOOGLE_DCHECK_LT(current_size_, total_size_);
-  return &elements()[current_size_++];
+  return &elements()[ExchangeCurrentSize(current_size_ + 1)];
 }
 
 template <typename Element>
@@ -628,9 +638,7 @@ inline Element* RepeatedField<Element>::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 <typename Element>
@@ -638,9 +646,11 @@ inline void RepeatedField<Element>::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 <typename Element>
@@ -680,26 +690,21 @@ inline void RepeatedField<Element>::Set(int index, const Element& value) {
 
 template <typename Element>
 inline void RepeatedField<Element>::Add(const Element& value) {
-  uint32_t size = current_size_;
-  if (static_cast<int>(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 <typename Element>
 inline Element* RepeatedField<Element>::Add() {
-  uint32_t size = current_size_;
-  if (static_cast<int>(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 <typename Element>
@@ -711,15 +716,15 @@ inline void RepeatedField<Element>::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<Element>::Add(Iter begin, Iter end) {
 template <typename Element>
 inline void RepeatedField<Element>::RemoveLast() {
   GOOGLE_DCHECK_GT(current_size_, 0);
-  current_size_--;
+  ExchangeCurrentSize(current_size_ - 1);
 }
 
 template <typename Element>
@@ -754,7 +759,7 @@ void RepeatedField<Element>::ExtractSubrange(int start, int num,
 
 template <typename Element>
 inline void RepeatedField<Element>::Clear() {
-  current_size_ = 0;
+  ExchangeCurrentSize(0);
 }
 
 template <typename Element>
@@ -975,7 +980,7 @@ template <typename Element>
 inline void RepeatedField<Element>::Truncate(int new_size) {
   GOOGLE_DCHECK_LE(new_size, current_size_);
   if (current_size_ > 0) {
-    current_size_ = new_size;
+    ExchangeCurrentSize(new_size);
   }
 }
 

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<MessageLite*>(rep_->elements[current_size_++]);
+    return reinterpret_cast<MessageLite*>(
+        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<ImplicitWeakMessage>(arena_);
-  rep_->elements[current_size_++] = result;
+  rep_->elements[ExchangeCurrentSize(current_size_ + 1)] = result;
   return result;
 }
 

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<TypeHandler>(rep_->elements[current_size_++]);
+      return cast<TypeHandler>(
+          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<TypeHandler::Movable::value>::type* = nullptr>
   inline void Add(typename TypeHandler::Type&& value) {
     if (rep_ != nullptr && current_size_ < rep_->allocated_size) {
-      *cast<TypeHandler>(rep_->elements[current_size_++]) = std::move(value);
+      *cast<TypeHandler>(
+          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 <typename TypeHandler>
@@ -288,7 +291,7 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
       do {
         TypeHandler::Clear(cast<TypeHandler>(elements[i++]));
       } while (i < n);
-      current_size_ = 0;
+      ExchangeCurrentSize(0);
     }
   }
 
@@ -321,7 +324,8 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
   template <typename TypeHandler>
   void RemoveLast() {
     GOOGLE_DCHECK_GT(current_size_, 0);
-    TypeHandler::Clear(cast<TypeHandler>(rep_->elements[--current_size_]));
+    ExchangeCurrentSize(current_size_ - 1);
+    TypeHandler::Clear(cast<TypeHandler>(rep_->elements[current_size_]));
   }
 
   template <typename TypeHandler>
@@ -403,7 +407,8 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
   template <typename TypeHandler>
   typename TypeHandler::Type* AddFromCleared() {
     if (rep_ != nullptr && current_size_ < rep_->allocated_size) {
-      return cast<TypeHandler>(rep_->elements[current_size_++]);
+      return cast<TypeHandler>(
+          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 <typename TypeHandler>
@@ -454,8 +459,9 @@ class PROTOBUF_EXPORT RepeatedPtrFieldBase {
   template <typename TypeHandler>
   typename TypeHandler::Type* UnsafeArenaReleaseLast() {
     GOOGLE_DCHECK_GT(current_size_, 0);
+    ExchangeCurrentSize(current_size_ - 1);
     typename TypeHandler::Type* result =
-        cast<TypeHandler>(rep_->elements[--current_size_]);
+        cast<TypeHandler>(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<TypeHandler>(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<TypeHandler>(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_;
     }

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"),