Bug 891107 - Part 10: Show information about value in pointer related error messages in js-ctypes. r=jorendorff
☠☠ backed out by ea30fdf3bca3 ☠ ☠
authorTooru Fujisawa <arai_a@mac.com>
Fri, 07 Aug 2015 06:58:52 +0900
changeset 288415 07d1058b745e603593394ce9465e91669256e4d4
parent 288414 f223b115ad37d91bb62907afab41646531622006
child 288416 c1bb74893286a8a73eb76700ea65bd5470b77da8
push id73418
push userarai_a@mac.com
push dateSat, 12 Mar 2016 17:22:03 +0000
treeherdermozilla-inbound@8db1ac84d30e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs891107
milestone48.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 891107 - Part 10: Show information about value in pointer related error messages in js-ctypes. r=jorendorff
js/src/ctypes/CTypes.cpp
js/src/ctypes/ctypes.msg
js/src/jit-test/tests/ctypes/pointer.js
toolkit/components/ctypes/tests/unit/test_jsctypes.js
--- a/js/src/ctypes/CTypes.cpp
+++ b/js/src/ctypes/CTypes.cpp
@@ -1728,16 +1728,43 @@ NonPrimitiveError(JSContext* cx, HandleO
     return false;
 
   JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
                        CTYPESMSG_NON_PRIMITIVE, typeStr);
   return false;
 }
 
 static bool
+NonStringBaseError(JSContext* cx, HandleValue thisVal)
+{
+  JSAutoByteString valBytes;
+  const char* valStr = CTypesToSourceForError(cx, thisVal, valBytes);
+  if (!valStr)
+    return false;
+
+  JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                       CTYPESMSG_NON_STRING_BASE, valStr);
+  return false;
+}
+
+static bool
+NullPointerError(JSContext* cx, const char* action, HandleObject obj)
+{
+  JSAutoByteString valBytes;
+  RootedValue val(cx, ObjectValue(*obj));
+  const char* valStr = CTypesToSourceForError(cx, val, valBytes);
+  if (!valStr)
+    return false;
+
+  JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                       CTYPESMSG_NULL_POINTER, action, valStr);
+  return false;
+}
+
+static bool
 PropNameNonStringError(JSContext* cx, HandleId id, HandleValue actual,
                        ConversionType convType,
                        HandleObject funObj = nullptr, unsigned argIndex = 0)
 {
   JSAutoByteString valBytes;
   const char* valStr = CTypesToSourceForError(cx, actual, valBytes);
   if (!valStr)
     return false;
@@ -1795,16 +1822,30 @@ TypeOverflow(JSContext* cx, const char* 
     return false;
 
   JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
                        CTYPESMSG_TYPE_OVERFLOW, valStr, expected);
   return false;
 }
 
 static bool
