Fixes for groups and 32-bit varints.

4 files changed, 20 insertions, 11 deletions

diff --git a/upb_context.c b/upb_context.c
index d83906c..d857d16 100644
--- a/upb_context.c
+++ b/upb_context.c
@@ -241,7 +241,8 @@ bool addfd(struct upb_strtable *addto, struct upb_strtable *existingdefs,
         struct upb_msg_field *f = &m->fields[i];
         google_protobuf_FieldDescriptorProto *fd = m->field_descriptors[i];
         union upb_symbol_ref ref;
-        if(fd->type == GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_MESSAGE)
+        if(fd->type == GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_MESSAGE ||
+           fd->type == GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_GROUP)
           ref = resolve2(existingdefs, addto, &e->e.key, fd->type_name,
                          UPB_SYM_MESSAGE);
         else if(fd->type == GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_ENUM)
diff --git a/upb_msg.c b/upb_msg.c
index c9fa260..bac7795 100644
--- a/upb_msg.c
+++ b/upb_msg.c
@@ -10,7 +10,7 @@
 #include "upb_parse.h"
 
 /* Rounds p up to the next multiple of t. */
-#define ALIGN_UP(p, t) (p % t == 0 ? p : p + (t - (p % t)))
+#define ALIGN_UP(p, t) ((p) % (t) == 0 ? (p) : (p) + ((t) - ((p) % (t))))
 
 static int div_round_up(int numerator, int denominator) {
   /* cf. http://stackoverflow.com/questions/17944/how-to-round-up-the-result-of-integer-division */
@@ -174,6 +174,7 @@ static void free_value(union upb_value_ptr p, struct upb_msg_field *f,
       break;
     }
     case GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_MESSAGE:
+    case GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_GROUP:
       if(free_submsgs) upb_msgdata_free(*p.message, f->ref.msg, free_submsgs);
       break;
     default: break;  /* For non-dynamic types, do nothing. */
@@ -322,6 +323,7 @@ static void submsg_start_cb(struct upb_parse_state *_s, void *user_field_desc)
   // TODO: find a non-hacky way to get a pointer to the old frame.
   struct parse_frame_data *oldframe = (void*)((char*)s->s.top - s->s.udata_size);
   union upb_value_ptr p = get_value_ptr(oldframe->data, f);
+  assert(f->ref.msg);
   upb_msg_reuse_submsg(p.message, f->ref.msg);
   set_frame_data(&s->s, f->ref.msg, *p.message);
   if(!s->merge) upb_msg_clear(frame->data, f->ref.msg);
@@ -367,4 +369,3 @@ void *upb_alloc_and_parse(struct upb_msg *m, struct upb_string *str, bool byref)
     return NULL;
   }
 }
-
diff --git a/upb_parse.c b/upb_parse.c
index b29ee2e..3195e71 100644
--- a/upb_parse.c
+++ b/upb_parse.c
@@ -60,15 +60,21 @@ static upb_status_t skip_v_uint64_t(void **buf, void *end)
 static upb_status_t get_v_uint32_t(void *restrict *buf, void *end,
                                    uint32_t *restrict val)
 {
-  uint8_t *b = *buf;
+  uint8_t *b = *buf, *dend;
   upb_status_t bound_error;
-  end = check_end(b, end, 5, &bound_error);  /* 2**32 is a 5-byte varint. */
+  dend = check_end(b, end, 5, &bound_error);  /* 2**32 is a 5-byte varint. */
+  end = check_end(b, end, 10, &bound_error);  /* May have to discard bytes. */
   uint8_t last = 0x80;
   *val = 0;
-  for(int bitpos = 0; b < (uint8_t*)end && (last & 0x80); b++, bitpos += 7)
+  for(int bitpos = 0; b < dend && (last & 0x80); b++, bitpos += 7)
     *val |= ((uint32_t)((last = *b) & 0x7F)) << bitpos;
 
-  if(unlikely(last & 0x80)) return bound_error;
+  /* Discard high bytes until varint ends. */
+  for(; b < (uint8_t*)end && (last & 0x80); b++)
+    last = *b;
+
+  if(unlikely(last & 0x80))
+    return bound_error;
   *buf = b;
   return UPB_STATUS_OK;
 }
