Unreviewed, rolling out r243672.
[WebKit-https.git] / Source / JavaScriptCore / ChangeLog
index 25f7144..501ef26 100644 (file)
@@ -1,3 +1,465 @@
+2019-04-15  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r243672.
+        https://bugs.webkit.org/show_bug.cgi?id=196952
+
+        [JSValue release] should be thread-safe (Requested by
+        yusukesuzuki on #webkit).
+
+        Reverted changeset:
+
+        "[JSC] JSWrapperMap should not use Objective-C Weak map
+        (NSMapTable with NSPointerFunctionsWeakMemory) for
+        m_cachedObjCWrappers"
+        https://bugs.webkit.org/show_bug.cgi?id=196392
+        https://trac.webkit.org/changeset/243672
+
+2019-04-15  Saam barati  <sbarati@apple.com>
+
+        SafeToExecute for GetByOffset/GetGetterByOffset/PutByOffset is using the wrong child for the base
+        https://bugs.webkit.org/show_bug.cgi?id=196945
+        <rdar://problem/49802750>
+
+        Reviewed by Filip Pizlo.
+
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+
+2019-04-15  Robin Morisset  <rmorisset@apple.com>
+
+        DFG should be able to constant fold Object.create() with a constant prototype operand
+        https://bugs.webkit.org/show_bug.cgi?id=196886
+
+        Reviewed by Yusuke Suzuki.
+
+
+        It is a fairly simple and limited patch, as it only works when the DFG can prove the exact object used as prototype.
+        But when it applies it can be a significant win:
+                                                        Baseline                   Optim                                       
+        object-create-constant-prototype              3.6082+-0.0979     ^      1.6947+-0.0756        ^ definitely 2.1292x faster
+        object-create-null                           11.4492+-0.2510     ?     11.5030+-0.2402        ?
+        object-create-unknown-object-prototype       15.6067+-0.1851     ?     15.7500+-0.2322        ?
+        object-create-untyped-prototype               8.8873+-0.1240     ?      8.9806+-0.1202        ? might be 1.0105x slower
+        <geometric>                                   8.6967+-0.1208     ^      7.2408+-0.1367        ^ definitely 1.2011x faster
+
+        The only subtlety is that we need to to access the StructureCache concurrently from the compiler thread (see https://bugs.webkit.org/show_bug.cgi?id=186199)
+        I solved this with a simple lock, taken when the compiler thread tries to read it, and when the main thread tries to modify it.
+        I expect it to be extremely low contention, but will watch the bots just in case.
+        The lock is taken neither when the main thread is only reading the cache (it has no-one to race with), nor when the GC purges it of dead entries (it does not free anything while a compiler thread is in the middle of a phase).
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGConstantFoldingPhase.cpp:
+        (JSC::DFG::ConstantFoldingPhase::foldConstants):
+        * runtime/StructureCache.cpp:
+        (JSC::StructureCache::createEmptyStructure):
+        (JSC::StructureCache::tryEmptyObjectStructureForPrototypeFromCompilerThread):
+        * runtime/StructureCache.h:
+
+2019-04-15  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: fake value descriptors for promises add a catch handler, preventing "rejectionhandled" events from being fired
+        https://bugs.webkit.org/show_bug.cgi?id=196484
+        <rdar://problem/49114725>
+
+        Reviewed by Joseph Pecoraro.
+
+        Only add a catch handler when the promise is reachable via a native getter and is known to
+        have rejected. A non-rejected promise doesn't need a catch handler, and any promise that
+        isn't reachable via a getter won't actually be reached, as `InjectedScript` doesn't call any
+        functions, instead only getting the function object itself.
+
+        * inspector/InjectedScriptSource.js:
+        (InjectedScript.prototype._propertyDescriptors.createFakeValueDescriptor):
+
+        * inspector/JSInjectedScriptHost.h:
+        * inspector/JSInjectedScriptHost.cpp:
+        (Inspector::JSInjectedScriptHost::isPromiseRejectedWithNativeGetterTypeError): Added.
+        * inspector/JSInjectedScriptHostPrototype.cpp:
+        (Inspector::JSInjectedScriptHostPrototype::finishCreation):
+        (Inspector::jsInjectedScriptHostPrototypeFunctionIsPromiseRejectedWithNativeGetterTypeError): Added.
+
+        * runtime/ErrorInstance.h:
+        (JSC::ErrorInstance::setNativeGetterTypeError): Added.
+        (JSC::ErrorInstance::isNativeGetterTypeError const): Added.
+
+        * runtime/Error.h:
+        (JSC::throwVMGetterTypeError): Added.
+        * runtime/Error.cpp:
+        (JSC::createGetterTypeError): Added.
+        (JSC::throwGetterTypeError): Added.
+        (JSC::throwDOMAttributeGetterTypeError):
+
+2019-04-15  Robin Morisset  <rmorisset@apple.com>
+
+        B3::Value should have different kinds of adjacency lists
+        https://bugs.webkit.org/show_bug.cgi?id=196091
+
+        Reviewed by Filip Pizlo.
+
+        The key idea of this optimization is to replace the Vector<Value*, 3> m_children in B3::Value (40 bytes on 64-bits platform) by one of the following:
+        - Nothing (0 bytes)
+        - 1 Value* (8 bytes)
+        - 2 Value* (16 bytes)
+        - 3 Value* (24 bytes)
+        - A Vector<Value*, 3>
+        after the end of the Value object, depending on the kind of the Value.
+        So for example, when allocating an Add, we would allocate an extra 16 bytes into which to store 2 Values.
+        This would halve the memory consumption of Const64/Const32/Nop/Identity and a bunch more kinds of values, and reduce by a more moderate amount the memory consumption of the rest of non-varargs values (e.g. Add would go from 72 to 48 bytes).
+
+        A few implementation points:
+        - Even if there is no children, we must remember to allocate at least enough space for replaceWithIdentity to work later. It needs sizeof(Value) (for the object itself) + sizeof(Value*) (for the pointer to its child)
+        - We must make sure to destroy the vector whenever we destroy a Value which is VarArgs
+        - We must remember how many elements there are in the case where we did not allocate a Vector. We cannot do it purely by relying on the kind, both for speed reasons and because Return can have either 0 or 1 argument in B3
+          Thankfully, we have an extra byte of padding to use in the middle of B3::Value
+        - In order to support clone(), we must have a separate version of allocate, which extracts the opcode from the to-be-cloned object instead of from the call to the constructor
+        - Speaking of which, we need a special templated function opcodeFromConstructor, because some of the constructors of subclasses of Value don't take an explicit Opcode as argument, typically because they match a single one.
+        - To maximize performance, we provide specialized versions of child/lastChild/numChildren/children in the subclasses of Value, skipping checks when the actual type of the Value is already known.
+          This is done through the B3_SPECIALIZE_VALUE_FOR_... defined at the bottom of B3Value.h
+        - In the constructors of Value, we convert all extra children arguments to Value* eagerly. It is not required for correctness (they will be converted when put into a Vector<Value*> or a Value* in the end), but it helps limit an explosion in the number of template instantiations.
+        - I moved DeepValueDump::dump from the .h to the .cpp, as there is no good reason to inline it, and recompiling JSC is already slow enough
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * b3/B3ArgumentRegValue.cpp:
+        (JSC::B3::ArgumentRegValue::cloneImpl const): Deleted.
+        * b3/B3ArgumentRegValue.h:
+        * b3/B3AtomicValue.cpp:
+        (JSC::B3::AtomicValue::AtomicValue):
+        (JSC::B3::AtomicValue::cloneImpl const): Deleted.
+        * b3/B3AtomicValue.h:
+        * b3/B3BasicBlock.h:
+        * b3/B3BasicBlockInlines.h:
+        (JSC::B3::BasicBlock::appendNewNonTerminal): Deleted.
+        * b3/B3CCallValue.cpp:
+        (JSC::B3::CCallValue::appendArgs):
+        (JSC::B3::CCallValue::cloneImpl const): Deleted.
+        * b3/B3CCallValue.h:
+        * b3/B3CheckValue.cpp:
+        (JSC::B3::CheckValue::cloneImpl const): Deleted.
+        * b3/B3CheckValue.h:
+        * b3/B3Const32Value.cpp:
+        (JSC::B3::Const32Value::cloneImpl const): Deleted.
+        * b3/B3Const32Value.h:
+        * b3/B3Const64Value.cpp:
+        (JSC::B3::Const64Value::cloneImpl const): Deleted.
+        * b3/B3Const64Value.h:
+        * b3/B3ConstDoubleValue.cpp:
+        (JSC::B3::ConstDoubleValue::cloneImpl const): Deleted.
+        * b3/B3ConstDoubleValue.h:
+        * b3/B3ConstFloatValue.cpp:
+        (JSC::B3::ConstFloatValue::cloneImpl const): Deleted.
+        * b3/B3ConstFloatValue.h:
+        * b3/B3ConstPtrValue.h:
+        (JSC::B3::ConstPtrValue::opcodeFromConstructor):
+        * b3/B3FenceValue.cpp:
+        (JSC::B3::FenceValue::FenceValue):
+        (JSC::B3::FenceValue::cloneImpl const): Deleted.
+        * b3/B3FenceValue.h:
+        * b3/B3MemoryValue.cpp:
+        (JSC::B3::MemoryValue::MemoryValue):
+        (JSC::B3::MemoryValue::cloneImpl const): Deleted.
+        * b3/B3MemoryValue.h:
+        * b3/B3MoveConstants.cpp:
+        * b3/B3PatchpointValue.cpp:
+        (JSC::B3::PatchpointValue::cloneImpl const): Deleted.
+        * b3/B3PatchpointValue.h:
+        (JSC::B3::PatchpointValue::opcodeFromConstructor):
+        * b3/B3Procedure.cpp:
+        * b3/B3Procedure.h:
+        * b3/B3ProcedureInlines.h:
+        (JSC::B3::Procedure::add):
+        * b3/B3SlotBaseValue.cpp:
+        (JSC::B3::SlotBaseValue::cloneImpl const): Deleted.
+        * b3/B3SlotBaseValue.h:
+        * b3/B3StackmapSpecial.cpp:
+        (JSC::B3::StackmapSpecial::forEachArgImpl):
+        (JSC::B3::StackmapSpecial::isValidImpl):
+        * b3/B3StackmapValue.cpp:
+        (JSC::B3::StackmapValue::append):
+        (JSC::B3::StackmapValue::StackmapValue):
+        * b3/B3StackmapValue.h:
+        * b3/B3SwitchValue.cpp:
+        (JSC::B3::SwitchValue::SwitchValue):
+        (JSC::B3::SwitchValue::cloneImpl const): Deleted.
+        * b3/B3SwitchValue.h:
+        (JSC::B3::SwitchValue::opcodeFromConstructor):
+        * b3/B3UpsilonValue.cpp:
+        (JSC::B3::UpsilonValue::cloneImpl const): Deleted.
+        * b3/B3UpsilonValue.h:
+        * b3/B3Value.cpp:
+        (JSC::B3::DeepValueDump::dump const):
+        (JSC::B3::Value::~Value):
+        (JSC::B3::Value::replaceWithIdentity):
+        (JSC::B3::Value::replaceWithNopIgnoringType):
+        (JSC::B3::Value::replaceWithPhi):
+        (JSC::B3::Value::replaceWithJump):
+        (JSC::B3::Value::replaceWithOops):
+        (JSC::B3::Value::replaceWith):
+        (JSC::B3::Value::invertedCompare const):
+        (JSC::B3::Value::returnsBool const):
+        (JSC::B3::Value::cloneImpl const): Deleted.
+        * b3/B3Value.h:
+        (JSC::B3::DeepValueDump::dump const): Deleted.
+        * b3/B3ValueInlines.h:
+        (JSC::B3::Value::adjacencyListOffset const):
+        (JSC::B3::Value::cloneImpl const):
+        * b3/B3VariableValue.cpp:
+        (JSC::B3::VariableValue::VariableValue):
+        (JSC::B3::VariableValue::cloneImpl const): Deleted.
+        * b3/B3VariableValue.h:
+        * b3/B3WasmAddressValue.cpp:
+        (JSC::B3::WasmAddressValue::WasmAddressValue):
+        (JSC::B3::WasmAddressValue::cloneImpl const): Deleted.
+        * b3/B3WasmAddressValue.h:
+        * b3/B3WasmBoundsCheckValue.cpp:
+        (JSC::B3::WasmBoundsCheckValue::WasmBoundsCheckValue):
+        (JSC::B3::WasmBoundsCheckValue::cloneImpl const): Deleted.
+        * b3/B3WasmBoundsCheckValue.h:
+        (JSC::B3::WasmBoundsCheckValue::accepts):
+        (JSC::B3::WasmBoundsCheckValue::opcodeFromConstructor):
+        * b3/testb3.cpp:
+        (JSC::B3::testCallFunctionWithHellaArguments):
+        (JSC::B3::testCallFunctionWithHellaArguments2):
+        (JSC::B3::testCallFunctionWithHellaArguments3):
+        (JSC::B3::testCallFunctionWithHellaDoubleArguments):
+        (JSC::B3::testCallFunctionWithHellaFloatArguments):
+        * ftl/FTLOutput.h:
+        (JSC::FTL::Output::call):
+
+2019-04-15  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Bytecode cache should not encode the SourceProvider for UnlinkedFunctionExecutable's classSource
+        https://bugs.webkit.org/show_bug.cgi?id=196878
+
+        Reviewed by Saam Barati.
+
+        Every time we encode an (Unlinked)SourceCode, we encode its SourceProvider,
+        including the full source if it's a StringSourceProvider. This wasn't an issue,
+        since the SourceCode contains a RefPtr to the SourceProvider, and the Encoder
+        would avoid encoding the provider multiple times. With the addition of the
+        incremental cache, each UnlinkedFunctionCodeBlock is encoded in isolation, which
+        means we can no longer deduplicate it and the full program text was being encoded
+        multiple times in the cache.
+        As a work around, this patch adds a custom cached type for encoding the SourceCode
+        without its provider, and later injects the SourceProvider through the Decoder.
+
+        * parser/SourceCode.h:
+        * parser/UnlinkedSourceCode.h:
+        (JSC::UnlinkedSourceCode::provider const):
+        * runtime/CachedTypes.cpp:
+        (JSC::Decoder::Decoder):
+        (JSC::Decoder::create):
+        (JSC::Decoder::provider const):
+        (JSC::CachedSourceCodeWithoutProvider::encode):
+        (JSC::CachedSourceCodeWithoutProvider::decode const):
+        (JSC::decodeCodeBlockImpl):
+        * runtime/CachedTypes.h:
+
+2019-04-15  Robin Morisset  <rmorisset@apple.com>
+
+        MarkedSpace.cpp is not in the Xcode workspace
+        https://bugs.webkit.org/show_bug.cgi?id=196928
+
+        Reviewed by Saam Barati.
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+
+2019-04-15  Tadeu Zagallo  <tzagallo@apple.com>
+
+        Incremental bytecode cache should not append function updates when loaded from memory
+        https://bugs.webkit.org/show_bug.cgi?id=196865
+
+        Reviewed by Filip Pizlo.
+
+        Function updates hold the assumption that a function can only be executed/cached
+        after its containing code block has already been cached. This assumptions does
+        not hold if the UnlinkedCodeBlock is loaded from memory by the CodeCache, since
+        we might have two independent SourceProviders executing different paths of the
+        code and causing the same UnlinkedCodeBlock to be modified in memory.
+        Use a RefPtr instead of Ref for m_cachedBytecode in ShellSourceProvider to distinguish
+        between a new, empty cache and a cache that was not loaded and therefore cannot be updated.
+
+        * jsc.cpp:
+        (ShellSourceProvider::ShellSourceProvider):
+
+2019-04-15  Saam barati  <sbarati@apple.com>
+
+        mergeOSREntryValue is wrong when the incoming value does not match up with the flush format
+        https://bugs.webkit.org/show_bug.cgi?id=196918
+
+        Reviewed by Yusuke Suzuki.
+
+        r244238 lead to some debug failures because we were calling checkConsistency()
+        before doing fixTypeForRepresentation when merging in must handle values in
+        CFA. This patch fixes that.
+        
+        However, as I was reading over mergeOSREntryValue, I realized it was wrong. It
+        was possible it could merge in a value/type outside of the variable's flushed type.
+        Once the flush format types are locked in, we can't introduce a type out of
+        that range. This probably never lead to any crashes as our profiling injection
+        and speculation decision code is solid. However, what we were doing is clearly
+        wrong, and something a fuzzer could have found if we fuzzed the must handle
+        values inside prediction injection. We should do that fuzzing:
+        https://bugs.webkit.org/show_bug.cgi?id=196924
+
+        * dfg/DFGAbstractValue.cpp:
+        (JSC::DFG::AbstractValue::mergeOSREntryValue):
+        * dfg/DFGAbstractValue.h:
+        * dfg/DFGCFAPhase.cpp:
+        (JSC::DFG::CFAPhase::injectOSR):
+
+2019-04-15  Robin Morisset  <rmorisset@apple.com>
+
+        Several structures and enums in the Yarr interpreter can be shrunk
+        https://bugs.webkit.org/show_bug.cgi?id=196923
+
+        Reviewed by Saam Barati.
+
+        YarrOp: 88 -> 80
+        RegularExpression: 40 -> 32
+        ByteTerm: 56 -> 48
+        PatternTerm: 56 -> 48
+
+        * yarr/RegularExpression.cpp:
+        * yarr/YarrInterpreter.h:
+        * yarr/YarrJIT.cpp:
+        (JSC::Yarr::YarrGenerator::YarrOp::YarrOp):
+        * yarr/YarrParser.h:
+        * yarr/YarrPattern.h:
+
+2019-04-15  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: REGRESSION(r244172): crash when trying to add extra domain while inspecting JSContext
+        https://bugs.webkit.org/show_bug.cgi?id=196925
+        <rdar://problem/49873994>
+
+        Reviewed by Joseph Pecoraro.
+
+        Move the logic for creating the `InspectorAgent` and `InspectorDebuggerAgent` into separate
+        functions so that callers can be guaranteed to have a valid instance of the agent.
+
+        * inspector/JSGlobalObjectInspectorController.h:
+        * inspector/JSGlobalObjectInspectorController.cpp:
+        (Inspector::JSGlobalObjectInspectorController::connectFrontend):
+        (Inspector::JSGlobalObjectInspectorController::frontendInitialized):
+        (Inspector::JSGlobalObjectInspectorController::appendExtraAgent):
+        (Inspector::JSGlobalObjectInspectorController::ensureInspectorAgent): Added.
+        (Inspector::JSGlobalObjectInspectorController::ensureDebuggerAgent): Added.
+        (Inspector::JSGlobalObjectInspectorController::createLazyAgents):
+
+2019-04-14  Don Olmstead  <don.olmstead@sony.com>
+
+        [CMake] JavaScriptCore derived sources should only be referenced inside JavaScriptCore
+        https://bugs.webkit.org/show_bug.cgi?id=196742
+
+        Reviewed by Konstantin Tokarev.
+
+        Migrate to using JavaScriptCore_DERIVED_SOURCES_DIR instead of DERIVED_SOURCES_JAVASCRIPTCORE_DIR
+        to support moving the JavaScriptCore derived sources outside of a shared directory.
+
+        Also use JavaScriptCore_DERIVED_SOURCES_DIR instead of DERIVED_SOUCES_DIR.
+
+        * CMakeLists.txt:
+
+2019-04-13  Tadeu Zagallo  <tzagallo@apple.com>
+
+        CodeCache should check that the UnlinkedCodeBlock was successfully created before caching it
+        https://bugs.webkit.org/show_bug.cgi?id=196880
+
+        Reviewed by Yusuke Suzuki.
+
+        CodeCache should not tell the SourceProvider to cache the bytecode if it failed
+        to create the UnlinkedCodeBlock.
+
+        * runtime/CodeCache.cpp:
+        (JSC::CodeCache::getUnlinkedGlobalCodeBlock):
+
+2019-04-12  Saam barati  <sbarati@apple.com>
+
+        r244079 logically broke shouldSpeculateInt52
+        https://bugs.webkit.org/show_bug.cgi?id=196884
+
+        Reviewed by Yusuke Suzuki.
+
+        In r244079, I changed shouldSpeculateInt52 to only return true
+        when the prediction is isAnyInt52Speculation(). However, it was
+        wrong to not to include SpecInt32 in this for two reasons:
+
+        1. We diligently write code that first checks if we should speculate Int32.
+        For example:
+        if (shouldSpeculateInt32()) ... 
+        else if (shouldSpeculateInt52()) ...
+
+        It would be wrong not to fall back to Int52 if we're dealing with the union of
+        Int32 and Int52.
+
+        It would be a performance mistake to not include Int32 here because
+        data flow can easily tell us that we have variables that are the union
+        of Int32 and Int52 values. It's better to speculate Int52 than Double
+        in that situation.
+
+        2. We also write code where we ask if the inputs can be Int52, e.g, if
+        we know via profiling that an Add overflows, we may not emit an Int32 add.
+        However, we only emit such an add if both inputs can be Int52, and Int32
+        can trivially become Int52.
+
+       This patch recovers the 0.5-1% regression r244079 caused on JetStream 2.
+
+        * bytecode/SpeculatedType.h:
+        (JSC::isInt32SpeculationForArithmetic):
+        (JSC::isInt32OrBooleanSpeculationForArithmetic):
+        (JSC::isInt32OrInt52Speculation):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::observeUseKindOnNode):
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::shouldSpeculateInt52):
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        * dfg/DFGVariableAccessData.cpp:
+        (JSC::DFG::VariableAccessData::couldRepresentInt52Impl):
+
+2019-04-12  Saam barati  <sbarati@apple.com>
+
+        Unreviewed. Build fix after r244233.
+
+        * assembler/CPU.cpp:
+
+2019-04-12  Saam barati  <sbarati@apple.com>
+
+        Sometimes we need to user fewer CPUs in our threading calculations
+        https://bugs.webkit.org/show_bug.cgi?id=196794
+        <rdar://problem/49389497>
+
+        Reviewed by Yusuke Suzuki.
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * Sources.txt:
+        * assembler/CPU.cpp: Added.
+        (JSC::isKernTCSMAvailable):
+        (JSC::enableKernTCSM):
+        (JSC::kernTCSMAwareNumberOfProcessorCores):
+        * assembler/CPU.h:
+        (JSC::isKernTCSMAvailable):
+        (JSC::enableKernTCSM):
+        (JSC::kernTCSMAwareNumberOfProcessorCores):
+        * heap/MachineStackMarker.h:
+        (JSC::MachineThreads::addCurrentThread):
+        * runtime/JSLock.cpp:
+        (JSC::JSLock::didAcquireLock):
+        * runtime/Options.cpp:
+        (JSC::computeNumberOfWorkerThreads):
+        (JSC::computePriorityDeltaOfWorkerThreads):
+        * wasm/WasmWorklist.cpp:
+        (JSC::Wasm::Worklist::Worklist):
+
+2019-04-12  Robin Morisset  <rmorisset@apple.com>
+
+        Use padding at end of ArrayBuffer
+        https://bugs.webkit.org/show_bug.cgi?id=196823
+
+        Reviewed by Filip Pizlo.
+
+        * runtime/ArrayBuffer.h:
+
 2019-04-11  Yusuke Suzuki  <ysuzuki@apple.com>
 
         [JSC] op_has_indexed_property should not assume subscript part is Uint32