Changeset 264679 in webkit

Timestamp:

Jul 21, 2020 2:10:29 PM (4 years ago)

Author:

mark.lam@apple.com

Message:

llint_slow_path_get_private_name() should not be using PropertySlot::InternalMethodType::VMInquiry.
​https://bugs.webkit.org/show_bug.cgi?id=214603

Reviewed by Yusuke Suzuki.

VMInquiry means (1) the get operation should not call back into JS, (2) it should
not throw any exceptions (except for OutOfMemoryError or StackOverflowError which
can be thrown at any time), or have any side effects that is observable from JS
code. In this case, llint_slow_path_get_private_name() is just implementating
PrivateFieldGet (​https://tc39.es/proposal-class-fields/#sec-privatefieldget) and
should actually be using PropertySlot::InternalMethodType::GetOwnProperty
(according to ​https://tc39.es/proposal-class-fields/#sec-privatefieldfind).

This patch makes the above change, and also adds an assert in JSObject::getPrivateField
to ensure that no one calls it for a VMInquiry since it is not supported.

Also added a PropertySlot::isVMInquiry() convenience query method.

• llint/LLIntSlowPaths.cpp:

(JSC::LLInt::LLINT_SLOW_PATH_DECL):

• runtime/JSObjectInlines.h:

(JSC::JSObject::getPrivateField):

• runtime/PropertySlot.h:

(JSC::PropertySlot::isVMInquiry const):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• llint/LLIntSlowPaths.cpp (modified) (1 diff)
• runtime/JSObjectInlines.h (modified) (2 diffs)
• runtime/PropertySlot.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-07-21  Mark Lam  <mark.lam@apple.com>
+
+        llint_slow_path_get_private_name() should not be using PropertySlot::InternalMethodType::VMInquiry.
+        https://bugs.webkit.org/show_bug.cgi?id=214603
+
+        Reviewed by Yusuke Suzuki.
+
+        VMInquiry means (1) the get operation should not call back into JS, (2) it should
+        not throw any exceptions (except for OutOfMemoryError or StackOverflowError which
+        can be thrown at any time), or have any side effects that is observable from JS
+        code.  In this case, llint_slow_path_get_private_name() is just implementating
+        PrivateFieldGet (https://tc39.es/proposal-class-fields/#sec-privatefieldget) and
+        should actually be using PropertySlot::InternalMethodType::GetOwnProperty
+        (according to https://tc39.es/proposal-class-fields/#sec-privatefieldfind).
+
+        This patch makes the above change, and also adds an assert in JSObject::getPrivateField
+        to ensure that no one calls it for a VMInquiry since it is not supported.
+
+        Also added a PropertySlot::isVMInquiry() convenience query method.
+
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
+        * runtime/JSObjectInlines.h:
+        (JSC::JSObject::getPrivateField):
+        * runtime/PropertySlot.h:
+        (JSC::PropertySlot::isVMInquiry const):
+
 2020-07-21  Keith Miller  <keith_miller@apple.com>
 

trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

@@ -1094 +1094 @@
     ASSERT(property.isPrivateName());
 
-    PropertySlot slot(baseValue, PropertySlot::InternalMethodType::VMInquiry);
+    PropertySlot slot(baseValue, PropertySlot::InternalMethodType::GetOwnProperty);
     asObject(baseValue)->getPrivateField(globalObject, property, slot);
     LLINT_CHECK_EXCEPTION();

trunk/Source/JavaScriptCore/runtime/JSObjectInlines.h

@@ -2 +2 @@
  *  Copyright (C) 1999-2001 Harri Porten (porten@kde.org)
  *  Copyright (C) 2001 Peter Kelly (pmk@post.com)
- *  Copyright (C) 2003-2019 Apple Inc. All rights reserved.
+ *  Copyright (C) 2003-2020 Apple Inc. All rights reserved.
  *  Copyright (C) 2007 Eric Seidel (eric@webkit.org)
  *
@@ -604 +604 @@
     VM& vm = getVM(globalObject);
     auto scope = DECLARE_THROW_SCOPE(vm);
+    ASSERT(!slot.isVMInquiry());
     if (!JSObject::getPrivateFieldSlot(this, globalObject, propertyName, slot)) {
         throwException(globalObject, scope, createInvalidPrivateNameError(globalObject));

trunk/Source/JavaScriptCore/runtime/PropertySlot.h

@@ -1 +1 @@
 /*
- *  Copyright (C) 2005-2019 Apple Inc. All rights reserved.
+ *  Copyright (C) 2005-2020 Apple Inc. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
@@ -140 +140 @@
 
     InternalMethodType internalMethodType() const { return m_internalMethodType; }
+    bool isVMInquiry() const { return m_internalMethodType == InternalMethodType::VMInquiry; }
 
     void disableCaching()