Check extension field numbers against extension ranges.

This makes extension checking more strict in most cases.
However it also fixes a bug with MessageSet where we were being
too strict.  MessageSet allows larger extension numbers than
normal extensions do.

upb/def.c

@@ -2202,10 +2202,6 @@ static void create_fielddef(
 
   field_number = google_protobuf_FieldDescriptorProto_number(field_proto);
 
-  if (field_number == 0 || field_number > UPB_MAX_FIELDNUMBER) {
-    symtab_errf(ctx, "invalid field number (%u)", field_number);
-  }
-
   f->full_name = full_name;
   f->json_name = json_name;
   f->label_ = (int)google_protobuf_FieldDescriptorProto_label(field_proto);
@@ -2245,6 +2241,10 @@ static void create_fielddef(
     upb_value v, field_v, json_v, existing_v;
     size_t json_size;
 
+    if (field_number == 0 || field_number > UPB_MAX_FIELDNUMBER) {
+      symtab_errf(ctx, "invalid field number (%u)", field_number);
+    }
+
     f->index_ = f - m->fields;
     f->msgdef = m;
     f->is_extension_ = false;
@@ -2647,15 +2647,13 @@ static void create_msgdef(symtab_addctx *ctx, const char *prefix,
   }
 }
 
-static void resolve_fielddef(symtab_addctx *ctx, const char *prefix,
+static void resolve_subdef(symtab_addctx *ctx, const char *prefix,
-                             upb_fielddef *f) {
+                           upb_fielddef *f) {
   const google_protobuf_FieldDescriptorProto *field_proto = f->sub.unresolved;
   upb_strview name =
       google_protobuf_FieldDescriptorProto_type_name(field_proto);
   bool has_name =
       google_protobuf_FieldDescriptorProto_has_type_name(field_proto);
-
-  // Resolve subdef by type name, if necessary.
   switch ((int)f->type_) {
     case FIELD_TYPE_UNSPECIFIED: {
       // Type was not specified and must be inferred.
@@ -2693,36 +2691,60 @@ static void resolve_fielddef(symtab_addctx *ctx, const char *prefix,
       // No resolution necessary.
       break;
   }
+}
+
+static void resolve_extension(
+    symtab_addctx *ctx, const char *prefix, upb_fielddef *f,
+    const google_protobuf_FieldDescriptorProto *field_proto) {
+  if (!google_protobuf_FieldDescriptorProto_has_extendee(field_proto)) {
+    symtab_errf(ctx, "extension for field '%s' had no extendee",
+                f->full_name);
+  }
 
-  if (f->is_extension_) {
+  upb_strview name = google_protobuf_FieldDescriptorProto_extendee(field_proto);
-    if (!google_protobuf_FieldDescriptorProto_has_extendee(field_proto)) {
+  const upb_msgdef *m =
-      symtab_errf(ctx, "extension for field '%s' had no extendee",
+      symtab_resolve(ctx, f->full_name, prefix, name, UPB_DEFTYPE_MSG);
-                  f->full_name);
+  f->msgdef = m;
-    }
 
-    name = google_protobuf_FieldDescriptorProto_extendee(field_proto);
+  bool found = false;
-    f->msgdef =
-        symtab_resolve(ctx, f->full_name, prefix, name, UPB_DEFTYPE_MSG);
 
-    const upb_msglayout_ext *ext = ctx->file->ext_layouts[f->layout_index];
+  for (int i = 0, n = m->ext_range_count; i < n; i++) {
-    if (ctx->layout) {
+    const upb_extrange *r = &m->ext_ranges[i];
-      UPB_ASSERT(upb_fielddef_number(f) == ext->field.number);
+    if (r->start <= f->number_ && f->number_ < r->end) {
-    } else {
+      found = true;
-      upb_msglayout_ext *mut_ext = (upb_msglayout_ext*)ext;
+      break;
-      fill_fieldlayout(&mut_ext->field, f);
-      mut_ext->field.presence = 0;
-      mut_ext->field.offset = 0;
-      mut_ext->field.submsg_index = 0;
-      mut_ext->extendee = f->msgdef->layout;
-      mut_ext->sub.submsg = f->sub.msgdef->layout;
     }
+  }
+
+  if (!found) {
+    symtab_errf(ctx,
+                "field number %u in extension %s has no extension range in "
+                "message %s",
+                (unsigned)f->number_, f->full_name, f->msgdef->full_name);
+  }
 
-    CHK_OOM(upb_inttable_insert(&ctx->symtab->exts, (uintptr_t)ext,
+  const upb_msglayout_ext *ext = ctx->file->ext_layouts[f->layout_index];
-                                upb_value_constptr(f), ctx->arena));
+  if (ctx->layout) {
+    UPB_ASSERT(upb_fielddef_number(f) == ext->field.number);
+  } else {
+    upb_msglayout_ext *mut_ext = (upb_msglayout_ext*)ext;
+    fill_fieldlayout(&mut_ext->field, f);
+    mut_ext->field.presence = 0;
+    mut_ext->field.offset = 0;
+    mut_ext->field.submsg_index = 0;
+    mut_ext->extendee = f->msgdef->layout;
+    mut_ext->sub.submsg = f->sub.msgdef->layout;
   }
 
-  /* Have to delay resolving of the default value until now because of the enum
+  CHK_OOM(upb_inttable_insert(&ctx->symtab->exts, (uintptr_t)ext,
-   * case, since enum defaults are specified with a label. */
+                              upb_value_constptr(f), ctx->arena));
+}
+
+static void resolve_default(
+    symtab_addctx *ctx, upb_fielddef *f,
+    const google_protobuf_FieldDescriptorProto *field_proto) {
+  // Have to delay resolving of the default value until now because of the enum
+  // case, since enum defaults are specified with a label.
   if (google_protobuf_FieldDescriptorProto_has_default_value(field_proto)) {
     upb_strview defaultval =
         google_protobuf_FieldDescriptorProto_default_value(field_proto);
@@ -2745,6 +2767,19 @@ static void resolve_fielddef(symtab_addctx *ctx, const char *prefix,
   }
 }
 
+static void resolve_fielddef(symtab_addctx *ctx, const char *prefix,
+                             upb_fielddef *f) {
+  // We have to stash this away since resolve_subdef() may overwrite it.
+  const google_protobuf_FieldDescriptorProto *field_proto = f->sub.unresolved;
+
+  resolve_subdef(ctx, prefix, f);
+  resolve_default(ctx, f, field_proto);
+
+  if (f->is_extension_) {
+    resolve_extension(ctx, prefix, f, field_proto);
+  }
+}
+
 static void resolve_msgdef(symtab_addctx *ctx, upb_msgdef *m) {
   for (int i = 0; i < m->field_count; i++) {
     resolve_fielddef(ctx, m->full_name, (upb_fielddef *)&m->fields[i]);

upb/util/def_to_proto_test.proto

@@ -74,3 +74,17 @@ service Service {
 extend Message {
   optional int32 ext = 1001;
 }
+
+message PretendMessageSet {
+  option message_set_wire_format = true;
+  // Since this is message_set_wire_format, "max" here means INT32_MAX.
+  // (For normal messages "max" would mean 2**29 - 1).
+  extensions 4 to max;
+}
+
+message MessageSetItem {
+  extend PretendMessageSet {
+    // Since max is exclusive, this is INT32_MAX-1, not INT32_MAX.
+    optional MessageSetItem message_set_extension = 2147483646;
+  }
+}