Bug 556329 - Allow ctypes to load exported data symbols. r=jorendorff

js/src/ctypes/CTypes.cpp

@@ -5077,7 +5077,7 @@ CClosure::ClosureStub(ffi_cif* cif, void* result, void** args, void* userData)
 //   a CData object; merely an object we want to keep alive.
 //   * If 'refObj' is a CData object, 'ownResult' must be false.
 //   * Otherwise, 'refObj' is a Library or CClosure object, and 'ownResult'
-//     must be true.
+//     may be true or false.
 // * Otherwise 'refObj' is NULL. In this case, 'ownResult' may be true or false.
 //
 // * If 'ownResult' is true, the CData object will allocate an appropriately
@@ -5085,7 +5085,7 @@ CClosure::ClosureStub(ffi_cif* cif, void* result, void** args, void* userData)
 //   supplied, the data will be copied from 'source' into the buffer;
 //   otherwise, the entirety of the new buffer will be initialized to zero.
 // * If 'ownResult' is false, the new CData's buffer refers to a slice of
-//   another CData's buffer given by 'refObj'. 'source' data must be provided,
+//   another buffer kept alive by 'refObj'. 'source' data must be provided,
 //   and the new CData's buffer will refer to 'source'.
 JSObject*
 CData::Create(JSContext* cx,
@@ -5098,9 +5098,7 @@ CData::Create(JSContext* cx,
   JS_ASSERT(CType::IsCType(cx, typeObj));
   JS_ASSERT(CType::IsSizeDefined(cx, typeObj));
   JS_ASSERT(ownResult || source);
-  if (refObj) {
-    JS_ASSERT(CData::IsCData(cx, refObj) ? !ownResult : ownResult);
-  }
+  JS_ASSERT_IF(refObj && CData::IsCData(cx, refObj), !ownResult);
 
   // Get the 'prototype' property from the type.
   jsval slot;

js/src/ctypes/Library.cpp

@@ -215,9 +215,18 @@ Library::Declare(JSContext* cx, uintN argc, jsval* vp)
     return JS_FALSE;
   }
 
-  // we always need at least a method name, a call type and a return type
-  if (argc < 3) {
-    JS_ReportError(cx, "declare requires at least three arguments");
+  // We allow two API variants:
+  // 1) library.declare(name, abi, returnType, argType1, ...)
+  //    declares a function with the given properties, and resolves the symbol
+  //    address in the library.
+  // 2) library.declare(name, type)
+  //    declares a symbol of 'type', and resolves it. The object that comes
+  //    back will be of type 'type', and will point into the symbol data.
+  //    This data will be both readable and writable via the usual CData
+  //    accessors. If 'type' is a FunctionType, the result will be a function
+  //    pointer, as with 1). 
+  if (argc < 2) {
+    JS_ReportError(cx, "declare requires at least two arguments");
     return JS_FALSE;
   }
 
@@ -231,24 +240,55 @@ Library::Declare(JSContext* cx, uintN argc, jsval* vp)
   if (!name)
     return JS_FALSE;
 
-  PRFuncPtr func = PR_FindFunctionSymbol(library, name);
-  if (!func) {
-    JS_ReportError(cx, "couldn't find function symbol in library");
-    return JS_FALSE;
+  JSObject* typeObj;
+  js::AutoValueRooter root(cx);
+  bool isFunction = argc > 2;
+  if (isFunction) {
+    // Case 1).
+    // Create a FunctionType representing the function.
+    typeObj = FunctionType::CreateInternal(cx,
+                argv[1], argv[2], &argv[3], argc - 3);
+    if (!typeObj)
+      return JS_FALSE;
+    root.setObject(typeObj);
+
+  } else {
+    // Case 2).
+    if (JSVAL_IS_PRIMITIVE(argv[1]) ||
+        !CType::IsCType(cx, JSVAL_TO_OBJECT(argv[1]))) {
+      JS_ReportError(cx, "second argument must be a type");
+      return JS_FALSE;
+    }
+
+    typeObj = JSVAL_TO_OBJECT(argv[1]);
+    isFunction = CType::GetTypeCode(cx, typeObj) == TYPE_function;
   }
 
-  // Create a FunctionType representing the function.
-  JSObject* typeObj = FunctionType::CreateInternal(cx,
-                        argv[1], argv[2], &argv[3], argc - 3);
-  if (!typeObj)
-    return JS_FALSE;
-  js::AutoValueRooter root(cx, typeObj);
+  void* data;
+  PRFuncPtr fnptr;
+  if (isFunction) {
+    // Look up the function symbol.
+    fnptr = PR_FindFunctionSymbol(library, name);
+    if (!fnptr) {
+      JS_ReportError(cx, "couldn't find function symbol in library");
+      return JS_FALSE;
+    }
+    data = &fnptr;
 
-  JSObject* fn = CData::Create(cx, typeObj, obj, &func, true);
-  if (!fn)
+  } else {
+    // 'typeObj' is another data type. Look up the data symbol.
+    data = PR_FindSymbol(library, name);
+    if (!data) {
+      JS_ReportError(cx, "couldn't find symbol in library");
+      return JS_FALSE;
+    }
+  }
+
+  JSObject* result = CData::Create(cx, typeObj, obj, data, isFunction);
+  if (!result)
     return JS_FALSE;
 
-  JS_SET_RVAL(cx, vp, OBJECT_TO_JSVAL(fn));
+  JS_SET_RVAL(cx, vp, OBJECT_TO_JSVAL(result));
 
   // Seal the CData object, to prevent modification of the function pointer.
   // This permanently associates this object with the library, and avoids
@@ -256,7 +296,10 @@ Library::Declare(JSContext* cx, uintN argc, jsval* vp)
   // change the pointer value.
   // XXX This will need to change when bug 541212 is fixed -- CData::ValueSetter
   // could be called on a sealed object.
-  return JS_SealObject(cx, fn, JS_FALSE);
+  if (isFunction && !JS_SealObject(cx, result, JS_FALSE))
+    return JS_FALSE;
+
+  return JS_TRUE;
 }
 
 }

toolkit/components/ctypes/tests/jsctypes-test.cpp

@@ -391,3 +391,6 @@ test_vector_add_va_cdecl(PRUint8 num_vecs,
   }
   return result;
 }
