Bug 898622 - Fix StructType errors and handling. r=nmatsakis
authorNikhil Marathe <nsm.nikhil@gmail.com>
Thu, 01 Aug 2013 13:10:50 -0700
changeset 140982 b6b697ead5340bc09d1bb0e86db22188ceb9d29d
parent 140981 42c35a94058bbd5259783bb8398b233eb6b20fb2
child 140983 e827cc07b0068be32bd6f2c107072dec47f9b9ef
push id25046
push useremorley@mozilla.com
push dateFri, 02 Aug 2013 12:29:51 +0000
treeherdermozilla-central@04dd60bdbc04 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnmatsakis
bugs898622
milestone25.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 898622 - Fix StructType errors and handling. r=nmatsakis
js/src/builtin/BinaryData.cpp
js/src/js.msg
js/src/tests/ecma_6/BinaryData/structtype.js
--- a/js/src/builtin/BinaryData.cpp
+++ b/js/src/builtin/BinaryData.cpp
@@ -55,20 +55,18 @@ static JSBool
 DataThrowError(JSContext *cx, unsigned argc, Value *vp)
 {
     return ReportIsNotFunction(cx, *vp);
 }
 
 static void
 ReportTypeError(JSContext *cx, HandleValue fromValue, const char *toType)
 {
-    char *valueStr = JS_EncodeString(cx, JS_ValueToString(cx, fromValue));
     JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CANT_CONVERT_TO,
-                         valueStr, toType);
-    JS_free(cx, (void *) valueStr);
+                         InformalValueTypeName(fromValue), toType);
 }
 
 static void
 ReportTypeError(JSContext *cx, HandleValue fromValue, JSString *toType)
 {
     const char *fnName = JS_EncodeString(cx, toType);
     ReportTypeError(cx, fromValue, fnName);
     JS_free(cx, (void *) fnName);
@@ -769,17 +767,18 @@ DataInstanceUpdate(JSContext *cx, unsign
 
     if (args.length() < 1) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage,
                              NULL, JSMSG_MORE_ARGS_NEEDED,
                              "update()", "0", "s");
         return false;
     }
 
-    RootedObject thisObj(cx, args.thisv().toObjectOrNull());
+    RootedObject thisObj(cx, args.thisv().isObject() ?
+                                args.thisv().toObjectOrNull() : NULL);
     if (!IsBlock(thisObj)) {
         RootedValue thisObjVal(cx, ObjectValue(*thisObj));
         ReportTypeError(cx, thisObjVal, "BinaryData block");
         return false;
     }
 
     RootedValue val(cx, args[0]);
     uint8_t *memory = (uint8_t*) thisObj->getPrivate();
@@ -826,25 +825,21 @@ ArrayType::repeat(JSContext *cx, unsigne
 
     if (args.length() < 1) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage,
                              NULL, JSMSG_MORE_ARGS_NEEDED,
                              "repeat()", "0", "s");
         return false;
     }
 
-    RootedObject thisObj(cx, args.thisv().toObjectOrNull());
+    RootedObject thisObj(cx, args.thisv().isObject() ?
+                                args.thisv().toObjectOrNull() : NULL);
     if (!IsArrayType(thisObj)) {
-        char *valueChars = const_cast<char*>("(unknown type)");
-        RootedString valueStr(cx, JS_ValueToString(cx, args.thisv()));
-        if (valueStr)
-            valueChars = JS_EncodeString(cx, valueStr);
-        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_INCOMPATIBLE_PROTO, "ArrayType", "repeat", valueChars);
-        if (valueStr)
-            JS_free(cx, valueChars);
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_INCOMPATIBLE_PROTO,
+                             "ArrayType", "repeat", InformalValueTypeName(args.thisv()));
         return false;
     }
 
     RootedObject binaryArray(cx, BinaryArray::create(cx, thisObj));
     if (!binaryArray)
         return false;
 
     RootedValue val(cx, args[0]);
@@ -855,22 +850,22 @@ ArrayType::repeat(JSContext *cx, unsigne
     return true;
 }
 
 JSBool
 ArrayType::toString(JSContext *cx, unsigned int argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
-    RootedObject thisObj(cx, args.thisv().toObjectOrNull());
-    JS_ASSERT(thisObj);
+    RootedObject thisObj(cx, args.thisv().isObject() ?
+                                args.thisv().toObjectOrNull() : NULL);
     if (!IsArrayType(thisObj)) {
-        RootedObject obj(cx, args.thisv().toObjectOrNull());
         JS_ReportErrorNumber(cx, js_GetErrorMessage,
-                             NULL, JSMSG_INCOMPATIBLE_PROTO, "ArrayType", "toString", JS_GetClass(obj)->name);
+                             NULL, JSMSG_INCOMPATIBLE_PROTO,
+                             "ArrayType", "toString", InformalValueTypeName(args.thisv()));
         return false;
     }
 
     StringBuffer contents(cx);
     contents.append("ArrayType(");
 
     RootedValue elementTypeVal(cx, ObjectValue(*elementType(cx, thisObj)));
     JSString *str = ToString<CanGC>(cx, elementTypeVal);
