Changeset 197794 in webkit for trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp

Timestamp:

Mar 8, 2016, 12:57:25 PM (9 years ago)

Author:

[email protected]

Message:

synthesizePrototype() and friends need to be followed by exception checks (or equivalent).
​https://bugs.webkit.org/show_bug.cgi?id=155169

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

With the exception checks, we may end up throwing new exceptions over an existing
one that has been thrown but not handled yet, thereby obscuring it. It may also
mean that the VM will continue running on potentially unstable state, which may
have undesirable consequences.

I first observed this in some failed assertion while running tests on a patch for
​https://bugs.webkit.org/show_bug.cgi?id=154865.

Performance is neutral with this patch (tested on x86_64).

1. Deleted JSNotAnObject, and removed all uses of it.

2. Added exception checks, when needed, following calls to synthesizePrototype() and JSValue::toObject().

The cases that do not need an exception check are the ones that already ensures
that JSValue::toObject() is only called on a value that is convertible to an
object. In those cases, I added an assertion that no exception was thrown
after the call.

• CMakeLists.txt:
• JavaScriptCore.xcodeproj/project.pbxproj:
• inspector/ScriptCallStackFactory.cpp:

(Inspector::createScriptCallStackFromException):

• interpreter/Interpreter.cpp:
• jit/JITOperations.cpp:
• llint/LLIntSlowPaths.cpp:

(JSC::LLInt::LLINT_SLOW_PATH_DECL):

• runtime/ArrayPrototype.cpp:

(JSC::arrayProtoFuncJoin):
(JSC::arrayProtoFuncConcat):
(JSC::arrayProtoFuncPop):
(JSC::arrayProtoFuncPush):
(JSC::arrayProtoFuncReverse):
(JSC::arrayProtoFuncShift):
(JSC::arrayProtoFuncSlice):
(JSC::arrayProtoFuncSplice):
(JSC::arrayProtoFuncUnShift):
(JSC::arrayProtoFuncIndexOf):
(JSC::arrayProtoFuncLastIndexOf):
(JSC::arrayProtoFuncValues):
(JSC::arrayProtoFuncEntries):
(JSC::arrayProtoFuncKeys):

• runtime/CommonSlowPaths.cpp:

(JSC::SLOW_PATH_DECL):

• runtime/ExceptionHelpers.cpp:
• runtime/JSCJSValue.cpp:

(JSC::JSValue::toObjectSlowCase):
(JSC::JSValue::toThisSlowCase):
(JSC::JSValue::synthesizePrototype):
(JSC::JSValue::putToPrimitive):
(JSC::JSValue::putToPrimitiveByIndex):

• runtime/JSCJSValueInlines.h:

(JSC::JSValue::getPropertySlot):
(JSC::JSValue::get):

• runtime/JSFunction.cpp:
• runtime/JSGlobalObjectFunctions.cpp:

(JSC::globalFuncProtoGetter):

• runtime/JSNotAnObject.cpp: Removed.
• runtime/JSNotAnObject.h: Removed.
• runtime/ObjectConstructor.cpp:

(JSC::objectConstructorDefineProperties):
(JSC::objectConstructorCreate):

• runtime/ObjectPrototype.cpp:

(JSC::objectProtoFuncValueOf):
(JSC::objectProtoFuncHasOwnProperty):
(JSC::objectProtoFuncIsPrototypeOf):
(JSC::objectProtoFuncToString):

• runtime/VM.cpp:

(JSC::VM::VM):

• runtime/VM.h:

Source/WebCore:

No new tests because this issue is covered by existing tests when the fix for
​https://bugs.webkit.org/show_bug.cgi?id=154865 lands. That patch is waiting for
this patch to land first so as to not introduce test failures.

• Modules/plugins/QuickTimePluginReplacement.mm:

(WebCore::QuickTimePluginReplacement::installReplacement):

• bindings/js/JSDeviceMotionEventCustom.cpp:

(WebCore::readAccelerationArgument):
(WebCore::readRotationRateArgument):

• bindings/js/JSGeolocationCustom.cpp:

(WebCore::createPositionOptions):

• bindings/js/JSHTMLCanvasElementCustom.cpp:

(WebCore::get3DContextAttributes):

• bindings/scripts/CodeGeneratorJS.pm:

(GenerateConstructorDefinition):

• bindings/scripts/test/JS/JSTestEventConstructor.cpp:

(WebCore::JSTestEventConstructorConstructor::construct):

• contentextensions/ContentExtensionParser.cpp:

(WebCore::ContentExtensions::getTypeFlags):

• html/HTMLMediaElement.cpp:

(WebCore::setPageScaleFactorProperty):
(WebCore::HTMLMediaElement::didAddUserAgentShadowRoot):
(WebCore::HTMLMediaElement::getCurrentMediaControlsStatus):

• html/HTMLPlugInImageElement.cpp:

(WebCore::HTMLPlugInImageElement::didAddUserAgentShadowRoot):

File:

• trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp (modified) (5 diffs)

trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp

@@ -550 +550 @@
     JSValue baseValue = OP_C(2).jsValue();
     JSObject* baseObject = baseValue.toObject(exec);
+    CHECK_EXCEPTION();
     
     JSValue subscript = OP_C(3).jsValue();
@@ -607 +608 @@
     BEGIN();
     JSObject* base = OP(2).jsValue().toObject(exec);
+    CHECK_EXCEPTION();
     JSValue property = OP(3).jsValue();
     pc[4].u.arrayProfile->observeStructure(base->structure(vm));
@@ -617 +619 @@
     BEGIN();
     JSObject* base = OP(2).jsValue().toObject(exec);
+    CHECK_EXCEPTION();
     JSValue property = OP(3).jsValue();
     ASSERT(property.isString());
@@ -629 +632 @@
     BEGIN();
     JSObject* base = OP(2).jsValue().toObject(exec);
+    CHECK_EXCEPTION();
     JSValue property = OP(3).jsValue();
     bool result;
@@ -657 +661 @@
 
     JSObject* base = baseValue.toObject(exec);
+    CHECK_EXCEPTION();
 
     RETURN(propertyNameEnumerator(exec, base));