+UndefinedSizePointerError(JSContext* cx, const char* action, HandleObject obj)
+{
+  JSAutoByteString valBytes;
+  RootedValue val(cx, ObjectValue(*obj));
+  const char* valStr = CTypesToSourceForError(cx, val, valBytes);
+  if (!valStr)
+    return false;
+
+  JS_ReportErrorNumber(cx, GetErrorMessage, nullptr,
+                       CTYPESMSG_UNDEFINED_SIZE, action, valStr);
+  return false;
+}
+
+static bool
 VariadicArgumentTypeError(JSContext* cx, uint32_t index, HandleValue actual)
 {
   JSAutoByteString valBytes;
   const char* valStr = CTypesToSourceForError(cx, actual, valBytes);
   if (!valStr)
     return false;
 
   char indexStr[16];
@@ -5138,17 +5179,17 @@ PointerType::IsNull(JSContext* cx, unsig
   void* data = *static_cast<void**>(CData::GetData(obj));
   args.rval().setBoolean(data == nullptr);
   return true;
 }
 
 bool
 PointerType::OffsetBy(JSContext* cx, const CallArgs& args, int offset)
 {
-  JSObject* obj = JS_THIS_OBJECT(cx, args.base());
+  RootedObject obj(cx, JS_THIS_OBJECT(cx, args.base()));
   if (!obj)
     return false;
   if (!CData::IsCData(obj)) {
     if (offset == 1) {
       return IncompatibleThisProto(cx, "PointerType.prototype.increment",
                                    args.thisv());
     }
     return IncompatibleThisProto(cx, "PointerType.prototype.decrement",
@@ -5162,18 +5203,17 @@ PointerType::OffsetBy(JSContext* cx, con
                                   "non-PointerType CData", args.thisv());
     }
     return IncompatibleThisType(cx, "PointerType.prototype.decrement",
                                 "non-PointerType CData", args.thisv());
   }
 
   RootedObject baseType(cx, PointerType::GetBaseType(typeObj));
   if (!CType::IsSizeDefined(baseType)) {
-    JS_ReportError(cx, "cannot modify pointer of undefined size");
-    return false;
+    return UndefinedSizePointerError(cx, "modify", obj);
   }
 
   size_t elementSize = CType::GetSize(baseType);
   char* data = static_cast<char*>(*static_cast<void**>(CData::GetData(obj)));
   void* address = data + offset * elementSize;
 
   // Create a PointerType CData object containing the new address.
   JSObject* result = CData::Create(cx, typeObj, nullptr, &address, true);
@@ -5199,48 +5239,44 @@ PointerType::Decrement(JSContext* cx, un
 }
 
 bool
 PointerType::ContentsGetter(JSContext* cx, const JS::CallArgs& args)
 {
   RootedObject obj(cx, &args.thisv().toObject());
   RootedObject baseType(cx, GetBaseType(CData::GetCType(obj)));
   if (!CType::IsSizeDefined(baseType)) {
-    JS_ReportError(cx, "cannot get contents of undefined size");
-    return false;
+    return UndefinedSizePointerError(cx, "get contents of", obj);
   }
 
   void* data = *static_cast<void**>(CData::GetData(obj));
   if (data == nullptr) {
-    JS_ReportError(cx, "cannot read contents of null pointer");
-    return false;
+    return NullPointerError(cx, "read contents of", obj);
   }
 
   RootedValue result(cx);
   if (!ConvertToJS(cx, baseType, nullptr, data, false, false, &result))
     return false;
 
   args.rval().set(result);
   return true;
 }
 
 bool
 PointerType::ContentsSetter(JSContext* cx, const JS::CallArgs& args)
 {
   RootedObject obj(cx, &args.thisv().toObject());
   RootedObject baseType(cx, GetBaseType(CData::GetCType(obj)));
   if (!CType::IsSizeDefined(baseType)) {
-    JS_ReportError(cx, "cannot set contents of undefined size");
-    return false;
+    return UndefinedSizePointerError(cx, "set contents of", obj);
   }
 
   void* data = *static_cast<void**>(CData::GetData(obj));
   if (data == nullptr) {
-    JS_ReportError(cx, "cannot write contents to null pointer");
-    return false;
+    return NullPointerError(cx, "write contents to", obj);
   }
 
   args.rval().setUndefined();
   return ImplicitConvert(cx, args.get(0), baseType, data,
                          ConversionType::Setter, nullptr);
 }
 
 /*******************************************************************************
@@ -7733,28 +7769,26 @@ ReadStringCommon(JSContext* cx, InflateU
   TypeCode typeCode = CType::GetTypeCode(typeObj);
   void* data;
   size_t maxLength = -1;
   switch (typeCode) {
   case TYPE_pointer:
     baseType = PointerType::GetBaseType(typeObj);
     data = *static_cast<void**>(CData::GetData(obj));
     if (data == nullptr) {
-      JS_ReportError(cx, "cannot read contents of null pointer");
-      return false;
+      return NullPointerError(cx, "read contents of", obj);
     }
     break;
   case TYPE_array:
     baseType = ArrayType::GetBaseType(typeObj);
     data = CData::GetData(obj);
     maxLength = ArrayType::GetLength(typeObj);
     break;
   default:
-    JS_ReportError(cx, "not a PointerType or ArrayType");
-    return false;
+    return TypeError(cx, "PointerType or ArrayType", args.thisv());
   }
 
   // Convert the string buffer, taking care to determine the correct string
   // length in the case of arrays (which may contain embedded nulls).
   JSString* result;
   switch (CType::GetTypeCode(baseType)) {
   case TYPE_int8_t:
   case TYPE_uint8_t:
@@ -7778,19 +7812,17 @@ ReadStringCommon(JSContext* cx, InflateU
   case TYPE_unsigned_short:
   case TYPE_char16_t: {
     char16_t* chars = static_cast<char16_t*>(data);
     size_t length = strnlen(chars, maxLength);
     result = JS_NewUCStringCopyN(cx, chars, length);
     break;
   }
   default:
-    JS_ReportError(cx,
-      "base type is not an 8-bit or 16-bit integer or character type");
-    return false;
+    return NonStringBaseError(cx, args.thisv());
   }
 
   if (!result)
     return false;
 
   args.rval().setString(result);
   return true;
 }
--- a/js/src/ctypes/ctypes.msg
+++ b/js/src/ctypes/ctypes.msg
@@ -60,8 +60,13 @@ MSG_DEF(CTYPESMSG_TYPE_OVERFLOW, 2, JSEX
 MSG_DEF(CTYPESMSG_ARG_COUNT_MISMATCH,4, JSEXN_RANGEERR, "number of arguments does not match declaration of {0} (expected {1}{2}, got {3})")
 MSG_DEF(CTYPESMSG_ARG_TYPE_ERROR,3, JSEXN_TYPEERR, "the type of argument {0} {1} (got {2})")
 MSG_DEF(CTYPESMSG_FUNCTION_CONSTRUCT,0, JSEXN_TYPEERR, "cannot construct from FunctionType; use FunctionType.ptr instead")
 MSG_DEF(CTYPESMSG_RET_TYPE_ERROR,2, JSEXN_TYPEERR, "return type {0} (got {1})")
 MSG_DEF(CTYPESMSG_VARG_TYPE_ERROR,2, JSEXN_TYPEERR, "variadic argument {0} must be a CData object (got {1})")
 
 /* ctype */
 MSG_DEF(CTYPESMSG_CANNOT_CONSTRUCT,1, JSEXN_TYPEERR, "cannot construct from {0}")
+
+/* pointer */
+MSG_DEF(CTYPESMSG_UNDEFINED_SIZE,2, JSEXN_TYPEERR, "cannot {0} pointer of undefined size {1}")
+MSG_DEF(CTYPESMSG_NULL_POINTER,  2, JSEXN_TYPEERR, "cannot {0} null pointer {1}")
+MSG_DEF(CTYPESMSG_NON_STRING_BASE,1, JSEXN_TYPEERR, "base type {0} is not an 8-bit or 16-bit integer or character type")
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ctypes/pointer.js
@@ -0,0 +1,31 @@
+load(libdir + 'asserts.js');
+
+function test() {
+  let p = ctypes.StructType("foo").ptr(0);
+
+  assertTypeErrorMessage(() => { p.increment(); },
+                         "cannot modify pointer of undefined size foo.ptr(ctypes.UInt64(\"0x0\"))");
+  assertTypeErrorMessage(() => { p.decrement(); },
+                         "cannot modify pointer of undefined size foo.ptr(ctypes.UInt64(\"0x0\"))");
+
+  assertTypeErrorMessage(() => { let a = p.contents; },
+                         "cannot get contents of pointer of undefined size foo.ptr(ctypes.UInt64(\"0x0\"))");
+  assertTypeErrorMessage(() => { p.contents = 1; },
+                         "cannot set contents of pointer of undefined size foo.ptr(ctypes.UInt64(\"0x0\"))");
+
+  let p2 = ctypes.int32_t.ptr(0);
+  assertTypeErrorMessage(() => { let a = p2.contents; },
+                         "cannot read contents of null pointer ctypes.int32_t.ptr(ctypes.UInt64(\"0x0\"))");
+  assertTypeErrorMessage(() => { p2.contents = 1; },
+                         "cannot write contents to null pointer ctypes.int32_t.ptr(ctypes.UInt64(\"0x0\"))");
+  assertTypeErrorMessage(() => { p2.readString(); },
+                         "cannot read contents of null pointer ctypes.int32_t.ptr(ctypes.UInt64(\"0x0\"))");
+
+  assertTypeErrorMessage(() => { ctypes.int32_t(0).readString(); },
+                         "expected PointerType or ArrayType, got ctypes.int32_t(0)");
+  assertTypeErrorMessage(() => { ctypes.int32_t(0).address().readString(); },
+                         /base type ctypes\.int32_t\.ptr\(ctypes\.UInt64\(\"[x0-9A-Fa-f]+\"\)\) is not an 8-bit or 16-bit integer or character type/);
+}
+
+if (typeof ctypes === "object")
+  test();
--- a/toolkit/components/ctypes/tests/unit/test_jsctypes.js
+++ b/toolkit/components/ctypes/tests/unit/test_jsctypes.js
@@ -1686,37 +1686,37 @@ function run_PointerType_tests() {
   do_check_true(p_t.ptr === ctypes.PointerType(p_t));
   do_check_eq(p_t.array().name, name + "*[]");
   do_check_eq(p_t.array(5).name, name + "*[5]");
 
   // Test ExplicitConvert.
   let p = p_t();
   do_check_throws(function() { p.value; }, TypeError);
   do_check_eq(ptrValue(p), 0);
-  do_check_throws(function() { p.contents; }, Error);
-  do_check_throws(function() { p.contents = g; }, Error);
+  do_check_throws(function() { p.contents; }, TypeError);
+  do_check_throws(function() { p.contents = g; }, TypeError);
   p = p_t(5);
   do_check_eq(ptrValue(p), 5);
   p = p_t(ctypes.UInt64(10));
   do_check_eq(ptrValue(p), 10);
 
   // Test ImplicitConvert.
   p.value = null;
   do_check_eq(ptrValue(p), 0);
   do_check_throws(function() { p.value = 5; }, TypeError);
 
   // Test opaque pointers.
   let f_t = ctypes.StructType("FILE").ptr;
   do_check_eq(f_t.name, "FILE*");
   do_check_eq(f_t.toSource(), 'ctypes.StructType("FILE").ptr');
   let f = new f_t();
-  do_check_throws(function() { f.contents; }, Error);
-  do_check_throws(function() { f.contents = 0; }, Error);
+  do_check_throws(function() { f.contents; }, TypeError);
+  do_check_throws(function() { f.contents = 0; }, TypeError);
   f = f_t(5);
-  do_check_throws(function() { f.contents = 0; }, Error);
+  do_check_throws(function() { f.contents = 0; }, TypeError);
   do_check_eq(f.toSource(), 'FILE.ptr(ctypes.UInt64("0x5"))');
 
   do_check_throws(function() { f_t(p); }, TypeError);
   do_check_throws(function() { f.value = p; }, TypeError);
   do_check_throws(function() { p.value = f; }, TypeError);
 
   // Test void pointers.
   let v_t = ctypes.PointerType(ctypes.void_t);
@@ -1749,17 +1749,17 @@ function run_PointerType_tests() {
   a_p = a_p.decrement();
   do_check_eq(a_p.contents.a, 1);
   do_check_eq(a_p.contents.b, 2);
 
   // Check that pointers to arrays of undefined or zero length are legal,
   // but that the former cannot be dereferenced.
   let z_t = ctypes.int32_t.array().ptr;
   do_check_eq(ptrValue(z_t()), 0);
-  do_check_throws(function() { z_t().contents }, Error);
+  do_check_throws(function() { z_t().contents }, TypeError);
   z_t = ctypes.int32_t.array(0).ptr;
   do_check_eq(ptrValue(z_t()), 0);
   let z = ctypes.int32_t.array(0)().address();
   do_check_eq(z.contents.length, 0);
 
   // TODO: Somehow, somewhere we should check that:
   //
   //  (a) ArrayBuffer and TypedArray can be passed by pointer to a C function