We should have a way of preventing a caller from making a tail call and we should...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Sep 2017 02:23:59 +0000 (02:23 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Sep 2017 02:23:59 +0000 (02:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176863

Reviewed by Keith Miller.

Source/JavaScriptCore:

* CMakeLists.txt:
* JavaScriptCore.xcodeproj/project.pbxproj:
* runtime/ProxyObject.cpp:
(JSC::performProxyGet):
(JSC::ProxyObject::performInternalMethodGetOwnProperty):
(JSC::ProxyObject::performHasProperty):
(JSC::ProxyObject::getOwnPropertySlotCommon):
(JSC::ProxyObject::performPut):
(JSC::performProxyCall):
(JSC::performProxyConstruct):
(JSC::ProxyObject::performDelete):
(JSC::ProxyObject::performPreventExtensions):
(JSC::ProxyObject::performIsExtensible):
(JSC::ProxyObject::performDefineOwnProperty):
(JSC::ProxyObject::performGetOwnPropertyNames):
(JSC::ProxyObject::performSetPrototype):
(JSC::ProxyObject::performGetPrototype):

Source/WTF:

This patch adds a way for a particular function to mark
that none of its calls should be tail calls.

It's useful in the following example if you don't want foo
to do a tail call to bar or baz:

int foo(bool b)
{
    NO_TAIL_CALLS();
    if (b)
        return baz();
    return bar();
}

Note that we're not saying that bar/baz should not be tail callable. bar/baz
may have other callers that are allowed to tail call it. This macro just says
that foo itself will not perform any tail calls.

* WTF.xcodeproj/project.pbxproj:
* wtf/NoTailCalls.h: Added.
(WTF::NoTailCalls::~NoTailCalls):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222071 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/CMakeLists.txt
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/runtime/ProxyObject.cpp
Source/WTF/ChangeLog
Source/WTF/WTF.xcodeproj/project.pbxproj
Source/WTF/wtf/NoTailCalls.h [new file with mode: 0644]

index 69bb36e..b6b7187 100644 (file)
@@ -879,6 +879,7 @@ set(JavaScriptCore_UNIFIABLE_SOURCES
     runtime/PropertyTable.cpp
     runtime/PrototypeMap.cpp
     runtime/ProxyConstructor.cpp
+    runtime/ProxyObject.cpp
     runtime/ProxyRevoke.cpp
     runtime/ReflectObject.cpp
     runtime/RegExp.cpp
@@ -1035,18 +1036,6 @@ endif ()
 
 list(APPEND JavaScriptCore_SOURCES ${generateUnifiedSourcesOutput})
 
-# These are special files that we can't or don't want to unified source compile
-list(APPEND JavaScriptCore_SOURCES
-    runtime/ProxyObject.cpp
-)
-
-# Extra flags for compile sources can go here.
-if (NOT MSVC)
-    set_source_files_properties(runtime/ProxyObject.cpp PROPERTIES COMPILE_FLAGS -fno-optimize-sibling-calls)
-else ()
-    # FIXME: Investigate if we need to set a similar flag on Windows.
-endif ()
-
 set(JavaScriptCore_OBJECT_LUT_SOURCES
     runtime/ArrayConstructor.cpp
     runtime/ArrayIteratorPrototype.cpp
index e58b36e..069dc15 100644 (file)
@@ -1,5 +1,30 @@
 2017-09-14  Saam Barati  <sbarati@apple.com>
 
+        We should have a way of preventing a caller from making a tail call and we should use it for ProxyObject instead of using build flags
+        https://bugs.webkit.org/show_bug.cgi?id=176863
+
+        Reviewed by Keith Miller.
+
+        * CMakeLists.txt:
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * runtime/ProxyObject.cpp:
+        (JSC::performProxyGet):
+        (JSC::ProxyObject::performInternalMethodGetOwnProperty):
+        (JSC::ProxyObject::performHasProperty):
+        (JSC::ProxyObject::getOwnPropertySlotCommon):
+        (JSC::ProxyObject::performPut):
+        (JSC::performProxyCall):
+        (JSC::performProxyConstruct):
+        (JSC::ProxyObject::performDelete):
+        (JSC::ProxyObject::performPreventExtensions):
+        (JSC::ProxyObject::performIsExtensible):
+        (JSC::ProxyObject::performDefineOwnProperty):
+        (JSC::ProxyObject::performGetOwnPropertyNames):
+        (JSC::ProxyObject::performSetPrototype):
+        (JSC::ProxyObject::performGetPrototype):
+
+2017-09-14  Saam Barati  <sbarati@apple.com>
+
         Make dumping the graph print when both when exitOK and !exitOK
         https://bugs.webkit.org/show_bug.cgi?id=176954
 
index 3df622c..9e17b90 100644 (file)
                79AF0BE41D3EFD4C00E95FA5 /* JITMathICInlineResult.h in Headers */ = {isa = PBXBuildFile; fileRef = 79AF0BE31D3EFD4C00E95FA5 /* JITMathICInlineResult.h */; settings = {ATTRIBUTES = (Private, ); }; };
                79B00CBC1C6AB07E0088C65D /* ProxyConstructor.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 79B00CB81C6AB07E0088C65D /* ProxyConstructor.cpp */; };
                79B00CBD1C6AB07E0088C65D /* ProxyConstructor.h in Headers */ = {isa = PBXBuildFile; fileRef = 79B00CB91C6AB07E0088C65D /* ProxyConstructor.h */; settings = {ATTRIBUTES = (Private, ); }; };
-               79B00CBE1C6AB07E0088C65D /* ProxyObject.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 79B00CBA1C6AB07E0088C65D /* ProxyObject.cpp */; settings = {COMPILER_FLAGS = "-fno-optimize-sibling-calls"; }; };
+               79B00CBE1C6AB07E0088C65D /* ProxyObject.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 79B00CBA1C6AB07E0088C65D /* ProxyObject.cpp */; };
                79B00CBF1C6AB07E0088C65D /* ProxyObject.h in Headers */ = {isa = PBXBuildFile; fileRef = 79B00CBB1C6AB07E0088C65D /* ProxyObject.h */; settings = {ATTRIBUTES = (Private, ); }; };
                79B1788E1D399B8000B1A567 /* JITMathICForwards.h in Headers */ = {isa = PBXBuildFile; fileRef = 79A899FE1D38612E00D18C73 /* JITMathICForwards.h */; settings = {ATTRIBUTES = (Private, ); }; };
                79B759741DFA4C600052174C /* WasmMemoryInformation.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 79B759711DFA4C600052174C /* WasmMemoryInformation.cpp */; };
