- Revision
- 142557
- Author
- oli...@apple.com
- Date
- 2013-02-11 17:20:57 -0800 (Mon, 11 Feb 2013)
Log Message
Make JSC API more NULL tolerant
https://bugs.webkit.org/show_bug.cgi?id=109515
Reviewed by Mark Hahnenberg.
We do so much marshalling for the C API these days anyway that a single null
check isn't a performance issue. Yet the existing "null is unsafe" behaviour
leads to crashes in embedding applications whenever there's an untested code
path, so it seems having defined behaviour is superior.
* API/APICast.h:
(toJS):
(toJSForGC):
* API/JSObjectRef.cpp:
(JSObjectIsFunction):
(JSObjectCallAsFunction):
(JSObjectIsConstructor):
(JSObjectCallAsConstructor):
* API/tests/testapi.c:
(main):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/API/APICast.h (142556 => 142557)
--- trunk/Source/_javascript_Core/API/APICast.h 2013-02-12 01:09:46 UTC (rev 142556)
+++ trunk/Source/_javascript_Core/API/APICast.h 2013-02-12 01:20:57 UTC (rev 142557)
@@ -63,23 +63,24 @@
inline JSC::JSValue toJS(JSC::ExecState* exec, JSValueRef v)
{
ASSERT_UNUSED(exec, exec);
- ASSERT(v);
#if USE(JSVALUE32_64)
JSC::JSCell* jsCell = reinterpret_cast<JSC::JSCell*>(const_cast<OpaqueJSValue*>(v));
if (!jsCell)
- return JSC::JSValue();
+ return JSC::jsNull();
if (jsCell->isAPIValueWrapper())
return JSC::jsCast<JSC::JSAPIValueWrapper*>(jsCell)->value();
return jsCell;
#else
- return JSC::JSValue::decode(reinterpret_cast<JSC::EncodedJSValue>(const_cast<OpaqueJSValue*>(v)));
+ JSC::JSValue result = JSC::JSValue::decode(reinterpret_cast<JSC::EncodedJSValue>(const_cast<OpaqueJSValue*>(v)));
+ if (!result)
+ return JSC::jsNull();
+ return result;
#endif
}
inline JSC::JSValue toJSForGC(JSC::ExecState* exec, JSValueRef v)
{
ASSERT_UNUSED(exec, exec);
- ASSERT(v);
#if USE(JSVALUE32_64)
JSC::JSCell* jsCell = reinterpret_cast<JSC::JSCell*>(const_cast<OpaqueJSValue*>(v));
if (!jsCell)
Modified: trunk/Source/_javascript_Core/API/JSObjectRef.cpp (142556 => 142557)
--- trunk/Source/_javascript_Core/API/JSObjectRef.cpp 2013-02-12 01:09:46 UTC (rev 142556)
+++ trunk/Source/_javascript_Core/API/JSObjectRef.cpp 2013-02-12 01:20:57 UTC (rev 142557)
@@ -416,6 +416,8 @@
bool JSObjectIsFunction(JSContextRef, JSObjectRef object)
{
+ if (!object)
+ return false;
CallData callData;
JSCell* cell = toJS(object);
return cell->methodTable()->getCallData(cell, callData) != CallTypeNone;
@@ -426,6 +428,9 @@
ExecState* exec = toJS(ctx);
APIEntryShim entryShim(exec);
+ if (!object)
+ return 0;
+
JSObject* jsObject = toJS(object);
JSObject* jsThisObject = toJS(thisObject);
@@ -455,6 +460,8 @@
bool JSObjectIsConstructor(JSContextRef, JSObjectRef object)
{
+ if (!object)
+ return false;
JSObject* jsObject = toJS(object);
ConstructData constructData;
return jsObject->methodTable()->getConstructData(jsObject, constructData) != ConstructTypeNone;
@@ -465,6 +472,9 @@
ExecState* exec = toJS(ctx);
APIEntryShim entryShim(exec);
+ if (!object)
+ return 0;
+
JSObject* jsObject = toJS(object);
ConstructData constructData;
Modified: trunk/Source/_javascript_Core/API/tests/testapi.c (142556 => 142557)
--- trunk/Source/_javascript_Core/API/tests/testapi.c 2013-02-12 01:09:46 UTC (rev 142556)
+++ trunk/Source/_javascript_Core/API/tests/testapi.c 2013-02-12 01:20:57 UTC (rev 142557)
@@ -1128,6 +1128,7 @@
free(buffer);
JSValueRef jsCFEmptyStringWithCharacters = JSValueMakeString(context, jsCFEmptyIStringWithCharacters);
+ ASSERT(JSValueGetType(context, NULL) == kJSTypeNull);
ASSERT(JSValueGetType(context, jsUndefined) == kJSTypeUndefined);
ASSERT(JSValueGetType(context, jsNull) == kJSTypeNull);
ASSERT(JSValueGetType(context, jsTrue) == kJSTypeBoolean);
@@ -1142,6 +1143,17 @@
ASSERT(JSValueGetType(context, jsCFEmptyString) == kJSTypeString);
ASSERT(JSValueGetType(context, jsCFEmptyStringWithCharacters) == kJSTypeString);
+ ASSERT(!JSValueIsBoolean(context, NULL));
+ ASSERT(!JSValueIsObject(context, NULL));
+ ASSERT(!JSValueIsString(context, NULL));
+ ASSERT(!JSValueIsNumber(context, NULL));
+ ASSERT(!JSValueIsUndefined(context, NULL));
+ ASSERT(JSValueIsNull(context, NULL));
+ ASSERT(!JSObjectCallAsFunction(context, NULL, NULL, 0, NULL, NULL));
+ ASSERT(!JSObjectCallAsConstructor(context, NULL, 0, NULL, NULL));
+ ASSERT(!JSObjectIsConstructor(context, NULL));
+ ASSERT(!JSObjectIsFunction(context, NULL));
+
JSStringRef nullString = JSStringCreateWithUTF8CString(0);
const JSChar* characters = JSStringGetCharactersPtr(nullString);
if (characters) {
Modified: trunk/Source/_javascript_Core/ChangeLog (142556 => 142557)
--- trunk/Source/_javascript_Core/ChangeLog 2013-02-12 01:09:46 UTC (rev 142556)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-02-12 01:20:57 UTC (rev 142557)
@@ -1,3 +1,26 @@
+2013-02-11 Oliver Hunt <oli...@apple.com>
+
+ Make JSC API more NULL tolerant
+ https://bugs.webkit.org/show_bug.cgi?id=109515
+
+ Reviewed by Mark Hahnenberg.
+
+ We do so much marshalling for the C API these days anyway that a single null
+ check isn't a performance issue. Yet the existing "null is unsafe" behaviour
+ leads to crashes in embedding applications whenever there's an untested code
+ path, so it seems having defined behaviour is superior.
+
+ * API/APICast.h:
+ (toJS):
+ (toJSForGC):
+ * API/JSObjectRef.cpp:
+ (JSObjectIsFunction):
+ (JSObjectCallAsFunction):
+ (JSObjectIsConstructor):
+ (JSObjectCallAsConstructor):
+ * API/tests/testapi.c:
+ (main):
+
2013-02-11 Filip Pizlo <fpi...@apple.com>
Unreviewed, adding a FIXME to remind ourselves of a bug.
