Changeset 230102 in webkit for trunk/Source/JavaScriptCore/runtime/Lookup.cpp

Timestamp:

Mar 30, 2018, 5:39:43 AM (7 years ago)

Author:

[email protected]

Message:

A stack overflow in the parsing of a builtin (called by createExecutable) cause a crash instead of a catchable js exception
​https://bugs.webkit.org/show_bug.cgi?id=184074
<rdar://problem/37165897>

Reviewed by Keith Miller.

JSTests:

• stress/stack-overflow-while-parsing-builtin.js: Added.

(f):

Source/JavaScriptCore:

Fixing this requires getting the ParserError (with information about the failure) and an ExecState* (to throw an exception) in the same place.
It is surprisingly painful, with quite a long call stack between the last function with an access to an ExecState* and the first function with the ParserError.
Even worse, many of these functions are generated by macros, themselves generated by a maze of python scripts.
As a result, this patch is grotesquely large, while all it does is adding enough plumbing to throw a proper exception in this specific case.

There are now bare calls to '.value()' on several paths that may crash. It is not a problem in my opinion, since we previously crashed in every case regardless of the path that took us to createExecutable when encountering a stack overflow.
If we ever find an example that can cause these calls to fail, it should be doable to throw a proper exception there too.

Two other minor changes:

• I removed BuiltinExecutableCreator.{cpp, h} as it was nearly empty, and only used in one place. That place now includes BuiltinExecutables.h directly instead.
• I moved code from ParserError.h into a newly created ParserError.cpp, as I see no need to inline functions that are only used when encountering a parser error, and ParserError.h is now included in quite a few places.

• JavaScriptCore.xcodeproj/project.pbxproj:
• Scripts/builtins/builtins_generate_combined_header.py:

(BuiltinsCombinedHeaderGenerator.generate_forward_declarations):
(ParserError):
(generate_section_for_object): Deleted.
(generate_externs_for_object): Deleted.
(generate_macros_for_object): Deleted.
(generate_section_for_code_table_macro): Deleted.
(generate_section_for_code_name_macro): Deleted.
(generate_section_for_global_private_code_name_macro): Deleted.

• Scripts/builtins/builtins_generate_separate_header.py:

(generate_secondary_header_includes):

• Scripts/builtins/builtins_templates.py:
• Sources.txt:
• builtins/BuiltinExecutableCreator.cpp: Removed.
• builtins/BuiltinExecutableCreator.h: Removed.
• builtins/BuiltinExecutables.cpp:

(JSC::BuiltinExecutables::createDefaultConstructor):
(JSC::BuiltinExecutables::createBuiltinExecutable):
(JSC::createBuiltinExecutable):
(JSC::BuiltinExecutables::createExecutableOrCrash):
(JSC::BuiltinExecutables::createExecutable):

• builtins/BuiltinExecutables.h:
• bytecompiler/BytecodeGenerator.h:
• parser/ParserError.cpp: Added.

(JSC::ParserError::toErrorObject):
(JSC::ParserError::throwStackOverflowOrOutOfMemory):
(WTF::printInternal):

• parser/ParserError.h:

(JSC::ParserError::toErrorObject): Deleted.
(WTF::printInternal): Deleted.

• runtime/AsyncIteratorPrototype.cpp:

(JSC::AsyncIteratorPrototype::finishCreation):

• runtime/FunctionPrototype.cpp:

(JSC::FunctionPrototype::addFunctionProperties):

• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):

• runtime/JSObject.cpp:

(JSC::JSObject::getOwnStaticPropertySlot):
(JSC::JSObject::reifyAllStaticProperties):

• runtime/JSObject.h:

(JSC::JSObject::getOwnNonIndexPropertySlot):
(JSC::JSObject::getOwnPropertySlot):
(JSC::JSObject::getPropertySlot):

• runtime/JSObjectInlines.h:

(JSC::JSObject::getNonIndexPropertySlot):

• runtime/JSTypedArrayViewPrototype.cpp:

(JSC::JSTypedArrayViewPrototype::finishCreation):

• runtime/Lookup.cpp:

(JSC::reifyStaticAccessor):
(JSC::setUpStaticFunctionSlot):

• runtime/Lookup.h:

(JSC::getStaticPropertySlotFromTable):
(JSC::reifyStaticProperty):

• runtime/MapPrototype.cpp:

(JSC::MapPrototype::finishCreation):

• runtime/SetPrototype.cpp:

(JSC::SetPrototype::finishCreation):

• tools/JSDollarVM.cpp:

(JSC::functionCreateBuiltin):

Source/WebCore:

I had to slightly change the type of some bindings between JSC and WebCore. No functional change intended on the WebCore side.

• bindings/js/JSReadableStreamPrivateConstructors.cpp:

(WebCore::JSBuiltinReadableStreamDefaultReaderPrivateConstructor::initializeExecutable):
(WebCore::JSBuiltinReadableStreamDefaultControllerPrivateConstructor::initializeExecutable):
(WebCore::JSBuiltinReadableByteStreamControllerPrivateConstructor::initializeExecutable):
(WebCore::JSBuiltinReadableStreamBYOBReaderPrivateConstructor::initializeExecutable):
(WebCore::JSBuiltinReadableStreamBYOBRequestPrivateConstructor::initializeExecutable):

• bindings/scripts/CodeGeneratorJS.pm:

(GenerateConstructorHelperMethods):

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

(WebCore::JSTestClassWithJSBuiltinConstructorConstructor::initializeExecutable):

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

(WebCore::JSTestJSBuiltinConstructorConstructor::initializeExecutable):

File:

• trunk/Source/JavaScriptCore/runtime/Lookup.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/runtime/Lookup.cpp

@@ -34 +34 @@
         JSFunction* function = nullptr;
         if (value.attributes() & PropertyAttribute::Builtin)
-            function = JSFunction::create(vm, value.builtinAccessorGetterGenerator()(vm), globalObject);
+            function = JSFunction::create(vm, value.builtinAccessorGetterGenerator()(vm).value(), globalObject);
         else {
             String getterName = tryMakeString(ASCIILiteral("get "), String(*propertyName.publicName()));
@@ -46 +46 @@
 }
 
-bool setUpStaticFunctionSlot(VM& vm, const ClassInfo* classInfo, const HashTableValue* entry, JSObject* thisObject, PropertyName propertyName, PropertySlot& slot)
+bool setUpStaticFunctionSlot(ExecState* exec, const ClassInfo* classInfo, const HashTableValue* entry, JSObject* thisObject, PropertyName propertyName, PropertySlot& slot)
 {
+    VM& vm = exec->vm();
+    auto scope = DECLARE_THROW_SCOPE(vm);
     ASSERT(thisObject->globalObject());
     ASSERT(entry->attributes() & PropertyAttribute::BuiltinOrFunctionOrAccessorOrLazyProperty);
@@ -60 +62 @@
             return false;
 
-        reifyStaticProperty(vm, classInfo, propertyName, *entry, *thisObject);
+        reifyStaticProperty(vm, exec, classInfo, propertyName, *entry, *thisObject);
+        RETURN_IF_EXCEPTION(scope, false);
 
         offset = thisObject->getDirectOffset(vm, propertyName, attributes);