@@ -176,6 +182,7 @@ struct upb_type_info upb_type_info[] = {
   [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_FIXED32] = {alignof(uint32_t), sizeof(uint32_t), UPB_WIRE_TYPE_32BIT},
   [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_BOOL]    = {alignof(bool),     sizeof(bool),     UPB_WIRE_TYPE_VARINT},
   [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_MESSAGE] = {alignof(void*),    sizeof(void*),    UPB_WIRE_TYPE_DELIMITED},
+  [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_GROUP]   = {alignof(void*),    sizeof(void*),    UPB_WIRE_TYPE_START_GROUP},
   [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_UINT32]  = {alignof(uint32_t), sizeof(uint32_t), UPB_WIRE_TYPE_VARINT},
   [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_ENUM]    = {alignof(uint32_t), sizeof(uint32_t), UPB_WIRE_TYPE_VARINT},
   [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_SFIXED32]= {alignof(int32_t),  sizeof(int32_t),  UPB_WIRE_TYPE_32BIT},
@@ -184,7 +191,6 @@ struct upb_type_info upb_type_info[] = {
   [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_SINT64]  = {alignof(int64_t),  sizeof(int64_t),  UPB_WIRE_TYPE_VARINT},
   [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_STRING]  = {alignof(struct upb_string*), sizeof(struct upb_string*), UPB_WIRE_TYPE_DELIMITED},
   [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_BYTES]   = {alignof(struct upb_string*), sizeof(struct upb_string*), UPB_WIRE_TYPE_DELIMITED},
-  [GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_GROUP]   = {0,0,0},
 };
 
 static upb_status_t parse_tag(void **buf, void *end, struct upb_tag *tag)
@@ -335,7 +341,7 @@ static upb_status_t parse_nondelimited(struct upb_parse_state *s,
     UPB_CHECK(skip_wire_value(buf, end, tag->wire_type));
   } else if(ft == GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_GROUP) {
     /* No length specified, an "end group" tag will mark the end. */
-    UPB_CHECK(push_stack_frame(s, 0, user_field_desc));
+    UPB_CHECK(push_stack_frame(s, UINT32_MAX, user_field_desc));
   } else {
     UPB_CHECK(s->value_cb(s, buf, end, user_field_desc));
   }
@@ -357,7 +363,8 @@ upb_status_t upb_parse(struct upb_parse_state *s, void *buf, size_t len,
     void *bufstart = buf;
     UPB_CHECK(parse_tag(&buf, end, &tag));
     if(unlikely(tag.wire_type == UPB_WIRE_TYPE_END_GROUP)) {
-      if(unlikely(s->top->end_offset != 0)) return UPB_ERROR_SPURIOUS_END_GROUP;
+      if(unlikely(s->top->end_offset != UINT32_MAX))
+        return UPB_ERROR_SPURIOUS_END_GROUP;
       pop_stack_frame(s);
     } else if(tag.wire_type == UPB_WIRE_TYPE_DELIMITED) {
       parse_delimited(s, &tag, &buf, end, s->offset + (char*)buf - (char*)bufstart);
diff --git a/upb_parse.h b/upb_parse.h
index 466c383..bbe6431 100644
--- a/upb_parse.h
+++ b/upb_parse.h
@@ -130,7 +130,7 @@ INLINE bool upb_check_type(upb_wire_type_t wt, upb_field_type_t ft) {
   if(ft == 10) { // GOOGLE_PROTOBUF_FIELDDESCRIPTORPROTO_TYPE_GROUP)
     return wt == UPB_WIRE_TYPE_START_GROUP;
   } else {
-    /* With packed arrays, anything can be delimited. */
+    /* With packed arrays, anything can be delimited (except groups). */
     return wt == UPB_WIRE_TYPE_DELIMITED ||
            upb_type_info[ft].expected_wire_type == wt;
  }