@@ -1035,20 +1030,20 @@ JSBool BinaryArray::subarray(JSContext *
     }
 
     if (!args[0].isInt32()) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
                              JSMSG_BINARYDATA_SUBARRAY_INTEGER_ARG, "1");
         return false;
     }
 
-    RootedObject thisObj(cx, &args.thisv().toObject());
+    RootedObject thisObj(cx, args.thisv().isObject() ?
+                                args.thisv().toObjectOrNull() : NULL);
     if (!IsBinaryArray(thisObj)) {
-        RootedValue thisObjVal(cx, ObjectValue(*thisObj));
-        ReportTypeError(cx, thisObjVal, "binary array");
+        ReportTypeError(cx, args.thisv(), "binary array");
         return false;
     }
 
     RootedObject type(cx, GetType(thisObj));
     RootedObject elementType(cx, ArrayType::elementType(cx, type));
     uint32_t length = ArrayType::length(cx, type);
 
     int32_t begin = args[0].toInt32();
@@ -1103,23 +1098,20 @@ BinaryArray::fill(JSContext *cx, unsigne
 
     if (args.length() < 1) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage,
                              NULL, JSMSG_MORE_ARGS_NEEDED,
                              "fill()", "0", "s");
         return false;
     }
 
-    if (!args.thisv().isObject())
-        return false;
-
-    RootedObject thisObj(cx, args.thisv().toObjectOrNull());
+    RootedObject thisObj(cx, args.thisv().isObject() ?
+                                args.thisv().toObjectOrNull() : NULL);
     if (!IsBinaryArray(thisObj)) {
-        RootedValue thisObjVal(cx, ObjectValue(*thisObj));
-        ReportTypeError(cx, thisObjVal, "binary array");
+        ReportTypeError(cx, args.thisv(), "binary array");
         return false;
     }
 
     Value funArrayTypeVal = GetFunctionNativeReserved(&args.callee(), 0);
     JS_ASSERT(funArrayTypeVal.isObject());
 
     RootedObject type(cx, GetType(thisObj));
     RootedObject funArrayType(cx, funArrayTypeVal.toObjectOrNull());
@@ -1514,45 +1506,62 @@ Class BinaryStruct::class_ = {
  */
 bool
 StructType::layout(JSContext *cx, HandleObject structType, HandleObject fields)
 {
     AutoIdVector fieldProps(cx);
     if (!GetPropertyNames(cx, fields, JSITER_OWNONLY, &fieldProps))
         return false;
 
+    if (fieldProps.length() <= 0) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
+                             JSMSG_BINARYDATA_STRUCTTYPE_EMPTY_DESCRIPTOR);
+        return false;
+    }
+
+    // All error branches from here onwards should |goto error;| to free this list.
     FieldList *fieldList = new FieldList(fieldProps.length());
 
     uint32_t structAlign = 0;
     uint32_t structMemSize = 0;
     uint32_t structByteSize = 0;
     size_t structTail = 0;
 
     for (unsigned int i = 0; i < fieldProps.length(); i++) {
         RootedValue fieldTypeVal(cx);
         RootedId id(cx, fieldProps[i]);
         if (!JSObject::getGeneric(cx, fields, fields, id, &fieldTypeVal))
             goto error;
 
+        if (!fieldTypeVal.isObject()) {
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
+                                 JSMSG_BINARYDATA_STRUCTTYPE_BAD_FIELD, JSID_TO_STRING(id));
+            goto error;
+        }
+
         RootedObject fieldType(cx, fieldTypeVal.toObjectOrNull());
-        if (!IsBinaryType(fieldType))
+        if (!IsBinaryType(fieldType)) {
+            JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
+                                 JSMSG_BINARYDATA_STRUCTTYPE_BAD_FIELD, JSID_TO_STRING(id));
             goto error;
+        }
 
         size_t fieldMemSize = GetMemSize(cx, fieldType);
         size_t fieldAlign = GetAlign(cx, fieldType);
         size_t fieldOffset = AlignBytes(structMemSize, fieldAlign);
 
         structMemSize = fieldOffset + fieldMemSize;
 
         if (fieldAlign > structAlign)
             structAlign = fieldAlign;
 
+        // If the field type is a BinaryType and we can't get its bytes, we have a problem.
         RootedValue fieldTypeBytes(cx);
-        if (!JSObject::getProperty(cx, fieldType, fieldType, cx->names().bytes, &fieldTypeBytes))
-            goto error;
+        bool r = JSObject::getProperty(cx, fieldType, fieldType, cx->names().bytes, &fieldTypeBytes);
+        JS_ASSERT(r);
 
         JS_ASSERT(fieldTypeBytes.isInt32());
         structByteSize += fieldTypeBytes.toInt32();
 
         (*fieldList)[i].name = fieldProps[i];
         (*fieldList)[i].type = fieldType.get();
         (*fieldList)[i].offset = fieldOffset;
     }
