Changeset 204248 in webkit for trunk/Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp

Timestamp:

Aug 7, 2016, 7:48:56 PM (9 years ago)

Author:

Yusuke Suzuki

Message:

[ES6] Module namespace object should not allow unset IC
​https://bugs.webkit.org/show_bug.cgi?id=160553

Reviewed by Saam Barati.

JSTests:

• modules/namespace-object-get-property.js: Added.

(import.as.ns.from.string_appeared_here.shouldThrow):

• modules/namespace-object-has-property.js: Added.
• modules/namespace-object-inline-caching.js: Added.

(import.as.A.from.string_appeared_here.import.as.B.from.string_appeared_here.lookup):
(shouldBe.lookup.lookup):
(shouldBe.lookup):

• modules/namespace-object-inline-caching/a.js: Added.
• modules/namespace-object-inline-caching/b.js: Added.
• modules/namespace-object-try-get.js: Added.

(import.as.ns.from.string_appeared_here.tryGetByIdText):
(tryGetByIdTextStrict):

• modules/namespace-object-typed-array-fast-path.js: Added.
• test262.yaml:

Source/JavaScriptCore:

Previously, module namespace object accidentally allow "unset IC". But this "unsetness" does not rely on
the structure. We should disable inline caching onto the namespace object. Once it is needed, we should
create the special caching for namespace object like the following: it should be similar to monomorphic IC,
but it caches the object itself instead of the structure. It checks the object itself (And in DFG, it should be
CheckCell) and loads the value from the target module environment directly[1].

And this patch also set setIsTaintedByProxy for the module namespace object to notify to the caller that
this object has impure ::getOwnPropertySlot. Then this function is now renamed to setIsTaintedByOpaqueObject.

We drop the hack in JSModuleNamespaceObject::getOwnPropertySlot since we already introduced InternalMethodType
for ProxyObject. Previously we cannot distinguish ::HasProperty and ::GetOwnProperty. So not to throw any
errors for ::HasProperty case, we used slot.setCustom to delay the observable operation.
But, this hack lacks the support for hasOwnProperty: hasOwnProperty uses GetOwnProperty, so it should throw an error.
However the previous implementation does not throw an error since the delayed observable part (custom function part) is
skipped in hasOwnProperty implementation. We now remove this custom property hack and fix the corresponding failure
in test262.

[1]: ​https://bugs.webkit.org/show_bug.cgi?id=160590

• jit/JITOperations.cpp:
• runtime/ArrayPrototype.cpp:

(JSC::getProperty):

• runtime/JSGenericTypedArrayViewConstructorInlines.h:

(JSC::constructGenericTypedArrayViewWithArguments):

• runtime/JSModuleNamespaceObject.cpp:

(JSC::JSModuleNamespaceObject::getOwnPropertySlot):
(JSC::callbackGetter): Deleted.

• runtime/JSModuleNamespaceObject.h:
• runtime/PropertySlot.cpp:

(JSC::PropertySlot::getPureResult):

• runtime/PropertySlot.h:

(JSC::PropertySlot::PropertySlot):
(JSC::PropertySlot::setIsTaintedByOpaqueObject):
(JSC::PropertySlot::isTaintedByOpaqueObject):
(JSC::PropertySlot::setIsTaintedByProxy): Deleted.
(JSC::PropertySlot::isTaintedByProxy): Deleted.

• runtime/ProxyObject.cpp:

(JSC::ProxyObject::getOwnPropertySlotCommon):

File:

• trunk/Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/runtime/JSModuleNamespaceObject.cpp

@@ -99 +99 @@
 }
 
