LazyClassStructure::setConstructor should not store the constructor t…

…o the global object

https://bugs.webkit.org/show_bug.cgi?id=201484
<rdar://problem/50400451>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/web-assembly-constructors-should-not-override-global-object-property.js: Added.

Source/JavaScriptCore:

LazyClassStructure::setConstructor sets the constructor as a property of the global object.
This became a problem when it started being used for WebAssembly constructors, such as Module
and Instance, since they are properties of the WebAssembly object, not the global object. That
resulted in properties of the global object replaced whenever a lazy WebAssembly constructor
was first accessed. e.g.

globalThis.Module = x;
WebAssembly.Module;
globalThis.Module === WebAssembly.Module;

* runtime/LazyClassStructure.cpp:
(JSC::LazyClassStructure::Initializer::setConstructor):
* runtime/LazyClassStructure.h:
* runtime/Lookup.h:
(JSC::reifyStaticProperty):


Canonical link: https://commits.webkit.org/215142@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249538 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog

@@ -1,3 +1,13 @@
+2019-09-05  Tadeu Zagallo  <tzagallo@apple.com>
+
+        LazyClassStructure::setConstructor should not store the constructor to the global object
+        https://bugs.webkit.org/show_bug.cgi?id=201484
+        <rdar://problem/50400451>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/web-assembly-constructors-should-not-override-global-object-property.js: Added.
+
 2019-09-05  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] Do not use FTLOutput::weakPointer directly

JSTests/stress/web-assembly-constructors-should-not-override-global-object-property.js

@@ -0,0 +1,4 @@
+var originalModule = this.Module = {};
+WebAssembly.Module;
+if (Module !== originalModule)
+    throw new Error('Global property `Module` was overwritten!');

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,27 @@
+2019-09-05  Tadeu Zagallo  <tzagallo@apple.com>
+
+        LazyClassStructure::setConstructor should not store the constructor to the global object
+        https://bugs.webkit.org/show_bug.cgi?id=201484
+        <rdar://problem/50400451>
+
+        Reviewed by Yusuke Suzuki.
+
+        LazyClassStructure::setConstructor sets the constructor as a property of the global object.
+        This became a problem when it started being used for WebAssembly constructors, such as Module
+        and Instance, since they are properties of the WebAssembly object, not the global object. That
+        resulted in properties of the global object replaced whenever a lazy WebAssembly constructor
+        was first accessed. e.g.
+
+        globalThis.Module = x;
+        WebAssembly.Module;
+        globalThis.Module === WebAssembly.Module;
+
+        * runtime/LazyClassStructure.cpp:
+        (JSC::LazyClassStructure::Initializer::setConstructor):
+        * runtime/LazyClassStructure.h:
+        * runtime/Lookup.h:
+        (JSC::reifyStaticProperty):
+
 2019-09-05  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] Do not use FTLOutput::weakPointer directly

Source/JavaScriptCore/runtime/LazyClassStructure.cpp

@@ -60,7 +60,7 @@ void LazyClassStructure::Initializer::setStructure(Structure* structure)
         prototype = structure->storedPrototypeObject();
 }
 
-void LazyClassStructure::Initializer::setConstructor(PropertyName propertyName, JSObject* constructor)
+void LazyClassStructure::Initializer::setConstructor(JSObject* constructor)
 {
     RELEASE_ASSERT(structure);
     RELEASE_ASSERT(prototype);
@@ -69,24 +69,9 @@ void LazyClassStructure::Initializer::setConstructor(PropertyName propertyName,
     this->constructor = constructor;
 
     prototype->putDirectWithoutTransition(vm, vm.propertyNames->constructor, constructor, static_cast<unsigned>(PropertyAttribute::DontEnum));
-    if (!propertyName.isNull())
-        global->putDirect(vm, propertyName, constructor, static_cast<unsigned>(PropertyAttribute::DontEnum));
     classStructure.m_constructor.set(vm, global, constructor);
 }
 
-void LazyClassStructure::Initializer::setConstructor(JSObject* constructor)
-{
-    String name;
-    if (InternalFunction* internalFunction = jsDynamicCast<InternalFunction*>(vm, constructor))
-        name = internalFunction->name();
-    else if (JSFunction* function = jsDynamicCast<JSFunction*>(vm, constructor))
-        name = function->name(vm);
-    else
-        RELEASE_ASSERT_NOT_REACHED();
-
-    setConstructor(Identifier::fromString(vm, name), constructor);
-}
-
 void LazyClassStructure::visit(SlotVisitor& visitor)
 {
     m_structure.visit(visitor);

Source/JavaScriptCore/runtime/LazyClassStructure.h

@@ -49,12 +49,6 @@ class LazyClassStructure {
 
         // Call this last. It's expected that the constructor is initialized to point to the
         // prototype already. This will automatically set prototype.constructor=constructor.
-        // This will also stuff the constructor into the global object at the given property.
-        // Note that the variant that does not take a property name attempts to deduce it by
-        // casting constructor to either JSFunction or InternalFunction. Also, you can pass
-        // nullptr for the property name, in which case we don't assign the property to the
-        // global object.
-        void setConstructor(PropertyName, JSObject* constructor);
         void setConstructor(JSObject* constructor);
 
         VM& vm;

Source/JavaScriptCore/runtime/Lookup.h

@@ -361,9 +361,10 @@ inline void reifyStaticProperty(VM& vm, const ClassInfo* classInfo, const Proper
     }
 
     if (value.attributes() & PropertyAttribute::ClassStructure) {
-        LazyClassStructure* structure = bitwise_cast<LazyClassStructure*>(
+        LazyClassStructure* lazyStructure = bitwise_cast<LazyClassStructure*>(
             bitwise_cast<char*>(&thisObj) + value.lazyClassStructureOffset());
-        structure->get(jsCast<JSGlobalObject*>(&thisObj));
+        JSObject* constructor = lazyStructure->constructor(jsCast<JSGlobalObject*>(&thisObj));
+        thisObj.putDirect(vm, propertyName, constructor, attributesForStructure(value.attributes()));
         return;
     }