Removed extra size from message.

upb/decode.c

@@ -325,7 +325,7 @@ static const char *decode_tosubmsg2(upb_decstate *d, const char *ptr,
                                     upb_msg *submsg, const upb_msglayout *subl,
                                     int size) {
   int saved_delta = decode_pushlimit(d, ptr, size);
-  if (--d->depth < 0) return decode_err(d, UPB_DECODE_MALFORMED);
+  if (--d->depth < 0) return decode_err(d, UPB_DECODE_MAXDEPTH_EXCEEDED);
   if (!decode_isdone(d, &ptr)) {
     ptr = decode_msg(d, ptr, submsg, subl);
   }
@@ -348,7 +348,7 @@ UPB_FORCEINLINE
 static const char *decode_group(upb_decstate *d, const char *ptr,
                                 upb_msg *submsg, const upb_msglayout *subl,
                                 uint32_t number) {
-  if (--d->depth < 0) return decode_err(d, UPB_DECODE_MALFORMED);
+  if (--d->depth < 0) return decode_err(d, UPB_DECODE_MAXDEPTH_EXCEEDED);
   if (decode_isdone(d, &ptr)) {
     return decode_err(d, UPB_DECODE_MALFORMED);
   }
@@ -667,14 +667,12 @@ static const char *decode_tomsg(upb_decstate *d, const char *ptr, upb_msg *msg,
 UPB_FORCEINLINE
 static bool decode_checkrequired(upb_decstate *d, const upb_msg *msg,
                                  const upb_msglayout *l) {
-  if (UPB_LIKELY(!l->required_mask)) return false;
-  // if (UPB_UNLIKELY(l->required_mask & 1)) {
-  //   return decode_checkrequired_slow(d, msg, l);
-  // }
+  if (UPB_LIKELY(l->required_count == 0)) return true;
+  uint64_t required_mask = ((1 << l->required_count) - 1) << 1;
   uint64_t msg_head;
   memcpy(&msg_head, msg, 8);
   msg_head = _upb_be_swap64(msg_head);
-  return l->required_mask & ~msg_head;
+  return (required_mask & ~msg_head) == 0;
 }
 
 UPB_FORCEINLINE
@@ -870,21 +868,22 @@ static const char *decode_known(upb_decstate *d, const char *ptr, upb_msg *msg,
 UPB_FORCEINLINE
 static const char *decode_unknown(upb_decstate *d, const char *ptr,
                                   upb_msg *msg, int field_number, int wire_type,
-                                  wireval val) {
+                                  wireval val, const char **field_start) {
   if (field_number == 0) return decode_err(d, UPB_DECODE_MALFORMED);
 
   if (wire_type == UPB_WIRE_TYPE_DELIMITED) ptr += val.size;
   if (msg) {
     if (wire_type == UPB_WIRE_TYPE_START_GROUP) {
-      const char *preserved_unknown = d->unknown;
+      d->unknown = *field_start;
       d->unknown_msg = msg;
       ptr = decode_group(d, ptr, NULL, NULL, field_number);
       d->unknown_msg = NULL;
-      d->unknown = preserved_unknown;
+      *field_start = d->unknown;
       // XXX: pointer could have flipped during decoding the group,
       // ptr - d->unknown below will be borked.
     }
-    if (!_upb_msg_addunknown(msg, d->unknown, ptr - d->unknown, &d->arena)) {
+    if (!_upb_msg_addunknown(msg, *field_start, ptr - *field_start,
+                             &d->arena)) {
       return decode_err(d, UPB_DECODE_OOM);
     }
   } else if (wire_type == UPB_WIRE_TYPE_START_GROUP) {
@@ -898,6 +897,7 @@ static const char *decode_msg(upb_decstate *d, const char *ptr, upb_msg *msg,
                               const upb_msglayout *layout) {
   int last_field_index = 0;
   while (true) {
+    const char *field_start = ptr;
     uint32_t tag;
     const upb_msglayout_field *field;
     int field_number;
@@ -906,7 +906,6 @@ static const char *decode_msg(upb_decstate *d, const char *ptr, upb_msg *msg,
     int op;
 
     UPB_ASSERT(ptr < d->limit_ptr);
-    d->unknown = ptr;
     ptr = decode_tag(d, ptr, &tag);
     field_number = tag >> 3;
     wire_type = tag & 7;
@@ -924,7 +923,8 @@ static const char *decode_msg(upb_decstate *d, const char *ptr, upb_msg *msg,
     } else {
       switch (op) {
         case OP_UNKNOWN:
-          ptr = decode_unknown(d, ptr, msg, field_number, wire_type, val);
+          ptr = decode_unknown(d, ptr, msg, field_number, wire_type, val,
+                               &field_start);
           break;
         case OP_MSGSET_ITEM:
           ptr = decode_msgset(d, ptr, msg, layout);
@@ -938,15 +938,16 @@ static const char *decode_msg(upb_decstate *d, const char *ptr, upb_msg *msg,
       }
     }
 
-    if (decode_isdone(d, &ptr) ||
-        decode_tryfastdispatch(d, &ptr, msg, layout)) {
-      if (UPB_UNLIKELY(d->options & UPB_CHECK_REQUIRED) &&
-          decode_checkrequired(d, msg, layout)) {
-        d->missing_required = true;
-      }
-      return ptr;
-    }
+    if (decode_isdone(d, &ptr)) break;
+    if (decode_tryfastdispatch(d, &ptr, msg, layout)) break;
   }
+
+  if (false && UPB_UNLIKELY(d->options & UPB_CHECK_REQUIRED) &&
+      !decode_checkrequired(d, msg, layout)) {
+    d->missing_required = true;
+  }
+
+  return ptr;
 }
 
 const char *fastdecode_generic(struct upb_decstate *d, const char *ptr,

upb/msg_internal.h

@@ -169,7 +169,6 @@ typedef enum {
 struct upb_msglayout {
   const upb_msglayout_sub *subs;
   const upb_msglayout_field *fields;
-  uint64_t required_mask;
   /* Must be aligned to sizeof(void*).  Doesn't include internal members like
    * unknown fields, extension dict, pointer to msglayout, etc. */
   uint16_t size;
@@ -177,8 +176,10 @@ struct upb_msglayout {
   uint8_t ext;  // upb_msgext_mode, declared as uint8_t so sizeof(ext) == 1
   uint8_t dense_below;
   uint8_t table_mask;
-  /* To constant-initialize the tables of variable length, we need a flexible
-   * array member, and we need to compile in C99 mode. */
+  uint8_t required_count;  // Required fields have the lowest hasbits.
+  /* To statically initialize the tables of variable length, we need a flexible
+   * array member, and we need to compile in gnu99 mode (constant initialization
+   * of flexible array members is a GNU extension, not in C99 unfortunately. */
   _upb_fasttable_entry fasttable[];
 };
 

upbc/message_layout.cc

@@ -167,7 +167,7 @@ void MessageLayout::PlaceNonOneofFields(
 
   // Place/count hasbits.
   hasbit_count_ = 0;
-  required_mask_ = 0;
+  required_count_ = 0;
   for (auto field : FieldHotnessOrder(descriptor)) {
     if (HasHasbit(field)) {
       // We don't use hasbit 0, so that 0 can indicate "no presence" in the
@@ -175,13 +175,15 @@ void MessageLayout::PlaceNonOneofFields(
       int index = ++hasbit_count_;
       hasbit_indexes_[field] = index;
       if (field->is_required()) {
-        if (index > 63) {
-          std::cerr << "upb does not support messages with more than 64 "
+        if (index >= 63) {
+          // This could be fixed in the decoder without too much trouble.  But
+          // we expect this to be so rare that we don't worry about it for now.
+          std::cerr << "upb does not support messages with more than 63 "
                        "required fields: "
                     << field->full_name() << "\n";
           exit(1);
         }
-        required_mask_ |= 1 << index;
+        required_count_++;
       }
     }
   }

upbc/message_layout.h

@@ -87,7 +87,9 @@ class MessageLayout {
 
   int hasbit_count() const { return hasbit_count_; }
   int hasbit_bytes() const { return hasbit_bytes_; }
-  uint64_t required_mask() const { return required_mask_; }
+
+  // Required fields always have the lowest hasbits.
+  int required_count() const { return required_count_; }
 
   static bool HasHasbit(const google::protobuf::FieldDescriptor* field);
   static SizeAndAlign SizeOfUnwrapped(
@@ -132,7 +134,7 @@ class MessageLayout {
   Size size_;
   int hasbit_count_;
   int hasbit_bytes_;
-  uint64_t required_mask_;
+  int required_count_;
 };
 
 // Returns fields in order of "hotness", eg. how frequently they appear in

upbc/protoc-gen-upb.cc

@@ -1253,12 +1253,12 @@ void WriteMessage(const protobuf::Descriptor* message, Output& output,
   output("  $0,\n", submsgs_array_ref);
   output("  $0,\n", fields_array_ref);
   output("  $0, $1, $2, $3, $4, $5,\n",
-         layout.required_mask(),
          GetSizeInit(layout.message_size()),
          field_number_order.size(),
          msgext,
          dense_below,
-         table_mask
+         table_mask,
+         layout.required_count()
   );
   if (!table.empty()) {
     output("  UPB_FASTTABLE_INIT({\n");