Diff
Modified: trunk/Source/_javascript_Core/API/APICallbackFunction.h (260342 => 260343)
--- trunk/Source/_javascript_Core/API/APICallbackFunction.h 2020-04-19 18:34:07 UTC (rev 260342)
+++ trunk/Source/_javascript_Core/API/APICallbackFunction.h 2020-04-19 21:18:07 UTC (rev 260343)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -62,8 +62,10 @@
JSLock::DropAllLocks dropAllLocks(globalObject);
result = jsCast<T*>(toJS(functionRef))->functionCallback()(execRef, functionRef, thisObjRef, argumentCount, arguments.data(), &exception);
}
- if (exception)
+ if (exception) {
throwException(globalObject, scope, toJS(globalObject, exception));
+ return JSValue::encode(jsUndefined());
+ }
// result must be a valid JSValue.
if (!result)
@@ -97,7 +99,7 @@
}
if (exception) {
throwException(globalObject, scope, toJS(globalObject, exception));
- return JSValue::encode(toJS(globalObject, exception));
+ return JSValue::encode(jsUndefined());
}
// result must be a valid JSValue.
if (!result)
Modified: trunk/Source/_javascript_Core/API/JSCallbackObjectFunctions.h (260342 => 260343)
--- trunk/Source/_javascript_Core/API/JSCallbackObjectFunctions.h 2020-04-19 18:34:07 UTC (rev 260342)
+++ trunk/Source/_javascript_Core/API/JSCallbackObjectFunctions.h 2020-04-19 21:18:07 UTC (rev 260343)
@@ -465,8 +465,10 @@
JSLock::DropAllLocks dropAllLocks(globalObject);
result = toJS(callAsConstructor(execRef, constructorRef, argumentCount, arguments.data(), &exception));
}
- if (exception)
+ if (exception) {
throwException(globalObject, scope, toJS(globalObject, exception));
+ return JSValue::encode(jsUndefined());
+ }
return JSValue::encode(result);
}
}
@@ -538,8 +540,10 @@
JSLock::DropAllLocks dropAllLocks(globalObject);
result = toJS(globalObject, callAsFunction(execRef, functionRef, thisObjRef, argumentCount, arguments.data(), &exception));
}
- if (exception)
+ if (exception) {
throwException(globalObject, scope, toJS(globalObject, exception));
+ return JSValue::encode(jsUndefined());
+ }
return JSValue::encode(result);
}
}
Modified: trunk/Source/_javascript_Core/API/JSObjectRef.cpp (260342 => 260343)
--- trunk/Source/_javascript_Core/API/JSObjectRef.cpp 2020-04-19 18:34:07 UTC (rev 260342)
+++ trunk/Source/_javascript_Core/API/JSObjectRef.cpp 2020-04-19 21:18:07 UTC (rev 260343)
@@ -348,7 +348,7 @@
{
if (!ctx || !object) {
ASSERT_NOT_REACHED();
- return 0;
+ return nullptr;
}
JSGlobalObject* globalObject = toJS(ctx);
VM& vm = globalObject->vm();
@@ -358,7 +358,8 @@
JSObject* jsObject = toJS(object);
JSValue jsValue = jsObject->get(globalObject, propertyName->identifier(&vm));
- handleExceptionIfNeeded(scope, ctx, exception);
+ if (handleExceptionIfNeeded(scope, ctx, exception) == ExceptionStatus::DidThrow)
+ return nullptr;
return toRef(globalObject, jsValue);
}
@@ -407,7 +408,8 @@
return false;
bool result = jsObject->hasProperty(globalObject, ident);
- handleExceptionIfNeeded(scope, ctx, exception);
+ if (handleExceptionIfNeeded(scope, ctx, exception) == ExceptionStatus::DidThrow)
+ return false;
return result;
}
@@ -428,7 +430,8 @@
return nullptr;
JSValue jsValue = jsObject->get(globalObject, ident);
- handleExceptionIfNeeded(scope, ctx, exception);
+ if (handleExceptionIfNeeded(scope, ctx, exception) == ExceptionStatus::DidThrow)
+ return nullptr;
return toRef(globalObject, jsValue);
}
@@ -480,7 +483,8 @@
return false;
bool result = JSCell::deleteProperty(jsObject, globalObject, ident);
- handleExceptionIfNeeded(scope, ctx, exception);
+ if (handleExceptionIfNeeded(scope, ctx, exception) == ExceptionStatus::DidThrow)
+ return false;
return result;
}
@@ -488,7 +492,7 @@
{
if (!ctx) {
ASSERT_NOT_REACHED();
- return 0;
+ return nullptr;
}
JSGlobalObject* globalObject = toJS(ctx);
VM& vm = globalObject->vm();
@@ -498,7 +502,8 @@
JSObject* jsObject = toJS(object);
JSValue jsValue = jsObject->get(globalObject, propertyIndex);
- handleExceptionIfNeeded(scope, ctx, exception);
+ if (handleExceptionIfNeeded(scope, ctx, exception) == ExceptionStatus::DidThrow)
+ return nullptr;
return toRef(globalObject, jsValue);
}
@@ -535,7 +540,8 @@
JSObject* jsObject = toJS(object);
bool result = JSCell::deleteProperty(jsObject, globalObject, propertyName->identifier(&vm));
- handleExceptionIfNeeded(scope, ctx, exception);
+ if (handleExceptionIfNeeded(scope, ctx, exception) == ExceptionStatus::DidThrow)
+ return false;
return result;
}
Modified: trunk/Source/_javascript_Core/API/JSValueRef.cpp (260342 => 260343)
--- trunk/Source/_javascript_Core/API/JSValueRef.cpp 2020-04-19 18:34:07 UTC (rev 260342)
+++ trunk/Source/_javascript_Core/API/JSValueRef.cpp 2020-04-19 21:18:07 UTC (rev 260343)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2006-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2006-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -235,7 +235,8 @@
JSValue jsB = toJS(globalObject, b);
bool result = JSValue::equal(globalObject, jsA, jsB); // false if an exception is thrown
- handleExceptionIfNeeded(scope, ctx, exception);
+ if (handleExceptionIfNeeded(scope, ctx, exception) == ExceptionStatus::DidThrow)
+ return false;
return result;
}
@@ -272,7 +273,8 @@
if (!jsConstructor->structure(vm)->typeInfo().implementsHasInstance())
return false;
bool result = jsConstructor->hasInstance(globalObject, jsValue); // false if an exception is thrown
- handleExceptionIfNeeded(scope, ctx, exception);
+ if (handleExceptionIfNeeded(scope, ctx, exception) == ExceptionStatus::DidThrow)
+ return false;
return result;
}
Modified: trunk/Source/_javascript_Core/API/ObjCCallbackFunction.mm (260342 => 260343)
--- trunk/Source/_javascript_Core/API/ObjCCallbackFunction.mm 2020-04-19 18:34:07 UTC (rev 260342)
+++ trunk/Source/_javascript_Core/API/ObjCCallbackFunction.mm 2020-04-19 21:18:07 UTC (rev 260343)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -67,7 +67,10 @@
class CallbackArgumentInteger : public CallbackArgument {
void set(NSInvocation *invocation, NSInteger argumentNumber, JSContext *context, JSValueRef argument, JSValueRef* exception) override
{
+ ASSERT(exception && !*exception);
T value = (T)JSC::toInt32(JSValueToNumber([context JSGlobalContextRef], argument, exception));
+ if (*exception)
+ return;
[invocation setArgument:&value atIndex:argumentNumber];
}
};
@@ -76,7 +79,10 @@
class CallbackArgumentDouble : public CallbackArgument {
void set(NSInvocation *invocation, NSInteger argumentNumber, JSContext *context, JSValueRef argument, JSValueRef* exception) override
{
+ ASSERT(exception && !*exception);
T value = (T)JSValueToNumber([context JSGlobalContextRef], argument, exception);
+ if (*exception)
+ return;
[invocation setArgument:&value atIndex:argumentNumber];
}
};
@@ -107,6 +113,7 @@
private:
void set(NSInvocation *invocation, NSInteger argumentNumber, JSContext *context, JSValueRef argument, JSValueRef* exception) override
{
+ ASSERT(exception && !*exception);
JSGlobalContextRef contextRef = [context JSGlobalContextRef];
id object = tryUnwrapObjcObject(contextRef, argument);
@@ -130,7 +137,10 @@
class CallbackArgumentNSNumber : public CallbackArgument {
void set(NSInvocation *invocation, NSInteger argumentNumber, JSContext *context, JSValueRef argument, JSValueRef* exception) override
{
+ ASSERT(exception && !*exception);
id value = valueToNumber([context JSGlobalContextRef], argument, exception);
+ if (*exception)
+ return;
[invocation setArgument:&value atIndex:argumentNumber];
}
};
@@ -138,7 +148,10 @@
class CallbackArgumentNSString : public CallbackArgument {
void set(NSInvocation *invocation, NSInteger argumentNumber, JSContext *context, JSValueRef argument, JSValueRef* exception) override
{
+ ASSERT(exception && !*exception);
id value = valueToString([context JSGlobalContextRef], argument, exception);
+ if (*exception)
+ return;
[invocation setArgument:&value atIndex:argumentNumber];
}
};
@@ -146,7 +159,10 @@
class CallbackArgumentNSDate : public CallbackArgument {
void set(NSInvocation *invocation, NSInteger argumentNumber, JSContext *context, JSValueRef argument, JSValueRef* exception) override
{
+ ASSERT(exception && !*exception);
id value = valueToDate([context JSGlobalContextRef], argument, exception);
+ if (*exception)
+ return;
[invocation setArgument:&value atIndex:argumentNumber];
}
};
@@ -154,7 +170,10 @@
class CallbackArgumentNSArray : public CallbackArgument {
void set(NSInvocation *invocation, NSInteger argumentNumber, JSContext *context, JSValueRef argument, JSValueRef* exception) override
{
+ ASSERT(exception && !*exception);
id value = valueToArray([context JSGlobalContextRef], argument, exception);
+ if (*exception)
+ return;
[invocation setArgument:&value atIndex:argumentNumber];
}
};
@@ -162,7 +181,10 @@
class CallbackArgumentNSDictionary : public CallbackArgument {
void set(NSInvocation *invocation, NSInteger argumentNumber, JSContext *context, JSValueRef argument, JSValueRef* exception) override
{
+ ASSERT(exception && !*exception);
id value = valueToDictionary([context JSGlobalContextRef], argument, exception);
+ if (*exception)
+ return;
[invocation setArgument:&value atIndex:argumentNumber];
}
};
@@ -447,6 +469,8 @@
static JSValueRef objCCallbackFunctionCallAsFunction(JSContextRef callerContext, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
{
+ ASSERT(exception && !*exception);
+
// Retake the API lock - we need this for a few reasons:
// (1) We don't want to support the C-API's confusing drops-locks-once policy - should only drop locks if we can do so recursively.
// (2) We're calling some JSC internals that require us to be on the 'inside' - e.g. createTypeError.
@@ -460,6 +484,8 @@
if (impl->type() == CallbackInitMethod) {
JSGlobalContextRef contextRef = [context JSGlobalContextRef];
*exception = toRef(JSC::createTypeError(toJS(contextRef), "Cannot call a class constructor without |new|"_s));
+ if (*exception)
+ return nullptr;
return JSValueMakeUndefined(contextRef);
}
@@ -472,11 +498,14 @@
*exception = valueInternalValue(context.exception);
[context endCallbackWithData:&callbackData];
}
+ if (*exception)
+ return nullptr;
return result;
}
static JSObjectRef objCCallbackFunctionCallAsConstructor(JSContextRef callerContext, JSObjectRef constructor, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
{
+ ASSERT(exception && !*exception);
JSC::JSLockHolder locker(toJS(callerContext));
ObjCCallbackFunction* callback = static_cast<ObjCCallbackFunction*>(toJS(constructor));
@@ -492,15 +521,15 @@
*exception = valueInternalValue(context.exception);
[context endCallbackWithData:&callbackData];
}
-
- JSGlobalContextRef contextRef = [context JSGlobalContextRef];
if (*exception)
return nullptr;
+ JSGlobalContextRef contextRef = [context JSGlobalContextRef];
if (!JSValueIsObject(contextRef, result)) {
*exception = toRef(JSC::createTypeError(toJS(contextRef), "Objective-C blocks called as constructors must return an object."_s));
return nullptr;
}
+ ASSERT(!*exception);
return const_cast<JSObjectRef>(result);
}
@@ -540,6 +569,7 @@
JSValueRef ObjCCallbackFunctionImpl::call(JSContext *context, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
{
+ ASSERT(exception && !*exception);
JSGlobalContextRef contextRef = [context JSGlobalContextRef];
id target;
@@ -550,6 +580,8 @@
target = [m_instanceClass alloc];
if (!target || ![target isKindOfClass:m_instanceClass.get()]) {
*exception = toRef(JSC::createTypeError(toJS(contextRef), "self type check failed for Objective-C instance method"_s));
+ if (*exception)
+ return nullptr;
return JSValueMakeUndefined(contextRef);
}
[m_invocation setTarget:target];
@@ -560,6 +592,8 @@
target = tryUnwrapObjcObject(contextRef, thisObject);
if (!target || ![target isKindOfClass:m_instanceClass.get()]) {
*exception = toRef(JSC::createTypeError(toJS(contextRef), "self type check failed for Objective-C instance method"_s));
+ if (*exception)
+ return nullptr;
return JSValueMakeUndefined(contextRef);
}
[m_invocation setTarget:target];
@@ -578,7 +612,7 @@
JSValueRef value = argumentNumber < argumentCount ? arguments[argumentNumber] : JSValueMakeUndefined(contextRef);
argument->set(m_invocation.get(), argumentNumber + firstArgument, context, value, exception);
if (*exception)
- return JSValueMakeUndefined(contextRef);
+ return nullptr;
++argumentNumber;
}
@@ -585,6 +619,8 @@
[m_invocation invoke];
JSValueRef result = m_result->get(m_invocation.get(), context, exception);
+ if (*exception)
+ return nullptr;
// Balance our call to -alloc with a call to -autorelease. We have to do this after calling -init
// because init family methods are allowed to release the allocated object and return something
Modified: trunk/Source/_javascript_Core/ChangeLog (260342 => 260343)
--- trunk/Source/_javascript_Core/ChangeLog 2020-04-19 18:34:07 UTC (rev 260342)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-04-19 21:18:07 UTC (rev 260343)
@@ -1,3 +1,47 @@
+2020-04-19 Mark Lam <[email protected]>
+
+ Fix missing exception checks and handling in JSC APIs.
+ https://bugs.webkit.org/show_bug.cgi?id=210715
+ <rdar://problem/61599658>
+
+ Reviewed by Saam Barati.
+
+ * API/APICallbackFunction.h:
+ (JSC::APICallbackFunction::call):
+ - We should return early if an exception was thrown. We should not be using the
+ result in any way since we cannot rely on it having any sane value.
+ (JSC::APICallbackFunction::construct):
+ - For consistency, also return an undefined here when an exception was thrown.
+
+ * API/JSCallbackObjectFunctions.h:
+ (JSC::JSCallbackObject<Parent>::construct):
+ (JSC::JSCallbackObject<Parent>::call):
+ - Return an undefined if an exception was thrown. Don't return the potentially
+ garbage result value. Who knows what the client code will do with it. Returning
+ an undefined here makes the code more robust.
+
+ * API/JSObjectRef.cpp:
+ (JSObjectGetProperty):
+ (JSObjectHasPropertyForKey):
+ (JSObjectGetPropertyForKey):
+ (JSObjectDeletePropertyForKey):
+ (JSObjectGetPropertyAtIndex):
+ (JSObjectDeleteProperty):
+ - Explicitly return a nullptr if an exception was thrown. The toRef() on the
+ result that follows the exception check may or may not return a nullptr
+ (also see toRef(JSC::VM& vm, JSC::JSValue v) for !CPU(ADDRESS64)).
+
+ * API/JSValueRef.cpp:
+ (JSValueIsEqual):
+ (JSValueIsInstanceOfConstructor):
+ - For consistency, make these return false if an exception is thrown.
+
+ * API/ObjCCallbackFunction.mm:
+ (JSC::objCCallbackFunctionCallAsFunction):
+ (JSC::objCCallbackFunctionCallAsConstructor):
+ (JSC::ObjCCallbackFunctionImpl::call):
+ - Add some assertions and return early if an exception was thrown.
+
2020-04-18 Keith Miller <[email protected]>
Fix CLoop build for iterator opcodes
