op_check_tdz does not def its argument
[WebKit-https.git] / Source / JavaScriptCore / ChangeLog
index 8e94272..082240b 100644 (file)
@@ -1,3 +1,461 @@
+2019-03-08  Tadeu Zagallo  <tzagallo@apple.com>
+
+        op_check_tdz does not def its argument
+        https://bugs.webkit.org/show_bug.cgi?id=192880
+        <rdar://problem/46221598>
+
+        Reviewed by Saam Barati.
+
+        This prevented the for-in loop optimization in the bytecode generator, since
+        the analysis sees a redefinition of the loop variable.
+
+        * bytecode/BytecodeUseDef.h:
+        (JSC::computeDefsForBytecodeOffset):
+
+2019-03-07  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Make more fields lazy in JSGlobalObject
+        https://bugs.webkit.org/show_bug.cgi?id=195449
+
+        Reviewed by Mark Lam.
+
+        This patch makes more fields lazy-allocated in JSGlobalObject to save memory.
+
+        1. Some minor structures like moduleRecordStructure.
+        2. Some functions like parseInt / parseFloat. While they are eagerly created in JIT mode anyway to materialize NumberConstructor, we can lazily allocate them in non JIT mode.
+        3. ArrayBuffer constructor. While it is eagerly allocated in WebCore, we can make lazily allocated in JSC.
+
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::execute):
+        * runtime/JSArrayBufferPrototype.h:
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::init):
+        (JSC::JSGlobalObject::visitChildren):
+        * runtime/JSGlobalObject.h:
+        (JSC::JSGlobalObject::parseIntFunction const):
+        (JSC::JSGlobalObject::parseFloatFunction const):
+        (JSC::JSGlobalObject::evalFunction const):
+        (JSC::JSGlobalObject::strictEvalActivationStructure const):
+        (JSC::JSGlobalObject::moduleRecordStructure const):
+        (JSC::JSGlobalObject::moduleNamespaceObjectStructure const):
+        (JSC::JSGlobalObject::proxyObjectStructure const):
+        (JSC::JSGlobalObject::callableProxyObjectStructure const):
+        (JSC::JSGlobalObject::proxyRevokeStructure const):
+        (JSC::JSGlobalObject::arrayBufferConstructor const):
+        (JSC::JSGlobalObject::arrayBufferPrototype const):
+        (JSC::JSGlobalObject::arrayBufferStructure const):
+        * runtime/ProxyObject.h:
+        * runtime/StrictEvalActivation.cpp:
+        (JSC::StrictEvalActivation::StrictEvalActivation):
+        * runtime/StrictEvalActivation.h:
+        * wasm/js/JSWebAssemblyMemory.cpp:
+        (JSC::JSWebAssemblyMemory::buffer):
+        * wasm/js/WebAssemblyModuleConstructor.cpp:
+        (JSC::webAssemblyModuleCustomSections):
+
+2019-03-07  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] Remove merging must handle values into proven types in CFA
+        https://bugs.webkit.org/show_bug.cgi?id=195444
+
+        Reviewed by Saam Barati.
+
+        Previously, we are merging must handle values as a proven constant in CFA. This is OK as long as this proven AbstractValue is blurred by merging the other legit AbstractValues
+        from the successors. But let's consider the following code, this is actually generated DFG graph from the attached test in r242626.
+
+            Block #2 (loop header) succ #3, #4
+            ...
+            1: ForceOSRExit
+            ...
+            2: JSConstant(0)
+            3: SetLocal(@2, loc6)
+            ...
+            4: Branch(#3, #4)
+
+            Block #3 (This is OSR entry target) pred #2, #3, must handle value for loc6 => JSConstant(Int32, 31)
+            ...
+            5: GetLocal(loc6)
+            6: StringFromCharCode(@5)
+            ...
+
+        Block #3 is OSR entry target. So we have must handle value for loc6 and it is Int32 constant 31. Then we merge this constant as a proven value in #3's loc6 AbstractValue.
+        If the value from #2 blurs the value, it is OK. However, #2 has ForceOSRExit. So must handle value suddenly becomes the only source of loc6 in #3. Then we use this constant
+        as a proven value. But this is not expected behavior since must handle value is just a snapshot of the locals when we kick off the concurrent compilation. In the above example,
+        we assume that loop index is an constant 31, but it is wrong, and OSR entry fails. Because there is no strong assumption that the must handle value is the proven type or value,
+        we should not merge it in CFA.
+
+        Since (1) this is just an optimization, (2) type information is already propagated in prediction injection phase, and (3) the must handle value does not show the performance
+        progression in r211461 and we no longer see type misprediction in marsaglia-osr-entry.js, this patch simply removes must handle value type widening in CFA.
+
+        * dfg/DFGCFAPhase.cpp:
+        (JSC::DFG::CFAPhase::run):
+        (JSC::DFG::CFAPhase::performBlockCFA):
+        (JSC::DFG::CFAPhase::injectOSR): Deleted.
+
+2019-03-07  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] StringFromCharCode fast path should accept 0xff in DFG and FTL
+        https://bugs.webkit.org/show_bug.cgi?id=195429
+
+        Reviewed by Saam Barati.
+
+        We can create single characters without allocation up to 0xff character code. But currently, DFGSpeculativeJIT and FTLLowerDFGToB3 go to the slow path
+        for 0xff case. On the other hand, DFG DoesGC phase says GC won't happen if the child is int32 constant and it is <= 0xff. So, if you have `String.fromCharCode(0xff)`,
+        this breaks the assumption in DFG DoesGC. The correct fix is changing the check in DFGSpeculativeJIT and FTLLowerDFGToB3 from AboveOrEqual to Above.
+        Note that ThunkGenerators's StringFromCharCode thunk was correct.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileFromCharCode):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileStringFromCharCode):
+
+2019-03-07  Mark Lam  <mark.lam@apple.com>
+
+        Follow up refactoring in try-finally code after r242591.
+        https://bugs.webkit.org/show_bug.cgi?id=195428
+
+        Reviewed by Saam Barati.
+
+        1. Added some comments in emitFinallyCompletion() to describe each completion case.
+        2. Converted CatchEntry into a struct.
+        3. Renamed variable hasBreaksOrContinuesNotCoveredByJumps to hasBreaksOrContinuesThatEscapeCurrentFinally
+           to be more clear about its purpose.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::generate):
+        (JSC::BytecodeGenerator::emitOutOfLineExceptionHandler):
+        (JSC::BytecodeGenerator::emitFinallyCompletion):
+        * bytecompiler/BytecodeGenerator.h:
+
+2019-03-07  Saam Barati  <sbarati@apple.com>
+
+        CompactVariableMap::Handle's copy operator= leaks the previous data
+        https://bugs.webkit.org/show_bug.cgi?id=195398
+
+        Reviewed by Yusuke Suzuki.
+
+        The copy constructor was just assigning |this| to the new value,
+        forgetting to decrement the ref count of the thing pointed to by
+        the |this| handle. Based on Yusuke's suggestion, this patch refactors
+        the move constructor, move operator=, and copy operator= to use the
+        swap() primitive and the copy constructor primitive.
+
+        * parser/VariableEnvironment.cpp:
+        (JSC::CompactVariableMap::Handle::Handle):
+        (JSC::CompactVariableMap::Handle::swap):
+        (JSC::CompactVariableMap::Handle::operator=): Deleted.
+        * parser/VariableEnvironment.h:
+        (JSC::CompactVariableMap::Handle::Handle):
+        (JSC::CompactVariableMap::Handle::operator=):
+
+2019-03-07  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Lazily decode cached bytecode
+        https://bugs.webkit.org/show_bug.cgi?id=194810
+
+        Reviewed by Saam Barati.
+
+        Like lazy parsing, we should pause at code block boundaries. Instead
+        of always eagerly decoding UnlinkedFunctionExecutable's UnlinkedCodeBlocks,
+        we store their offsets in the executable and lazily decode them on the next
+        call to `unlinkedCodeBlockFor`.
+
+        * bytecode/UnlinkedFunctionExecutable.cpp:
+        (JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable):
+        (JSC::UnlinkedFunctionExecutable::~UnlinkedFunctionExecutable):
+        (JSC::UnlinkedFunctionExecutable::visitChildren):
+        (JSC::UnlinkedFunctionExecutable::unlinkedCodeBlockFor):
+        (JSC::UnlinkedFunctionExecutable::decodeCachedCodeBlocks):
+        * bytecode/UnlinkedFunctionExecutable.h:
+        * runtime/CachedTypes.cpp:
+        (JSC::Decoder::Decoder):
+        (JSC::Decoder::~Decoder):
+        (JSC::Decoder::create):
+        (JSC::Decoder::offsetOf):
+        (JSC::Decoder::cacheOffset):
+        (JSC::Decoder::ptrForOffsetFromBase):
+        (JSC::Decoder::handleForEnvironment const):
+        (JSC::Decoder::setHandleForEnvironment):
+        (JSC::Decoder::addFinalizer):
+        (JSC::VariableLengthObject::isEmpty const):
+        (JSC::CachedWriteBarrier::isEmpty const):
+        (JSC::CachedFunctionExecutable::unlinkedCodeBlockForCall const):
+        (JSC::CachedFunctionExecutable::unlinkedCodeBlockForConstruct const):
+        (JSC::CachedFunctionExecutable::decode const):
+        (JSC::UnlinkedFunctionExecutable::UnlinkedFunctionExecutable):
+        (JSC::decodeCodeBlockImpl):
+        (JSC::isCachedBytecodeStillValid):
+        (JSC::decodeFunctionCodeBlock):
+        * runtime/CachedTypes.h:
+        (JSC::Decoder::vm):
+
+2019-03-06  Mark Lam  <mark.lam@apple.com>
+
+        Exception is a JSCell, not a JSObject.
+        https://bugs.webkit.org/show_bug.cgi?id=195392
+
+        Reviewed by Saam Barati.
+
+        Exception is a VM implementation construct to carry a stack trace for the point
+        where it is thrown from.  As a reminder, an Exception is needed because:
+        1. JS code can throw primitives as well that are non-cells.
+        2. Error objects capture the stack trace at the point where they are constructed,
+           which is not always the same as the point where they are thrown (if they are
+           thrown).
+
+        Hence, Exception should not be visible to JS code, and therefore should not be a
+        JSObject.  Hence, it should not inherit from JSDestructibleObject.
+
+        This patch changes the following:
+
+        1. Exception now inherits directly from JSCell instead.
+
+        2. Places where we return an Exception masquerading as a JSObject* are now
+           updated to return a nullptr when we encounter an exception.
+
+        3. We still return Exception* as JSValue or EncodedJSValue when we encounter an
+           exception in functions that return JSValue or EncodedJSValue.  This is because
+           the number that implements the following pattern is too numerous:
+
+                return throw<Some Error>(...)
+
+           We'll leave these as is for now.
+
+        * bytecode/CodeBlock.h:
+        (JSC::ScriptExecutable::prepareForExecution):
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::executeProgram):
+        (JSC::Interpreter::executeCall):
+        (JSC::Interpreter::executeConstruct):
+        (JSC::Interpreter::prepareForRepeatCall):
+        (JSC::Interpreter::execute):
+        (JSC::Interpreter::executeModuleProgram):
+        * jit/JITOperations.cpp:
+        * llint/LLIntSlowPaths.cpp:
+        (JSC::LLInt::setUpCall):
+        * runtime/ConstructData.cpp:
+        (JSC::construct):
+        * runtime/Error.cpp:
+        (JSC::throwConstructorCannotBeCalledAsFunctionTypeError):
+        (JSC::throwTypeError):
+        (JSC::throwSyntaxError):
+        * runtime/Error.h:
+        (JSC::throwRangeError):
+        * runtime/Exception.cpp:
+        (JSC::Exception::createStructure):
+        * runtime/Exception.h:
+        * runtime/ExceptionHelpers.cpp:
+        (JSC::throwOutOfMemoryError):
+        (JSC::throwStackOverflowError):
+        (JSC::throwTerminatedExecutionException):
+        * runtime/ExceptionHelpers.h:
+        * runtime/FunctionConstructor.cpp:
+        (JSC::constructFunction):
+        (JSC::constructFunctionSkippingEvalEnabledCheck):
+        * runtime/IntlPluralRules.cpp:
+        (JSC::IntlPluralRules::resolvedOptions):
+        * runtime/JSGenericTypedArrayViewConstructorInlines.h:
+        (JSC::constructGenericTypedArrayViewWithArguments):
+        * runtime/JSObject.h:
+        * runtime/ObjectConstructor.cpp:
+        (JSC::objectConstructorSeal):
+        (JSC::objectConstructorFreeze):
+        * runtime/ProgramExecutable.cpp:
+        (JSC::ProgramExecutable::initializeGlobalProperties):
+        * runtime/RegExpConstructor.cpp:
+        (JSC::regExpCreate):
+        (JSC::constructRegExp):
+        * runtime/ScriptExecutable.cpp:
+        (JSC::ScriptExecutable::newCodeBlockFor):
+        (JSC::ScriptExecutable::prepareForExecutionImpl):
+        * runtime/ScriptExecutable.h:
+        * runtime/ThrowScope.cpp:
+        (JSC::ThrowScope::throwException):
+        * runtime/ThrowScope.h:
+        (JSC::ThrowScope::throwException):
+        (JSC::throwException):
+        * runtime/VM.cpp:
+        (JSC::VM::throwException):
+        * runtime/VM.h:
+
+2019-03-06  Ross Kirsling  <ross.kirsling@sony.com>
+
+        [Win] Remove -DUCHAR_TYPE=wchar_t stopgap and learn to live with char16_t.
+        https://bugs.webkit.org/show_bug.cgi?id=195346
+
+        Reviewed by Fujii Hironori.
+
+        * jsc.cpp:
+        (currentWorkingDirectory):
+        (fetchModuleFromLocalFileSystem):
+        * runtime/DateConversion.cpp:
+        (JSC::formatDateTime):
+        Use wchar helpers as needed.
+
+2019-03-06  Mark Lam  <mark.lam@apple.com>
+
+        Fix incorrect handling of try-finally completion values.
+        https://bugs.webkit.org/show_bug.cgi?id=195131
+        <rdar://problem/46222079>
+
+        Reviewed by Saam Barati and Yusuke Suzuki.
+
+        Consider the following:
+
+            function foo() {                        // line 1
+                try {
+                    return 42;                      // line 3
+                } finally {
+                    for (var j = 0; j < 1; j++) {   // line 5
+                        try {
+                            throw '';               // line 7
+                        } finally {
+                            continue;               // line 9
+                        }
+                    }
+                }                                   // line 11
+            }
+            var result = foo();
+
+        With the current (before fix) code base, result will be the exception object thrown
+        at line 7.  The expected result should be 42, returned at line 3.
+
+        The bug is that we were previously only using one set of completion type and
+        value registers for the entire function.  This is inadequate because the outer
+        try-finally needs to preserve its own completion type and value ({ Return, 42 }
+        in this case) in order to be able to complete correctly.
+
+        One might be deceived into thinking that the above example should complete with
+        the exception thrown at line 7.  However, according to Section 13.15.8 of the
+        ECMAScript spec, the 'continue' in the finally at line 9 counts as an abrupt
+        completion.  As a result, it overrides the throw from line 7.  After the continue,
+        execution resumes at the top of the loop at line 5, followed by a normal completion
+        at line 11.
+
+        Also according to Section 13.15.8, given that the completion type of the outer
+        finally is normal, the resultant completion of the outer try-finally should be
+        the completion of the outer try block i.e. { Return, 42 }.
+
+        This patch makes the following changes:
+        
+        1. Fix handling of finally completion to use a unique set of completion
+           type and value registers for each FinallyContext.
+
+        2. Move the setting of Throw completion type to the out of line exception handler.
+           This makes the mainline code slightly less branchy.
+
+        3. Introduce emitOutOfLineCatchHandler(), emitOutOfLineFinallyHandler(), and
+           emitOutOfLineExceptionHandler() to make it clearer that these are not emitting
+           bytecode inline.  Also, these make it clearer when we're emitting a handler
+           for a catch vs a finally.
+
+        4. Allocate the FinallyContext on the stack instead of as a member of the
+           heap allocated ControlFlowScope.  This simplifies its life-cycle management
+           and reduces the amount of needed copying.
+
+        5. Update emitFinallyCompletion() to propagate the completion type and value to
+           the outer FinallyContext when needed.
+
+        6. Fix emitJumpIf() to use the right order of operands.  Previously, we were
+           only using it to do op_stricteq and op_nstricteq comparisons.  So, the order
+           wasn't important.  We now use it to also do op_beloweq comparisons.  Hence,
+           the order needs to be corrected.
+
+        7. Remove the unused CompletionType::Break and Continue.  These are encoded with
+           the jumpIDs of the jump targets instead.
+
+        Relevant specifications:
+        Section 13.15.8: https://www.ecma-international.org/ecma-262/9.0/index.html#sec-try-statement-runtime-semantics-evaluation
+        Section 6.3.2.4: https://www.ecma-international.org/ecma-262/9.0/index.html#sec-updateempty
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::FinallyContext::FinallyContext):
+        (JSC::BytecodeGenerator::generate):
+        (JSC::BytecodeGenerator::BytecodeGenerator):
+        (JSC::BytecodeGenerator::pushFinallyControlFlowScope):
+        (JSC::BytecodeGenerator::popFinallyControlFlowScope):
+        (JSC::BytecodeGenerator::emitOutOfLineCatchHandler):
+        (JSC::BytecodeGenerator::emitOutOfLineFinallyHandler):
+        (JSC::BytecodeGenerator::emitOutOfLineExceptionHandler):
+        (JSC::BytecodeGenerator::emitEnumeration):
+        (JSC::BytecodeGenerator::emitJumpViaFinallyIfNeeded):
+        (JSC::BytecodeGenerator::emitReturnViaFinallyIfNeeded):
+        (JSC::BytecodeGenerator::emitFinallyCompletion):
+        (JSC::BytecodeGenerator::emitJumpIf):
+        (JSC::BytecodeGenerator::emitCatch): Deleted.
+        (JSC::BytecodeGenerator::allocateCompletionRecordRegisters): Deleted.
+        (JSC::BytecodeGenerator::releaseCompletionRecordRegisters): Deleted.
+        * bytecompiler/BytecodeGenerator.h:
+        (JSC::FinallyContext::completionTypeRegister const):
+        (JSC::FinallyContext::completionValueRegister const):
+        (JSC::ControlFlowScope::ControlFlowScope):
+        (JSC::BytecodeGenerator::emitLoad):
+        (JSC::BytecodeGenerator::CompletionRecordScope::CompletionRecordScope): Deleted.
+        (JSC::BytecodeGenerator::CompletionRecordScope::~CompletionRecordScope): Deleted.
+        (JSC::BytecodeGenerator::completionTypeRegister const): Deleted.
+        (JSC::BytecodeGenerator::completionValueRegister const): Deleted.
+        (JSC::BytecodeGenerator::emitSetCompletionType): Deleted.
+        (JSC::BytecodeGenerator::emitSetCompletionValue): Deleted.
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::TryNode::emitBytecode):
+
+2019-03-06  Saam Barati  <sbarati@apple.com>
+
+        JSScript should keep the cache file locked for the duration of its existence and should truncate the cache when it is out of date
+        https://bugs.webkit.org/show_bug.cgi?id=195186
+
+        Reviewed by Keith Miller.
+
+        This patch makes it so that JSScript will keep its bytecode cache file
+        locked as long as the JSScript is alive. This makes it obvious that it's
+        safe to update that file, as it will only be used in a single VM, across
+        all processes, at a single time. We may be able to extend this in the future
+        if we can atomically update it across VMs/processes. However, we're choosing
+        more restricted semantics now as it's always easier to extend these semantics
+        in the future opposed to having to support the more flexible behavior
+        up front.
+        
+        This patch also:
+        - Adds error messages if writing the cache fails. We don't expect this to
+          fail, but previously we would say we cached it even if write() fails.
+        - Removes the unused m_moduleKey field.
+        - Makes calling cacheBytecodeWithError with an already non-empty cache file fail.
+          In the future, we should extend this to just fill in the parts of the cache
+          that are not present. But we don't have the ability to do that yet, so we
+          just result in an error for now.
+
+        * API/JSScript.mm:
+        (-[JSScript dealloc]):
+        (-[JSScript readCache]):
+        (-[JSScript init]):
+        (-[JSScript writeCache:]):
+        * API/JSScriptInternal.h:
+        * API/tests/testapi.mm:
+        (testCacheFileIsExclusive):
+        (testCacheFileFailsWhenItsAlreadyCached):
+        (testObjectiveCAPI):
+
+2019-03-06  Christopher Reid  <chris.reid@sony.com>
+
+        Followups to (r242306): Use LockHolder instead of std::lock_guard on Remote Inspector Locks
+        https://bugs.webkit.org/show_bug.cgi?id=195381
+
+        Reviewed by Mark Lam.
+
+        Replacing std::lock_guard uses in Remote Inspector with WTF::LockHolder.
+        Also using `= { }` for struct initialization instead of memeset.
+
+        * inspector/remote/RemoteConnectionToTarget.cpp:
+        * inspector/remote/RemoteInspector.cpp:
+        * inspector/remote/cocoa/RemoteConnectionToTargetCocoa.mm:
+        * inspector/remote/cocoa/RemoteInspectorCocoa.mm:
+        * inspector/remote/cocoa/RemoteInspectorXPCConnection.mm:
+        * inspector/remote/glib/RemoteInspectorGlib.cpp:
+        * inspector/remote/playstation/RemoteInspectorPlayStation.cpp:
+        * inspector/remote/playstation/RemoteInspectorSocketClientPlayStation.cpp:
+        * inspector/remote/playstation/RemoteInspectorSocketPlayStation.cpp:
+        * inspector/remote/playstation/RemoteInspectorSocketServerPlayStation.cpp:
+
 2019-03-06  Saam Barati  <sbarati@apple.com>
 
         Air::reportUsedRegisters must padInterference