[JSC] Drop isEnvironmentRecord type info flag and use JSType information instead
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Oct 2016 01:33:14 +0000 (01:33 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Oct 2016 01:33:14 +0000 (01:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163761

Reviewed by Keith Miller.

JSTests:

* modules/string-prototype-module-scope.js: Added.
(shouldBe):
(catch):
(refer):
* stress/string-prototype-scopes-global-lexical-environment-strict.js: Added.
(shouldBe):
(catch):
* stress/string-prototype-scopes-global-lexical-environment.js: Added.
(shouldBe):
(catch):
* stress/string-prototype-scopes-strict.js: Added.
(shouldBe):
(catch):
(try.refer):
(refer):
* stress/string-prototype-scopes.js: Added.
(shouldBe):
(catch):
(try.refer):
(refer):
(object.toString):
(with):

Source/JavaScriptCore:

When we call a function in the following form,

    var charAt = String.prototype.charAt;
    charAt();  // |this| becomes the global object.

we should see |this| as undefined/null. In StringPrototype.cpp,
we use IsEnvironmentRecord type info flag to check whther the
given |this| is an environment record.
However, type info flag is precious thing and only StringPrototype.cpp
uses IsEnvironmentRecord. In addition to that, JSType should
already knows whether the given object is an environment record.
So IsEnvironmentRecord type info flag should be dropped.

This patch adds a new JSType, StrictEvalActivation. And we add a new
method JSObject::isEnvironmentRecord(). This method uses JSType to
return the result. And we drop IsEnvironmentRecord type info flag.
This patch makes a room for putting one bit flag to the out of line
type info flag. Previously, it is already exhausted.

* llint/LLIntData.cpp:
(JSC::LLInt::Data::performAssertions):
* llint/LowLevelInterpreter.asm:
* runtime/JSObject.h:
(JSC::JSObject::isStrictEvalActivation):
(JSC::JSObject::isEnvironmentRecord):
* runtime/JSSymbolTableObject.h:
* runtime/JSType.h:
* runtime/JSTypeInfo.h:
(JSC::TypeInfo::newImpurePropertyFiresWatchpoints):
(JSC::TypeInfo::isEnvironmentRecord): Deleted.
* runtime/StrictEvalActivation.h:
(JSC::StrictEvalActivation::createStructure):
* runtime/StringPrototype.cpp:
(JSC::checkObjectCoercible):

LayoutTests:

* js/dom/script-tests/string-prototype-scopes-in-workers.js: Added.
(catch):
* js/dom/script-tests/string-prototype-scopes.js: Added.
(catch):
* js/dom/string-prototype-scopes-expected.txt: Added.
* js/dom/string-prototype-scopes-in-workers-expected.txt: Added.
* js/dom/string-prototype-scopes-in-workers.html: Added.
* js/dom/string-prototype-scopes.html: Added.

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

22 files changed:
JSTests/ChangeLog
JSTests/modules/string-prototype-module-scope.js [new file with mode: 0644]
JSTests/stress/string-prototype-scopes-global-lexical-environment-strict.js [new file with mode: 0644]
JSTests/stress/string-prototype-scopes-global-lexical-environment.js [new file with mode: 0644]
JSTests/stress/string-prototype-scopes-strict.js [new file with mode: 0644]
JSTests/stress/string-prototype-scopes.js [new file with mode: 0644]
LayoutTests/ChangeLog
LayoutTests/js/dom/script-tests/string-prototype-scopes-in-workers.js [new file with mode: 0644]
LayoutTests/js/dom/script-tests/string-prototype-scopes.js [new file with mode: 0644]
LayoutTests/js/dom/string-prototype-scopes-expected.txt [new file with mode: 0644]
LayoutTests/js/dom/string-prototype-scopes-in-workers-expected.txt [new file with mode: 0644]
LayoutTests/js/dom/string-prototype-scopes-in-workers.html [new file with mode: 0644]
LayoutTests/js/dom/string-prototype-scopes.html [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/llint/LLIntData.cpp
Source/JavaScriptCore/llint/LowLevelInterpreter.asm
Source/JavaScriptCore/runtime/JSObject.h
Source/JavaScriptCore/runtime/JSSymbolTableObject.h
Source/JavaScriptCore/runtime/JSType.h
Source/JavaScriptCore/runtime/JSTypeInfo.h
Source/JavaScriptCore/runtime/StrictEvalActivation.h
Source/JavaScriptCore/runtime/StringPrototype.cpp

index 87fab15..08ed3c1 100644 (file)
@@ -1,3 +1,33 @@
+2016-10-20  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Drop isEnvironmentRecord type info flag and use JSType information instead
+        https://bugs.webkit.org/show_bug.cgi?id=163761
+
+        Reviewed by Keith Miller.
+
+        * modules/string-prototype-module-scope.js: Added.
+        (shouldBe):
+        (catch):
+        (refer):
+        * stress/string-prototype-scopes-global-lexical-environment-strict.js: Added.
+        (shouldBe):
+        (catch):
+        * stress/string-prototype-scopes-global-lexical-environment.js: Added.
+        (shouldBe):
+        (catch):
+        * stress/string-prototype-scopes-strict.js: Added.
+        (shouldBe):
+        (catch):
+        (try.refer):
+        (refer):
+        * stress/string-prototype-scopes.js: Added.
+        (shouldBe):
+        (catch):
+        (try.refer):
+        (refer):
+        (object.toString):
+        (with):
+
 2016-10-20  JF Bastien  <jfbastien@apple.com>
 
         WebAssembly API: implement exception constructors properly
diff --git a/JSTests/modules/string-prototype-module-scope.js b/JSTests/modules/string-prototype-module-scope.js
new file mode 100644 (file)
index 0000000..d04b858
--- /dev/null
@@ -0,0 +1,15 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var error = null;
+try {
+    var charAt = String.prototype.charAt;
+    charAt();
+} catch (e) {
+    error = e;
+}
+shouldBe(String(error), `TypeError: Type error`);
+
+function refer() { charAt; }
diff --git a/JSTests/stress/string-prototype-scopes-global-lexical-environment-strict.js b/JSTests/stress/string-prototype-scopes-global-lexical-environment-strict.js
new file mode 100644 (file)
index 0000000..665ed4b
--- /dev/null
@@ -0,0 +1,15 @@
+"use strict";
+
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var error = null;
+let charAt = String.prototype.charAt;
+try {
+    charAt();
+} catch (e) {
+    error = e;
+}
+shouldBe(String(error), `TypeError: Type error`);
diff --git a/JSTests/stress/string-prototype-scopes-global-lexical-environment.js b/JSTests/stress/string-prototype-scopes-global-lexical-environment.js
new file mode 100644 (file)
index 0000000..8a5ca01
--- /dev/null
@@ -0,0 +1,13 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var error = null;
+let charAt = String.prototype.charAt;
+try {
+    charAt();
+} catch (e) {
+    error = e;
+}
+shouldBe(String(error), `TypeError: Type error`);
diff --git a/JSTests/stress/string-prototype-scopes-strict.js b/JSTests/stress/string-prototype-scopes-strict.js
new file mode 100644 (file)
index 0000000..2830646
--- /dev/null
@@ -0,0 +1,51 @@
+"use strict";
+
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var error = null;
+try {
+    eval(`
+    var charAt = String.prototype.charAt;
+    charAt();
+    `);
+} catch (e) {
+    error = e;
+}
+shouldBe(String(error), `TypeError: Type error`);
+
+var error = null;
+try {
+    var charAt = String.prototype.charAt;
+    charAt();
+} catch (e) {
+    error = e;
+}
+shouldBe(String(error), `TypeError: Type error`);
+
+var error = null;
+try {
+    let charAt = String.prototype.charAt;
+    charAt();
+    function refer() { charAt; }
+} catch (e) {
+    error = e;
+}
+shouldBe(String(error), `TypeError: Type error`);
+
+(function () {
+    var error = null;
+    var ok = 42;
+    try {
+        var charAt = String.prototype.charAt;
+        charAt();
+    } catch (e) {
+        error = e;
+    }
+
+    function refer() { charAt; } // Refer the charAt variable.
+    shouldBe(String(error), `TypeError: Type error`);
+    return ok;
+}());
diff --git a/JSTests/stress/string-prototype-scopes.js b/JSTests/stress/string-prototype-scopes.js
new file mode 100644 (file)
index 0000000..3b61e71
--- /dev/null
@@ -0,0 +1,54 @@
+function shouldBe(actual, expected) {
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+var error = null;
+try {
+    eval(`
+    var charAt = String.prototype.charAt;
+    charAt();
+    `);
+} catch (e) {
+    error = e;
+}
+shouldBe(String(error), `TypeError: Type error`);
+
+var error = null;
+try {
+    var charAt = String.prototype.charAt;
+    charAt();
+} catch (e) {
+    error = e;
+}
+shouldBe(String(error), `TypeError: Type error`);
+
+var error = null;
+try {
+    let charAt = String.prototype.charAt;
+    charAt();
+    function refer() { charAt; }
+} catch (e) {
+    error = e;
+}
+shouldBe(String(error), `TypeError: Type error`);
+
+(function () {
+    var error = null;
+    var ok = 42;
+    try {
+        var charAt = String.prototype.charAt;
+        charAt();
+    } catch (e) {
+        error = e;
+    }
+
+    function refer() { charAt; } // Refer the charAt variable.
+    shouldBe(String(error), `TypeError: Type error`);
+    return ok;
+}());
+
+var object = { __proto__: String.prototype, toString() { return "Cocoa"; } };
+with (object) {
+    shouldBe(charAt(0), `C`);
+}
index 3712a28..25f8788 100644 (file)
@@ -1,3 +1,19 @@
+2016-10-20  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Drop isEnvironmentRecord type info flag and use JSType information instead
+        https://bugs.webkit.org/show_bug.cgi?id=163761
+
+        Reviewed by Keith Miller.
+
+        * js/dom/script-tests/string-prototype-scopes-in-workers.js: Added.
+        (catch):
+        * js/dom/script-tests/string-prototype-scopes.js: Added.
+        (catch):
+        * js/dom/string-prototype-scopes-expected.txt: Added.
+        * js/dom/string-prototype-scopes-in-workers-expected.txt: Added.
+        * js/dom/string-prototype-scopes-in-workers.html: Added.
+        * js/dom/string-prototype-scopes.html: Added.
+
 2016-10-20  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Implement WebGL2 bufferData() and bufferSubData() methods
diff --git a/LayoutTests/js/dom/script-tests/string-prototype-scopes-in-workers.js b/LayoutTests/js/dom/script-tests/string-prototype-scopes-in-workers.js
new file mode 100644 (file)
index 0000000..74b1a18
--- /dev/null
@@ -0,0 +1,16 @@
+importScripts('../../../resources/js-test-pre.js');
+
+description("This test checks whether String.prototype methods correctly handle |this| if |this| becomes worker global scope.");
+
+var global = this;
+global.jsTestIsAsync = true;
+
+var error = null;
+try {
+    var charAt = String.prototype.charAt;
+    charAt();
+} catch (e) {
+    error = e;
+}
+shouldBe(`String(error)`, `"TypeError: Type error"`);
+finishJSTest();
diff --git a/LayoutTests/js/dom/script-tests/string-prototype-scopes.js b/LayoutTests/js/dom/script-tests/string-prototype-scopes.js
new file mode 100644 (file)
index 0000000..a76b7ff
--- /dev/null
@@ -0,0 +1,10 @@
+description("This test checks whether String.prototype methods correctly handle |this| if |this| becomes window.");
+
+var error = null;
+try {
+    var charAt = String.prototype.charAt;
+    charAt();
+} catch (e) {
+    error = e;
+}
+shouldBe(`String(error)`, `"TypeError: Type error"`);
diff --git a/LayoutTests/js/dom/string-prototype-scopes-expected.txt b/LayoutTests/js/dom/string-prototype-scopes-expected.txt
new file mode 100644 (file)
index 0000000..7a907fd
--- /dev/null
@@ -0,0 +1,10 @@
+This test checks whether String.prototype methods correctly handle |this| if |this| becomes window.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS String(error) is "TypeError: Type error"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/dom/string-prototype-scopes-in-workers-expected.txt b/LayoutTests/js/dom/string-prototype-scopes-in-workers-expected.txt
new file mode 100644 (file)
index 0000000..af049d7
--- /dev/null
@@ -0,0 +1,10 @@
+[Worker] This test checks whether String.prototype methods correctly handle |this| if |this| becomes worker global scope.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+Starting worker: ./script-tests/string-prototype-scopes-in-workers.js
+PASS [Worker] String(error) is "TypeError: Type error"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/dom/string-prototype-scopes-in-workers.html b/LayoutTests/js/dom/string-prototype-scopes-in-workers.html
new file mode 100644 (file)
index 0000000..701f322
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<div id="description"></div>
+<div id="console"></div>
+<script>
+worker = startWorker('./script-tests/string-prototype-scopes-in-workers.js');
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/js/dom/string-prototype-scopes.html b/LayoutTests/js/dom/string-prototype-scopes.html
new file mode 100644 (file)
index 0000000..77d69c8
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/string-prototype-scopes.js"></script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index 4c41695..1dd5217 100644 (file)
@@ -1,3 +1,45 @@
+2016-10-20  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Drop isEnvironmentRecord type info flag and use JSType information instead
+        https://bugs.webkit.org/show_bug.cgi?id=163761
+
+        Reviewed by Keith Miller.
+
+        When we call a function in the following form,
+
+            var charAt = String.prototype.charAt;
+            charAt();  // |this| becomes the global object.
+
+        we should see |this| as undefined/null. In StringPrototype.cpp,
+        we use IsEnvironmentRecord type info flag to check whther the
+        given |this| is an environment record.
+        However, type info flag is precious thing and only StringPrototype.cpp
+        uses IsEnvironmentRecord. In addition to that, JSType should
+        already knows whether the given object is an environment record.
+        So IsEnvironmentRecord type info flag should be dropped.
+
+        This patch adds a new JSType, StrictEvalActivation. And we add a new
+        method JSObject::isEnvironmentRecord(). This method uses JSType to
+        return the result. And we drop IsEnvironmentRecord type info flag.
+        This patch makes a room for putting one bit flag to the out of line
+        type info flag. Previously, it is already exhausted.
+
+        * llint/LLIntData.cpp:
+        (JSC::LLInt::Data::performAssertions):
+        * llint/LowLevelInterpreter.asm:
+        * runtime/JSObject.h:
+        (JSC::JSObject::isStrictEvalActivation):
+        (JSC::JSObject::isEnvironmentRecord):
+        * runtime/JSSymbolTableObject.h:
+        * runtime/JSType.h:
+        * runtime/JSTypeInfo.h:
+        (JSC::TypeInfo::newImpurePropertyFiresWatchpoints):
+        (JSC::TypeInfo::isEnvironmentRecord): Deleted.
+        * runtime/StrictEvalActivation.h:
+        (JSC::StrictEvalActivation::createStructure):
+        * runtime/StringPrototype.cpp:
+        (JSC::checkObjectCoercible):
+
 2016-10-20  JF Bastien  <jfbastien@apple.com>
 
         WebAssembly API: implement exception constructors properly
index e5324cd..3a2113a 100644 (file)
@@ -161,7 +161,7 @@ void Data::performAssertions(VM& vm)
     STATIC_ASSERT(JSFunctionType == 23);
     STATIC_ASSERT(ArrayType == 31);
     STATIC_ASSERT(DerivedArrayType == 32);
-    STATIC_ASSERT(ProxyObjectType == 49);
+    STATIC_ASSERT(ProxyObjectType == 50);
     STATIC_ASSERT(Int8ArrayType == 33);
     STATIC_ASSERT(Int16ArrayType == 34);
     STATIC_ASSERT(Int32ArrayType == 35);
index a5e29da..560df3e 100644 (file)
@@ -350,7 +350,7 @@ const FinalObjectType = 21
 const JSFunctionType = 23
 const ArrayType = 31
 const DerivedArrayType = 32
-const ProxyObjectType = 49
+const ProxyObjectType = 50
 
 # The typed array types need to be numbered in a particular order because of the manually written
 # switch statement in get_by_val and put_by_val.
index b96a63c..6220ec7 100644 (file)
@@ -706,12 +706,15 @@ public:
 
     JS_EXPORT_PRIVATE static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
 
+    bool isEnvironmentRecord() const;
     bool isGlobalObject() const;
     bool isJSLexicalEnvironment() const;
     bool isGlobalLexicalEnvironment() const;
-    bool isErrorInstance() const;
+    bool isStrictEvalActivation() const;
     bool isWithScope() const;
 
+    bool isErrorInstance() const;
+
     JS_EXPORT_PRIVATE void seal(VM&);
     JS_EXPORT_PRIVATE void freeze(VM&);
     JS_EXPORT_PRIVATE static bool preventExtensions(JSObject*, ExecState*);
@@ -1175,6 +1178,18 @@ inline bool JSObject::isGlobalLexicalEnvironment() const
     return type() == GlobalLexicalEnvironmentType;
 }
 
+inline bool JSObject::isStrictEvalActivation() const
+{
+    return type() == StrictEvalActivationType;
+}
+
+inline bool JSObject::isEnvironmentRecord() const
+{
+    bool result = GlobalObjectType <= type() && type() <= StrictEvalActivationType;
+    ASSERT((isGlobalObject() || isJSLexicalEnvironment() || isGlobalLexicalEnvironment() || isStrictEvalActivation()) == result);
+    return result;
+}
+
 inline bool JSObject::isErrorInstance() const
 {
     return type() == ErrorInstanceType;
index e65f54f..8a299e5 100644 (file)
@@ -39,7 +39,7 @@ namespace JSC {
 class JSSymbolTableObject : public JSScope {
 public:
     typedef JSScope Base;
-    static const unsigned StructureFlags = Base::StructureFlags | IsEnvironmentRecord | OverridesGetPropertyNames;
+    static const unsigned StructureFlags = Base::StructureFlags | OverridesGetPropertyNames;
     
     SymbolTable* symbolTable() const { return m_symbolTable.get(); }
     
index aa247cb..f8bb7af 100644 (file)
@@ -77,10 +77,15 @@ enum JSType : uint8_t {
     DataViewType,
 
     GetterSetterType,
+
+    // Start environment record types.
     GlobalObjectType,
     LexicalEnvironmentType,
     GlobalLexicalEnvironmentType,
     ModuleEnvironmentType,
+    StrictEvalActivationType,
+    // End environment record types.
+
     RegExpObjectType,
     ProxyObjectType,
     JSMapType,
index 5c32f03..d266e72 100644 (file)
@@ -48,7 +48,6 @@ static const unsigned OverridesGetPropertyNames = 1 << 9;
 static const unsigned ProhibitsPropertyCaching = 1 << 10;
 static const unsigned GetOwnPropertySlotIsImpure = 1 << 11;
 static const unsigned NewImpurePropertyFiresWatchpoints = 1 << 12;
-static const unsigned IsEnvironmentRecord = 1 << 13;
 static const unsigned GetOwnPropertySlotIsImpureForPropertyAbsence = 1 << 14;
 static const unsigned InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero = 1 << 15;
 
@@ -90,7 +89,6 @@ public:
     bool getOwnPropertySlotIsImpure() const { return isSetOnFlags2(GetOwnPropertySlotIsImpure); }
     bool getOwnPropertySlotIsImpureForPropertyAbsence() const { return isSetOnFlags2(GetOwnPropertySlotIsImpureForPropertyAbsence); }
     bool newImpurePropertyFiresWatchpoints() const { return isSetOnFlags2(NewImpurePropertyFiresWatchpoints); }
-    bool isEnvironmentRecord() const { return isSetOnFlags2(IsEnvironmentRecord); }
     bool interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero() const { return isSetOnFlags2(InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero); }
 
     static ptrdiff_t flagsOffset()
index d8e2c7a..d800e96 100644 (file)
@@ -32,7 +32,7 @@ namespace JSC {
 class StrictEvalActivation : public JSScope {
 public:
     typedef JSScope Base;
-    static const unsigned StructureFlags = Base::StructureFlags | IsEnvironmentRecord | OverridesToThis;
+    static const unsigned StructureFlags = Base::StructureFlags | OverridesToThis;
 
     static StrictEvalActivation* create(ExecState* exec, JSScope* currentScope)
     {
@@ -46,7 +46,7 @@ public:
 
     static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
     {
-        return Structure::create(vm, globalObject, prototype, TypeInfo(ObjectType, StructureFlags), info());
+        return Structure::create(vm, globalObject, prototype, TypeInfo(StrictEvalActivationType, StructureFlags), info());
     }
     
     DECLARE_INFO;
index 12c44da..6ca6782 100644 (file)
@@ -746,7 +746,7 @@ static inline bool checkObjectCoercible(JSValue thisValue)
     if (thisValue.isUndefinedOrNull())
         return false;
 
-    if (thisValue.isCell() && thisValue.asCell()->structure()->typeInfo().isEnvironmentRecord())
+    if (thisValue.isObject() && asObject(thisValue)->isEnvironmentRecord())
         return false;
 
     return true;