index 07bec09..21fde0c 100644 (file)
@@ -35,8 +35,9 @@
 #include "SlotVisitorInlines.h"
 #include "StructureInlines.h"
 #include "VMInlines.h"
+#include <wtf/NoTailCalls.h>
 
-// Note that this file is compile with -fno-optimize-sibling-calls because we rely on the machine stack
+// Note that we use NO_TAIL_CALLS() throughout this file because we rely on the machine stack
 // growing larger for throwing OOM errors for when we have an effectively cyclic prototype chain.
 
 namespace JSC {
@@ -118,6 +119,8 @@ static const char* s_proxyAlreadyRevokedErrorMessage = "Proxy has already been r
 
 static JSValue performProxyGet(ExecState* exec, ProxyObject* proxyObject, JSValue receiver, PropertyName propertyName)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -180,6 +183,8 @@ static JSValue performProxyGet(ExecState* exec, ProxyObject* proxyObject, JSValu
 
 bool ProxyObject::performGet(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     JSValue result = performProxyGet(exec, this, slot.thisValue(), propertyName);
@@ -191,6 +196,8 @@ bool ProxyObject::performGet(ExecState* exec, PropertyName propertyName, Propert
 
 bool ProxyObject::performInternalMethodGetOwnProperty(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -296,6 +303,8 @@ bool ProxyObject::performInternalMethodGetOwnProperty(ExecState* exec, PropertyN
 
 bool ProxyObject::performHasProperty(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -404,6 +413,8 @@ bool ProxyObject::getOwnPropertySlotByIndex(JSObject* object, ExecState* exec, u
 template <typename PerformDefaultPutFunction>
 bool ProxyObject::performPut(ExecState* exec, JSValue putValue, JSValue thisValue, PropertyName propertyName, PerformDefaultPutFunction performDefaultPut)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -499,6 +510,8 @@ bool ProxyObject::putByIndex(JSCell* cell, ExecState* exec, unsigned propertyNam
 
 static EncodedJSValue JSC_HOST_CALL performProxyCall(ExecState* exec)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -549,6 +562,8 @@ CallType ProxyObject::getCallData(JSCell* cell, CallData& callData)
 
 static EncodedJSValue JSC_HOST_CALL performProxyConstruct(ExecState* exec)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -603,6 +618,8 @@ ConstructType ProxyObject::getConstructData(JSCell* cell, ConstructData& constru
 template <typename DefaultDeleteFunction>
 bool ProxyObject::performDelete(ExecState* exec, PropertyName propertyName, DefaultDeleteFunction performDefaultDelete)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -680,6 +697,8 @@ bool ProxyObject::deletePropertyByIndex(JSCell* cell, ExecState* exec, unsigned
 
 bool ProxyObject::performPreventExtensions(ExecState* exec)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -731,6 +750,8 @@ bool ProxyObject::preventExtensions(JSObject* object, ExecState* exec)
 
 bool ProxyObject::performIsExtensible(ExecState* exec)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -788,6 +809,8 @@ bool ProxyObject::isExtensible(JSObject* object, ExecState* exec)
 
 bool ProxyObject::performDefineOwnProperty(ExecState* exec, PropertyName propertyName, const PropertyDescriptor& descriptor, bool shouldThrow)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -882,6 +905,8 @@ bool ProxyObject::defineOwnProperty(JSObject* object, ExecState* exec, PropertyN
 
 void ProxyObject::performGetOwnPropertyNames(ExecState* exec, PropertyNameArray& trapResult, EnumerationMode enumerationMode)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
@@ -1005,6 +1030,7 @@ void ProxyObject::getOwnPropertyNames(JSObject* object, ExecState* exec, Propert
 
 void ProxyObject::getPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNameArray, EnumerationMode enumerationMode)
 {
+    NO_TAIL_CALLS();
     JSObject::getPropertyNames(object, exec, propertyNameArray, enumerationMode);
 }
 
@@ -1026,6 +1052,8 @@ void ProxyObject::getGenericPropertyNames(JSObject*, ExecState*, PropertyNameArr
 
 bool ProxyObject::performSetPrototype(ExecState* exec, JSValue prototype, bool shouldThrowIfCantSet)
 {
+    NO_TAIL_CALLS();
+
     ASSERT(prototype.isObject() || prototype.isNull());
 
     VM& vm = exec->vm();
@@ -1090,6 +1118,8 @@ bool ProxyObject::setPrototype(JSObject* object, ExecState* exec, JSValue protot
 
 JSValue ProxyObject::performGetPrototype(ExecState* exec)
 {
+    NO_TAIL_CALLS();
+
     VM& vm = exec->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
     if (UNLIKELY(!vm.isSafeToRecurseSoft())) {
index fc5e124..5492c8b 100644 (file)
@@ -1,3 +1,32 @@
+2017-09-14  Saam Barati  <sbarati@apple.com>
+
+        We should have a way of preventing a caller from making a tail call and we should use it for ProxyObject instead of using build flags
+        https://bugs.webkit.org/show_bug.cgi?id=176863
+
+        Reviewed by Keith Miller.
+
+        This patch adds a way for a particular function to mark
+        that none of its calls should be tail calls.
+        
+        It's useful in the following example if you don't want foo
+        to do a tail call to bar or baz:
+        
+        int foo(bool b)
+        {
+            NO_TAIL_CALLS();
+            if (b)
+                return baz();
+            return bar();
+        }
+        
+        Note that we're not saying that bar/baz should not be tail callable. bar/baz
+        may have other callers that are allowed to tail call it. This macro just says
+        that foo itself will not perform any tail calls.
+
+        * WTF.xcodeproj/project.pbxproj:
+        * wtf/NoTailCalls.h: Added.
+        (WTF::NoTailCalls::~NoTailCalls):
+
 2017-09-14  Mark Lam  <mark.lam@apple.com>
 
         AddressSanitizer: stack-buffer-underflow in JSC::Probe::Page::Page
index 96683aa..8e3ed18 100644 (file)
                51F175281F3D486000C74950 /* PersistentDecoder.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PersistentDecoder.h; sourceTree = "<group>"; };
                51F175291F3D486000C74950 /* PersistentEncoder.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PersistentEncoder.cpp; sourceTree = "<group>"; };
                51F1752A1F3D486000C74950 /* PersistentEncoder.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PersistentEncoder.h; sourceTree = "<group>"; };
+               526AEC911F6B4E5C00695F5D /* NoTailCalls.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = NoTailCalls.h; sourceTree = "<group>"; };
                5311BD511EA71CAD00525281 /* Signals.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = Signals.cpp; sourceTree = "<group>"; };
                5311BD551EA7E15A00525281 /* LocklessBag.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LocklessBag.h; sourceTree = "<group>"; };
                5311BD571EA7E1A100525281 /* Signals.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Signals.h; sourceTree = "<group>"; };
                                0F5BF1651F2317830029D91D /* NaturalLoops.h */,
                                1A3F6BE6174ADA2100B2EEA7 /* NeverDestroyed.h */,
                                0F0D85B317234CB100338210 /* NoLock.h */,
+                               526AEC911F6B4E5C00695F5D /* NoTailCalls.h */,
                                A8A472D0151A825B004123FF /* Noncopyable.h */,
                                7CEAE5AC1EA6E10F00DB6890 /* NotFound.h */,
                                A8A472D5151A825B004123FF /* NumberOfCores.cpp */,
diff --git a/Source/WTF/wtf/NoTailCalls.h b/Source/WTF/wtf/NoTailCalls.h
new file mode 100644 (file)
index 0000000..5895509
--- /dev/null
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2017 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
+ */
+
+#pragma once
+
+#include "Atomics.h"
+
+namespace WTF {
+
+struct NoTailCalls {
+    ~NoTailCalls()
+    {
+        compilerFence();
+    }
+};
+
+} // namespace WTF
+
+// Use this macro when you don't want to perform any tail calls from within a given scope.
+// For example, if you don't want the function foo to do a tail call to bar or baz, you'd do:
+// int foo(bool b)
+// {
+//     NO_TAIL_CALLS();
+//     if (b)
+//         return baz();
+//     return bar();
+// }
+//
+// This is helpful when bar or baz have other callers that are allowed to tail call it.
+
+#define NO_TAIL_CALLS() WTF::NoTailCalls _noTailCalls_