-static EncodedJSValue callbackGetter(ExecState* exec, EncodedJSValue thisValue, PropertyName propertyName)
-{
-    JSModuleNamespaceObject* thisObject = jsCast<JSModuleNamespaceObject*>(JSValue::decode(thisValue));
-    JSModuleRecord* moduleRecord = thisObject->moduleRecord();
-
-    JSModuleRecord::Resolution resolution = moduleRecord->resolveExport(exec, Identifier::fromUid(exec, propertyName.uid()));
-    ASSERT(resolution.type != JSModuleRecord::Resolution::Type::NotFound && resolution.type != JSModuleRecord::Resolution::Type::Ambiguous);
-
-    JSModuleRecord* targetModule = resolution.moduleRecord;
-    JSModuleEnvironment* targetEnvironment = targetModule->moduleEnvironment();
-
-    PropertySlot trampolineSlot(targetEnvironment, PropertySlot::InternalMethodType::Get);
-    if (!targetEnvironment->methodTable(exec->vm())->getOwnPropertySlot(targetEnvironment, exec, resolution.localName, trampolineSlot))
-        return JSValue::encode(jsUndefined());
-
-    JSValue value = trampolineSlot.getValue(exec, propertyName);
-    if (exec->hadException())
-        return JSValue::encode(jsUndefined());
-
-    // If the value is filled with TDZ value, throw a reference error.
-    if (!value)
-        return throwVMError(exec, createTDZError(exec));
-    return JSValue::encode(value);
-}
-
 bool JSModuleNamespaceObject::getOwnPropertySlot(JSObject* cell, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
 {
@@ -136 +111 @@
         return JSObject::getOwnPropertySlot(thisObject, exec, propertyName, slot);
 
+    // FIXME: Add IC for module namespace object.
+    // https://bugs.webkit.org/show_bug.cgi?id=160590
+    slot.disableCaching();
+    slot.setIsTaintedByOpaqueObject();
     if (!thisObject->m_exports.contains(propertyName.uid()))
         return false;
 
-    // https://esdiscuss.org/topic/march-24-meeting-notes
-    // http://www.ecma-international.org/ecma-262/6.0/#sec-module-namespace-exotic-objects-getownproperty-p
-    // section 9.4.6.5, step 6.
-    // This property will be seen as writable: true, enumerable:true, configurable: false.
-    // But this does not mean that this property is writable by users.
-    //
-    // In JSC, getOwnPropertySlot is not designed to throw any errors. But looking up the value from the module
-    // environment may throw error if the loaded variable is the TDZ value. To workaround, we set the custom
-    // getter function. When it is called, it looks up the variable and throws an error if the variable is not
-    // initialized.
-    slot.setCustom(thisObject, DontDelete, callbackGetter);
-    return true;
+    switch (slot.internalMethodType()) {
+    case PropertySlot::InternalMethodType::Get:
+    case PropertySlot::InternalMethodType::GetOwnProperty: {
+        JSModuleRecord* moduleRecord = thisObject->moduleRecord();
+
+        JSModuleRecord::Resolution resolution = moduleRecord->resolveExport(exec, Identifier::fromUid(exec, propertyName.uid()));
+        ASSERT(resolution.type != JSModuleRecord::Resolution::Type::NotFound && resolution.type != JSModuleRecord::Resolution::Type::Ambiguous);
+
+        JSModuleRecord* targetModule = resolution.moduleRecord;
+        JSModuleEnvironment* targetEnvironment = targetModule->moduleEnvironment();
+
+        PropertySlot trampolineSlot(targetEnvironment, PropertySlot::InternalMethodType::Get);
+        bool found = targetEnvironment->methodTable(exec->vm())->getOwnPropertySlot(targetEnvironment, exec, resolution.localName, trampolineSlot);
+        ASSERT_UNUSED(found, found);
+
+        JSValue value = trampolineSlot.getValue(exec, propertyName);
+        ASSERT(!exec->hadException());
+
+        // If the value is filled with TDZ value, throw a reference error.
+        if (!value) {
+            throwVMError(exec, createTDZError(exec));
+            return false;
+        }
+
+        slot.setValue(thisObject, DontDelete, value);
+        return true;
+    }
+    case PropertySlot::InternalMethodType::HasProperty: {
+        // Do not perform [[Get]] for [[HasProperty]].
+        // [[Get]] / [[GetOwnProperty]] onto namespace object could throw an error while [[HasProperty]] just returns true here.
+        // https://tc39.github.io/ecma262/#sec-module-namespace-exotic-objects-hasproperty-p
+        slot.setValue(thisObject, DontDelete, jsUndefined());
+        return true;
+    }
+
+    case PropertySlot::InternalMethodType::VMInquiry:
+        return false;
+    }
+
+    RELEASE_ASSERT_NOT_REACHED();
+    return false;
 }