+
+RECT data_rect = { -1, -2, 3, 4 };
+

toolkit/components/ctypes/tests/jsctypes-test.h

@@ -202,4 +202,6 @@ NS_EXTERN_C
   EXPORT_CDECL(PRInt32*) test_vector_add_va_cdecl(PRUint8 num_vecs,
                                                   PRUint8 vec_len,
                                                   PRInt32* result, ...);
+
+  NS_EXPORT extern RECT data_rect;
 }

toolkit/components/ctypes/tests/unit/test_jsctypes.js.in

@@ -192,6 +192,7 @@ function run_test()
   run_function_tests(library);
   run_closure_tests(library);
   run_variadic_tests(library);
+  run_static_data_tests(library);
 
   // test library.close
   let test_void_t = library.declare("test_void_t_cdecl", ctypes.default_abi, ctypes.void_t);
@@ -2082,6 +2083,13 @@ function run_function_tests(library)
     "ctypes.FunctionType(ctypes.default_abi, ctypes.int32_t, [ctypes.char.ptr])");
   do_check_throws(function() { test_ansi_len.value = null; }, Error);
   do_check_eq(ptrValue(test_ansi_len), ptrValue(ptr));
+
+  // Test that the library.declare(name, functionType) form works.
+  let test_ansi_len_2 = library.declare("test_ansi_len", fn_t);
+  do_check_true(test_ansi_len_2.constructor === fn_t);
+  do_check_eq(ptrValue(test_ansi_len), ptrValue(test_ansi_len_2));
+  do_check_throws(function() { test_ansi_len_2.value = null; }, Error);
+  do_check_eq(ptrValue(test_ansi_len_2), ptrValue(ptr));
 }
 
 function run_closure_tests(library)
@@ -2211,6 +2219,42 @@ function run_variadic_tests(library) {
   }, Error);
 }
 
+function run_static_data_tests(library)
+{
+  const rect_t = new ctypes.StructType("RECT",
+                       [{ top   : ctypes.int32_t },
+                        { left  : ctypes.int32_t },
+                        { bottom: ctypes.int32_t },
+                        { right : ctypes.int32_t }]);
+
+  let data_rect = library.declare("data_rect", rect_t);
+
+  // Test reading static data.
+  do_check_true(data_rect.constructor === rect_t);
+  do_check_eq(data_rect.top, -1);
+  do_check_eq(data_rect.left, -2);
+  do_check_eq(data_rect.bottom, 3);
+  do_check_eq(data_rect.right, 4);
+
+  // Test writing.
+  data_rect.top = 9;
+  data_rect.left = 8;
+  data_rect.bottom = -11;
+  data_rect.right = -12;
+  do_check_eq(data_rect.top, 9);
+  do_check_eq(data_rect.left, 8);
+  do_check_eq(data_rect.bottom, -11);
+  do_check_eq(data_rect.right, -12);
+
+  // Make sure it's been written, not copied.
+  let data_rect_2 = library.declare("data_rect", rect_t);
+  do_check_eq(data_rect_2.top, 9);
+  do_check_eq(data_rect_2.left, 8);
+  do_check_eq(data_rect_2.bottom, -11);
+  do_check_eq(data_rect_2.right, -12);
+  do_check_eq(ptrValue(data_rect.address()), ptrValue(data_rect_2.address()));
+}
+
 // bug 522360 - try loading system library without full path
 function run_load_system_library()
 {