@@ -1646,21 +1655,18 @@ StructType::reify(JSContext *cx, HandleO
 JSObject *
 StructType::create(JSContext *cx, HandleObject structTypeGlobal,
                    HandleObject fields)
 {
     RootedObject obj(cx, NewBuiltinClassInstance(cx, &StructType::class_));
     if (!obj)
         return NULL;
 
-    if (!StructType::layout(cx, obj, fields)) {
-        RootedValue fieldsVal(cx, ObjectValue(*fields));
-        ReportTypeError(cx, fieldsVal, "StructType field specifier");
+    if (!StructType::layout(cx, obj, fields))
         return NULL;
-    }
 
     RootedObject fieldsProto(cx);
     if (!JSObject::getProto(cx, fields, &fieldsProto))
         return NULL;
 
     RootedObject clone(cx, CloneObject(cx, fields, fieldsProto, NullPtr()));
     if (!clone)
         return NULL;
@@ -1726,20 +1732,29 @@ StructType::trace(JSTracer *tracer, JSOb
     }
 }
 
 JSBool
 StructType::toString(JSContext *cx, unsigned int argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
+    if (!args.thisv().isObject()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_INCOMPATIBLE_PROTO,
+                             "StructType", "toString", InformalValueTypeName(args.thisv()));
+        return false;
+    }
+
     RootedObject thisObj(cx, args.thisv().toObjectOrNull());
 
-    if (!IsStructType(thisObj))
+    if (!IsStructType(thisObj)) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_INCOMPATIBLE_PROTO,
+                             "StructType", "toString", InformalValueTypeName(args.thisv()));
         return false;
+    }
 
     StringBuffer contents(cx);
     contents.append("StructType({");
 
     FieldList *fieldList = GetStructTypeFieldList(thisObj);
     JS_ASSERT(fieldList);
 
     for (FieldList::const_iterator it = fieldList->begin(); it != fieldList->end(); ++it) {
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -402,8 +402,10 @@ MSG_DEF(JSMSG_PAR_ARRAY_SCATTER_BAD_TARG
 MSG_DEF(JSMSG_SELFHOSTED_UNBOUND_NAME,349, 0, JSEXN_TYPEERR, "self-hosted code may not contain unbound name lookups")
 MSG_DEF(JSMSG_DEPRECATED_SOURCE_MAP,  350, 0, JSEXN_SYNTAXERR, "Using //@ to indicate source map URL pragmas is deprecated. Use //# instead")
 MSG_DEF(JSMSG_BAD_DESTRUCT_ASSIGN,    351, 1, JSEXN_SYNTAXERR, "can't assign to {0} using destructuring assignment")
 MSG_DEF(JSMSG_BINARYDATA_ARRAYTYPE_BAD_ARGS, 352, 0, JSEXN_ERR, "Invalid arguments")
 MSG_DEF(JSMSG_BINARYDATA_BINARYARRAY_BAD_INDEX, 353, 0, JSEXN_RANGEERR, "invalid or out-of-range index")
 MSG_DEF(JSMSG_BINARYDATA_STRUCTTYPE_BAD_ARGS, 354, 0, JSEXN_RANGEERR, "invalid field descriptor")
 MSG_DEF(JSMSG_BINARYDATA_NOT_BINARYSTRUCT,   355, 1, JSEXN_TYPEERR, "{0} is not a BinaryStruct")
 MSG_DEF(JSMSG_BINARYDATA_SUBARRAY_INTEGER_ARG, 356, 1, JSEXN_ERR, "argument {0} must be an integer")
+MSG_DEF(JSMSG_BINARYDATA_STRUCTTYPE_EMPTY_DESCRIPTOR, 357, 0, JSEXN_ERR, "field descriptor cannot be empty")
+MSG_DEF(JSMSG_BINARYDATA_STRUCTTYPE_BAD_FIELD, 358, 1, JSEXN_ERR, "field {0} is not a valid BinaryData Type descriptor")
--- a/js/src/tests/ecma_6/BinaryData/structtype.js
+++ b/js/src/tests/ecma_6/BinaryData/structtype.js
@@ -30,16 +30,19 @@ function runTests() {
     assertEq(s.__proto__, S.prototype);
     s.x = 2;
     s.y = 255;
     s.z = 12.342345;
     assertEq(s.x, 2);
     assertEq(s.y, 255);
     assertEq(s.z, 12.342345);
 
+    assertThrows(function() new StructType(RegExp));
+    assertThrows(function() new StructType(RegExp()));
+
     var Color = new StructType({r: uint8, g: uint8, b: uint8});
     var white = new Color();
     white.r = white.g = white.b = 255;
 
     var Car = new StructType({color: Color, weight: uint32});
     assertEq(Car.toString(), "StructType({color: StructType({r: uint8, g: uint8, b: uint8}), weight: uint32})");
 
     var civic = new Car();