Attachment 251101 Details for Bug 143923 – Patch

[patch] Patch

bug-143923-20150419075823.patch (text/plain), 7.72 KB, created by Yusuke Suzuki on 2015-04-18 15:59:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2015-04-18 15:59:24 PDT

Size: 7.72 KB

patch

obsolete

>Subversion Revision: 182985
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 9f816f890cff1feb93bdc2c684b822bd4e05fdf8..0fda84e4f154e099c50c5d03a90df354b81aaacf 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2015-04-18  Yusuke Suzuki  <utatane.tea@gmail.com>
>+
>+        Make Builtin functions non constructible
>+        https://bugs.webkit.org/show_bug.cgi?id=143923
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Builtin functions defined by builtins/*.js accidentally have [[Construct]].
>+        According to the spec, these functions except for explicitly defined as a constructor do not have [[Construct]].
>+        This patch fixes it. When the JS function used for a construction is builtin function, throw not a constructor error.
>+
>+        Ideally, returning ConstructTypeNone in JSFunction::getConstructData is enough.
>+        However, to avoid calling getConstructData (it involves indirect call of function pointer of getConstructData), some places do not check ConstructType.
>+        In these places, they only check the target function is JSFunction because previously JSFunction always has [[Construct]].
>+        So in this patch, we check `isBuiltinFunction()` in those places.
>+
>+        * dfg/DFGByteCodeParser.cpp:
>+        (JSC::DFG::ByteCodeParser::inliningCost):
>+        * jit/JITOperations.cpp:
>+        * llint/LLIntSlowPaths.cpp:
>+        (JSC::LLInt::setUpCall):
>+        * runtime/JSFunction.cpp:
>+        (JSC::JSFunction::getConstructData):
>+        * tests/stress/builtin-function-is-construct-type-none.js: Added.
>+        (shouldThrow):
>+
> 2015-04-17  Alexey Proskuryakov  <ap@apple.com>
> 
>         Remove unused BoundsCheckedPointer
>diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>index d973885c1ca32a3c6e2060f901d72a281b187b78..970db1582631170b6f952bc68bea31d813ae58fb 100644
>--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
>@@ -1208,11 +1208,12 @@ unsigned ByteCodeParser::inliningCost(CallVariant callee, int argumentCountInclu
>     }
>     
>     // Do we have a code block, and does the code block's size match the heuristics/requirements for
>-    // being an inline candidate? We might not have a code block if code was thrown away or if we
>-    // simply hadn't actually made this call yet. We could still theoretically attempt to inline it
>-    // if we had a static proof of what was being called; this might happen for example if you call a
>-    // global function, where watchpointing gives us static information. Overall, it's a rare case
>-    // because we expect that any hot callees would have already been compiled.
>+    // being an inline candidate? We might not have a code block (1) if code was thrown away,
>+    // (2) if we simply hadn't actually made this call yet or (3) code is a builtin function and
>+    // specialization kind is construct. In the former 2 cases, we could still theoretically attempt
>+    // to inline it if we had a static proof of what was being called; this might happen for example
>+    // if you call a global function, where watchpointing gives us static information. Overall,
>+    // it's a rare case because we expect that any hot callees would have already been compiled.
>     CodeBlock* codeBlock = executable->baselineCodeBlockFor(kind);
>     if (!codeBlock) {
>         if (verbose)
>diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp
>index be3418091db31d058873d7fd8ff336a049ccd8ea..72149737b85b01a059c67db21163467e4c3a6390 100644
>--- a/Source/JavaScriptCore/jit/JITOperations.cpp
>+++ b/Source/JavaScriptCore/jit/JITOperations.cpp
>@@ -734,6 +734,12 @@ inline char* linkFor(
>         codePtr = executable->entrypointFor(*vm, kind, MustCheckArity, registers);
>     else {
>         FunctionExecutable* functionExecutable = static_cast<FunctionExecutable*>(executable);
>+
>+        if (!isCall(kind) && functionExecutable->isBuiltinFunction()) {
>+            exec->vm().throwException(exec, createNotAConstructorError(exec, callee));
>+            return reinterpret_cast<char*>(vm->getCTIStub(throwExceptionFromCallSlowPathGenerator).code().executableAddress());
>+        }
>+
>         JSObject* error = functionExecutable->prepareForExecution(execCallee, callee, scope, kind);
>         if (error) {
>             throwStackOverflowError(exec);
>@@ -793,6 +799,12 @@ inline char* virtualForWithFunction(
>     ExecutableBase* executable = function->executable();
>     if (UNLIKELY(!executable->hasJITCodeFor(kind))) {
>         FunctionExecutable* functionExecutable = static_cast<FunctionExecutable*>(executable);
>+
>+        if (!isCall(kind) && functionExecutable->isBuiltinFunction()) {
>+            exec->vm().throwException(exec, createNotAConstructorError(exec, function));
>+            return reinterpret_cast<char*>(vm->getCTIStub(throwExceptionFromCallSlowPathGenerator).code().executableAddress());
>+        }
>+
>         JSObject* error = functionExecutable->prepareForExecution(execCallee, function, scope, kind);
>         if (error) {
>             exec->vm().throwException(exec, error);
>diff --git a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>index 069755b53125c4744fd88c2b34f26ebbb5893162..a029938182879d55ba2f08e20d8bae3ad1e05f2e 100644
>--- a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>+++ b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>@@ -1097,6 +1097,10 @@ inline SlowPathReturnType setUpCall(ExecState* execCallee, Instruction* pc, Code
>         codePtr = executable->entrypointFor(vm, kind, MustCheckArity, RegisterPreservationNotRequired);
>     else {
>         FunctionExecutable* functionExecutable = static_cast<FunctionExecutable*>(executable);
>+
>+        if (!isCall(kind) && functionExecutable->isBuiltinFunction())
>+            LLINT_CALL_THROW(exec, createNotAConstructorError(exec, callee));
>+
>         JSObject* error = functionExecutable->prepareForExecution(execCallee, callee, scope, kind);
>         if (error)
>             LLINT_CALL_THROW(exec, error);
>diff --git a/Source/JavaScriptCore/runtime/JSFunction.cpp b/Source/JavaScriptCore/runtime/JSFunction.cpp
>index 6840a40b2e90306e35f254b1f52197727c84df2c..ed706eaacd8090f1d29acaeffb313263e1bd5011 100644
>--- a/Source/JavaScriptCore/runtime/JSFunction.cpp
>+++ b/Source/JavaScriptCore/runtime/JSFunction.cpp
>@@ -501,6 +501,10 @@ bool JSFunction::defineOwnProperty(JSObject* object, ExecState* exec, PropertyNa
> ConstructType JSFunction::getConstructData(JSCell* cell, ConstructData& constructData)
> {
>     JSFunction* thisObject = jsCast<JSFunction*>(cell);
>+
>+    if (thisObject->isBuiltinFunction())
>+        return ConstructTypeNone;
>+
>     if (thisObject->isHostFunction()) {
>         constructData.native.function = thisObject->nativeConstructor();
>         return ConstructTypeHost;
>diff --git a/Source/JavaScriptCore/tests/stress/builtin-function-is-construct-type-none.js b/Source/JavaScriptCore/tests/stress/builtin-function-is-construct-type-none.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..626248dea07e1a20a81a2199afd9d7a18b52a152
>--- /dev/null
>+++ b/Source/JavaScriptCore/tests/stress/builtin-function-is-construct-type-none.js
>@@ -0,0 +1,18 @@
>+function shouldThrow(func, message) {
>+    var error = null;
>+    try {
>+        func();
>+    } catch (e) {
>+        error = e;
>+    }
>+    if (!error)
>+        throw new Error("not thrown.");
>+    if (String(error) !== message)
>+        throw new Error("bad error: " + String(error));
>+}
>+
>+for (var i = 0; i < 10000; ++i) {
>+    shouldThrow(function () {
>+        new Array.prototype.forEach(function () { });
>+    }, "TypeError: function is not a constructor (evaluating 'new Array.prototype.forEach(function () { })')");
>+}

Actions: View | Formatted Diff | Diff

Attachments on bug 143923: 251098 | 251099 | 251100 | 251101