Attachment 341342 Details for Bug 185995 – proposed patch.

[patch] proposed patch.

bug-185995.patch (text/plain), 6.59 KB, created by Mark Lam on 2018-05-25 16:00:43 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2018-05-25 16:00:43 PDT

Size: 6.59 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 232210)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2018-05-25  Mark Lam  <mark.lam@apple.com>
>+
>+        for-in loops should preserve and restore the TDZ stack for each of its internal loops.
>+        https://bugs.webkit.org/show_bug.cgi?id=185995
>+        <rdar://problem/40173142>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/regress-185995.js: Added.
>+
> 2018-05-23  Keith Miller  <keith_miller@apple.com>
> 
>         Define length on CoW array should properly convert to writable
>Index: JSTests/stress/regress-185995.js
>===================================================================
>--- JSTests/stress/regress-185995.js	(nonexistent)
>+++ JSTests/stress/regress-185995.js	(working copy)
>@@ -0,0 +1,16 @@
>+(function() {
>+    script =
>+        "var list = { 'a' : 5 };" + "\n" +
>+        "for(const { x = x } in list)" + "\n" +
>+        "    x();";
>+
>+    var exception;
>+    try {
>+        eval(script);
>+    } catch (e) {
>+        exception = e;
>+    }
>+
>+    if (exception != "ReferenceError: Cannot access uninitialized variable.")
>+        throw "FAILED";
>+})();
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 232100)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2018-05-25  Mark Lam  <mark.lam@apple.com>
>+
>+        for-in loops should preserve and restore the TDZ stack for each of its internal loops.
>+        https://bugs.webkit.org/show_bug.cgi?id=185995
>+        <rdar://problem/40173142>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This is because there's no guarantee that any of the loop bodies will be
>+        executed.  Hence, there's no guarantee that the TDZ variables will have been
>+        initialized after each loop body.
>+
>+        * bytecompiler/BytecodeGenerator.cpp:
>+        (JSC::BytecodeGenerator::preserveTDZStack):
>+        (JSC::BytecodeGenerator::restoreTDZStack):
>+        * bytecompiler/BytecodeGenerator.h:
>+        * bytecompiler/NodesCodegen.cpp:
>+        (JSC::ForInNode::emitBytecode):
>+
> 2018-05-22  Mark Lam  <mark.lam@apple.com>
> 
>         BytecodeGeneratorification shouldn't add a ValueProfile if the JIT is disabled.
>Index: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp	(revision 232099)
>+++ Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2008-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2008-2018 Apple Inc. All rights reserved.
>  * Copyright (C) 2008 Cameron Zwarich <cwzwarich@uwaterloo.ca>
>  * Copyright (C) 2012 Igalia, S.L.
>  *
>@@ -3139,6 +3139,16 @@ void BytecodeGenerator::getVariablesUnde
>     }
> }
> 
>+void BytecodeGenerator::preserveTDZStack(BytecodeGenerator::PreservedTDZStack& preservedStack)
>+{
>+    preservedStack.m_preservedTDZStack = m_TDZStack;
>+}
>+
>+void BytecodeGenerator::restoreTDZStack(const BytecodeGenerator::PreservedTDZStack& preservedStack)
>+{
>+    m_TDZStack = preservedStack.m_preservedTDZStack;
>+}
>+
> RegisterID* BytecodeGenerator::emitNewObject(RegisterID* dst)
> {
>     size_t begin = instructions().size();
>Index: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h
>===================================================================
>--- Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h	(revision 232099)
>+++ Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2008-2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2008-2018 Apple Inc. All rights reserved.
>  * Copyright (C) 2008 Cameron Zwarich <cwzwarich@uwaterloo.ca>
>  * Copyright (C) 2012 Igalia, S.L.
>  *
>@@ -1113,6 +1113,13 @@ namespace JSC {
>         void initializeArrowFunctionContextScopeIfNeeded(SymbolTable* functionSymbolTable = nullptr, bool canReuseLexicalEnvironment = false);
>         bool needsDerivedConstructorInArrowFunctionLexicalEnvironment();
> 
>+        enum class TDZNecessityLevel {
>+            NotNeeded,
>+            Optimize,
>+            DoNotOptimize
>+        };
>+        typedef HashMap<RefPtr<UniquedStringImpl>, TDZNecessityLevel, IdentifierRepHash> TDZMap;
>+
>     public:
>         JSString* addStringConstant(const Identifier&);
>         JSValue addBigIntConstant(const Identifier&, uint8_t radix);
>@@ -1122,6 +1129,15 @@ namespace JSC {
> 
>         RegisterID* emitThrowExpressionTooDeepException();
> 
>+        class PreservedTDZStack {
>+        private:
>+            Vector<TDZMap> m_preservedTDZStack;
>+            friend class BytecodeGenerator;
>+        };
>+
>+        void preserveTDZStack(PreservedTDZStack&);
>+        void restoreTDZStack(const PreservedTDZStack&);
>+
>     private:
>         Vector<UnlinkedInstruction, 0, UnsafeVectorOverflow> m_instructions;
> 
>@@ -1134,12 +1150,7 @@ namespace JSC {
>             int m_symbolTableConstantIndex;
>         };
>         Vector<LexicalScopeStackEntry> m_lexicalScopeStack;
>-        enum class TDZNecessityLevel {
>-            NotNeeded,
>-            Optimize,
>-            DoNotOptimize
>-        };
>-        typedef HashMap<RefPtr<UniquedStringImpl>, TDZNecessityLevel, IdentifierRepHash> TDZMap;
>+
>         Vector<TDZMap> m_TDZStack;
>         std::optional<size_t> m_varScopeLexicalScopeStackIndex;
>         void pushTDZVariables(const VariableEnvironment&, TDZCheckOptimization, TDZRequirement);
>Index: Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp	(revision 232099)
>+++ Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp	(working copy)
>@@ -3075,6 +3075,9 @@ void ForInNode::emitBytecode(BytecodeGen
> 
>     enumerator = generator.emitGetPropertyEnumerator(generator.newTemporary(), base.get());
> 
>+    BytecodeGenerator::PreservedTDZStack preservedTDZStack;
>+    generator.preserveTDZStack(preservedTDZStack);
>+
>     // Indexed property loop.
>     {
>         Ref<LabelScope> scope = generator.newLabelScope(LabelScope::Loop);
>@@ -3114,6 +3117,7 @@ void ForInNode::emitBytecode(BytecodeGen
>         generator.emitJump(end.get());
>         generator.emitLabel(loopEnd.get());
>     }
>+    generator.restoreTDZStack(preservedTDZStack);
> 
>     // Structure property loop.
>     {
>@@ -3154,6 +3158,7 @@ void ForInNode::emitBytecode(BytecodeGen
>         generator.emitJump(end.get());
>         generator.emitLabel(loopEnd.get());
>     }
>+    generator.restoreTDZStack(preservedTDZStack);
> 
>     // Generic property loop.
>     {

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185995: 341342 | 341345 | 341346