diff --git a/v8/BUILD.gn b/v8/BUILD.gn index 159ea937fa..7aa9c61544 100644 --- a/v8/BUILD.gn +++ b/v8/BUILD.gn @@ -210,7 +210,7 @@ declare_args() { # site-isolation in Chrome and on simulator builds which test code generation # on these platforms. v8_untrusted_code_mitigations = - v8_current_cpu != "x86" && (is_android || target_is_simulator) + v8_current_cpu != "x86" && (is_android || target_is_simulator || is_ohos) # Enable minor mark compact. v8_enable_minor_mc = true @@ -398,7 +398,7 @@ assert(v8_current_cpu == "arm64" || !v8_control_flow_integrity, if (v8_enable_shared_ro_heap && v8_enable_pointer_compression) { assert( - is_linux || is_chromeos || is_android, + is_linux || is_chromeos || is_android || is_ohos, "Sharing read-only heap with pointer compression is only supported on Linux or Android") } @@ -989,7 +989,7 @@ config("toolchain") { # mksnapshot. We additionally set V8_HAVE_TARGET_OS to determine that a # target OS has in fact been set; otherwise we internally assume that target # OS == host OS (see v8config.h). - if (target_os == "android") { + if (target_os == "android" || target_os == "ohos") { defines += [ "V8_HAVE_TARGET_OS" ] defines += [ "V8_TARGET_OS_ANDROID" ] } else if (target_os == "fuchsia") { @@ -1114,7 +1114,7 @@ config("always_optimize") { # TODO(crbug.com/621335) Rework this so that we don't have the confusion # between "optimize_speed" and "optimize_max". - if (((is_posix && !is_android) || is_fuchsia) && !using_sanitizer) { + if (((is_posix && !is_android && !is_ohos) || is_fuchsia) && !using_sanitizer) { configs += [ "//build/config/compiler:optimize_speed" ] } else { configs += [ "//build/config/compiler:optimize_max" ] @@ -4059,7 +4059,7 @@ v8_source_set("v8_base_without_compiler") { # iOS Xcode simulator builds run on an x64 target. iOS and macOS are both # based on Darwin and thus POSIX-compliant to a similar degree. - if (is_linux || is_chromeos || is_mac || is_ios || target_os == "freebsd") { + if (is_linux || is_chromeos || is_mac || is_ios || target_os == "freebsd" || is_ohos) { sources += [ "src/trap-handler/handler-inside-posix.cc", "src/trap-handler/handler-outside-posix.cc", @@ -4602,7 +4602,7 @@ v8_component("v8_libbase") { ] libs = [ "dl" ] - } else if (is_android) { + } else if (is_android || is_ohos) { if (current_toolchain == host_toolchain) { libs = [ "dl", diff --git a/v8/include/v8config.h b/v8/include/v8config.h index acd34d7a1f..2eb2d802d6 100644 --- a/v8/include/v8config.h +++ b/v8/include/v8config.h @@ -82,7 +82,7 @@ path. Add it with -I to the command line // V8_OS_AIX - AIX // V8_OS_WIN - Microsoft Windows -#if defined(__ANDROID__) +#if defined(__ANDROID__) || defined(OS_OHOS) || defined(__OHOS__) # define V8_OS_ANDROID 1 # define V8_OS_LINUX 1 # define V8_OS_POSIX 1 diff --git a/v8/src/builtins/builtins-collections-gen.cc b/v8/src/builtins/builtins-collections-gen.cc index 785a1af90a..8e343d9d2d 100644 --- a/v8/src/builtins/builtins-collections-gen.cc +++ b/v8/src/builtins/builtins-collections-gen.cc @@ -1752,6 +1752,9 @@ TF_BUILTIN(MapPrototypeDelete, CollectionsBuiltinsAssembler) { ThrowIfNotInstanceType(context, receiver, JS_MAP_TYPE, "Map.prototype.delete"); + // This check breaks a known exploitation technique. See crbug.com/1263462 + CSA_CHECK(this, TaggedNotEqual(key, TheHoleConstant())); + const TNode table = LoadObjectField(CAST(receiver), JSMap::kTableOffset); @@ -1920,6 +1923,9 @@ TF_BUILTIN(SetPrototypeDelete, CollectionsBuiltinsAssembler) { ThrowIfNotInstanceType(context, receiver, JS_SET_TYPE, "Set.prototype.delete"); + // This check breaks a known exploitation technique. See crbug.com/1263462 + CSA_CHECK(this, TaggedNotEqual(key, TheHoleConstant())); + const TNode table = LoadObjectField(CAST(receiver), JSMap::kTableOffset); @@ -2866,6 +2872,9 @@ TF_BUILTIN(WeakMapPrototypeDelete, CodeStubAssembler) { ThrowIfNotInstanceType(context, receiver, JS_WEAK_MAP_TYPE, "WeakMap.prototype.delete"); + // This check breaks a known exploitation technique. See crbug.com/1263462 + CSA_CHECK(this, TaggedNotEqual(key, TheHoleConstant())); + Return(CallBuiltin(Builtins::kWeakCollectionDelete, context, receiver, key)); } @@ -2914,6 +2923,9 @@ TF_BUILTIN(WeakSetPrototypeDelete, CodeStubAssembler) { ThrowIfNotInstanceType(context, receiver, JS_WEAK_SET_TYPE, "WeakSet.prototype.delete"); + // This check breaks a known exploitation technique. See crbug.com/1263462 + CSA_CHECK(this, TaggedNotEqual(value, TheHoleConstant())); + Return( CallBuiltin(Builtins::kWeakCollectionDelete, context, receiver, value)); } diff --git a/v8/src/builtins/finalization-registry.tq b/v8/src/builtins/finalization-registry.tq index 84499e19e1..389b9a5ce0 100644 --- a/v8/src/builtins/finalization-registry.tq +++ b/v8/src/builtins/finalization-registry.tq @@ -143,21 +143,22 @@ FinalizationRegistryRegister( ThrowTypeError( MessageTemplate::kWeakRefsRegisterTargetAndHoldingsMustNotBeSame); } - const unregisterToken = arguments[2]; // 5. If Type(unregisterToken) is not Object, // a. If unregisterToken is not undefined, throw a TypeError exception. // b. Set unregisterToken to empty. - let hasUnregisterToken: bool = false; - typeswitch (unregisterToken) { + const unregisterTokenRaw = arguments[2]; + let unregisterToken: JSReceiver|Undefined; + typeswitch (unregisterTokenRaw) { case (Undefined): { + unregisterToken = Undefined; } - case (JSReceiver): { - hasUnregisterToken = true; + case (unregisterTokenObj: JSReceiver): { + unregisterToken = unregisterTokenObj; } case (JSAny): deferred { ThrowTypeError( MessageTemplate::kWeakRefsUnregisterTokenMustBeObject, - unregisterToken); + unregisterTokenRaw); } } // 6. Let cell be the Record { [[WeakRefTarget]] : target, [[HeldValue]]: @@ -178,7 +179,7 @@ FinalizationRegistryRegister( }; // 7. Append cell to finalizationRegistry.[[Cells]]. PushCell(finalizationRegistry, cell); - if (hasUnregisterToken) { + if (unregisterToken != Undefined) { // If an unregister token is provided, a runtime call is needed to // do some OrderedHashTable operations and register the mapping. // See v8:10705. diff --git a/v8/src/codegen/external-reference-table.cc b/v8/src/codegen/external-reference-table.cc index 2741bd8ec2..4c544f16d5 100644 --- a/v8/src/codegen/external-reference-table.cc +++ b/v8/src/codegen/external-reference-table.cc @@ -9,7 +9,7 @@ #include "src/ic/stub-cache.h" #include "src/logging/counters.h" -#if defined(DEBUG) && defined(V8_OS_LINUX) && !defined(V8_OS_ANDROID) +#if defined(DEBUG) && defined(V8_OS_LINUX) && !defined(V8_OS_ANDROID) && !defined(__MUSL__) #define SYMBOLIZE_FUNCTION #include diff --git a/v8/src/compiler/backend/ia32/instruction-selector-ia32.cc b/v8/src/compiler/backend/ia32/instruction-selector-ia32.cc index 6e51346c50..7c07a701bb 100644 --- a/v8/src/compiler/backend/ia32/instruction-selector-ia32.cc +++ b/v8/src/compiler/backend/ia32/instruction-selector-ia32.cc @@ -98,11 +98,14 @@ class IA32OperandGenerator final : public OperandGenerator { bool CanBeImmediate(Node* node) { switch (node->opcode()) { case IrOpcode::kInt32Constant: - case IrOpcode::kNumberConstant: case IrOpcode::kExternalConstant: case IrOpcode::kRelocatableInt32Constant: case IrOpcode::kRelocatableInt64Constant: return true; + case IrOpcode::kNumberConstant: { + const double value = OpParameter(node->op()); + return bit_cast(value) == 0; + } case IrOpcode::kHeapConstant: { // TODO(bmeurer): We must not dereference handles concurrently. If we // really have to this here, then we need to find a way to put this diff --git a/v8/src/compiler/backend/instruction-selector.cc b/v8/src/compiler/backend/instruction-selector.cc index 4c40835634..0abf01e4d6 100644 --- a/v8/src/compiler/backend/instruction-selector.cc +++ b/v8/src/compiler/backend/instruction-selector.cc @@ -274,7 +274,7 @@ Instruction* InstructionSelector::Emit(Instruction* instr) { bool InstructionSelector::CanCover(Node* user, Node* node) const { // 1. Both {user} and {node} must be in the same basic block. - if (schedule()->block(node) != schedule()->block(user)) { + if (schedule()->block(node) != current_block_) { return false; } // 2. Pure {node}s must be owned by the {user}. @@ -282,7 +282,7 @@ bool InstructionSelector::CanCover(Node* user, Node* node) const { return node->OwnedBy(user); } // 3. Impure {node}s must match the effect level of {user}. - if (GetEffectLevel(node) != GetEffectLevel(user)) { + if (GetEffectLevel(node) != current_effect_level_) { return false; } // 4. Only {node} must have value edges pointing to {user}. @@ -294,21 +294,6 @@ bool InstructionSelector::CanCover(Node* user, Node* node) const { return true; } -bool InstructionSelector::CanCoverTransitively(Node* user, Node* node, - Node* node_input) const { - if (CanCover(user, node) && CanCover(node, node_input)) { - // If {node} is pure, transitivity might not hold. - if (node->op()->HasProperty(Operator::kPure)) { - // If {node_input} is pure, the effect levels do not matter. - if (node_input->op()->HasProperty(Operator::kPure)) return true; - // Otherwise, {user} and {node_input} must have the same effect level. - return GetEffectLevel(user) == GetEffectLevel(node_input); - } - return true; - } - return false; -} - bool InstructionSelector::IsOnlyUserOfNodeInSameBlock(Node* user, Node* node) const { BasicBlock* bb_user = schedule()->block(user); @@ -1212,6 +1197,7 @@ void InstructionSelector::VisitBlock(BasicBlock* block) { int effect_level = 0; for (Node* const node : *block) { SetEffectLevel(node, effect_level); + current_effect_level_ = effect_level; if (node->opcode() == IrOpcode::kStore || node->opcode() == IrOpcode::kUnalignedStore || node->opcode() == IrOpcode::kCall || @@ -1231,6 +1217,7 @@ void InstructionSelector::VisitBlock(BasicBlock* block) { // control input should be on the same effect level as the last node. if (block->control_input() != nullptr) { SetEffectLevel(block->control_input(), effect_level); + current_effect_level_ = effect_level; } auto FinishEmittedInstructions = [&](Node* node, int instruction_start) { diff --git a/v8/src/compiler/backend/instruction-selector.h b/v8/src/compiler/backend/instruction-selector.h index c7bc99005d..51aafc36b5 100644 --- a/v8/src/compiler/backend/instruction-selector.h +++ b/v8/src/compiler/backend/instruction-selector.h @@ -417,12 +417,12 @@ class V8_EXPORT_PRIVATE InstructionSelector final { // Used in pattern matching during code generation. // Check if {node} can be covered while generating code for the current // instruction. A node can be covered if the {user} of the node has the only - // edge and the two are in the same basic block. + // edge, the two are in the same basic block, and there are no side-effects + // in-between. The last check is crucial for soundness. + // For pure nodes, CanCover(a,b) is checked to avoid duplicated execution: + // If this is not the case, code for b must still be generated for other + // users, and fusing is unlikely to improve performance. bool CanCover(Node* user, Node* node) const; - // CanCover is not transitive. The counter example are Nodes A,B,C such that - // CanCover(A, B) and CanCover(B,C) and B is pure: The the effect level of A - // and B might differ. CanCoverTransitively does the additional checks. - bool CanCoverTransitively(Node* user, Node* node, Node* node_input) const; // Used in pattern matching during code generation. // This function checks that {node} and {user} are in the same basic block, @@ -741,6 +741,7 @@ class V8_EXPORT_PRIVATE InstructionSelector final { BoolVector defined_; BoolVector used_; IntVector effect_level_; + int current_effect_level_; IntVector virtual_registers_; IntVector virtual_register_rename_; InstructionScheduler* scheduler_; diff --git a/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc b/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc index 1fa6ce8f3d..d43a8a56e0 100644 --- a/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc +++ b/v8/src/compiler/backend/mips64/instruction-selector-mips64.cc @@ -1523,7 +1523,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) { if (CanCover(node, value)) { switch (value->opcode()) { case IrOpcode::kWord64Sar: { - if (CanCoverTransitively(node, value, value->InputAt(0)) && + if (CanCover(value, value->InputAt(0)) && TryEmitExtendingLoad(this, value, node)) { return; } else { diff --git a/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc b/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc index 1d6b506685..7d2e5e7853 100644 --- a/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc +++ b/v8/src/compiler/backend/riscv64/instruction-selector-riscv64.cc @@ -1277,7 +1277,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) { if (CanCover(node, value)) { switch (value->opcode()) { case IrOpcode::kWord64Sar: { - if (CanCoverTransitively(node, value, value->InputAt(0)) && + if (CanCover(value, value->InputAt(0)) && TryEmitExtendingLoad(this, value, node)) { return; } else { diff --git a/v8/src/compiler/backend/x64/instruction-selector-x64.cc b/v8/src/compiler/backend/x64/instruction-selector-x64.cc index 3f005475b8..ca33549e3a 100644 --- a/v8/src/compiler/backend/x64/instruction-selector-x64.cc +++ b/v8/src/compiler/backend/x64/instruction-selector-x64.cc @@ -1685,7 +1685,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) { case IrOpcode::kWord64Shr: { Int64BinopMatcher m(value); if (m.right().Is(32)) { - if (CanCoverTransitively(node, value, value->InputAt(0)) && + if (CanCover(value, value->InputAt(0)) && TryMatchLoadWord64AndShiftRight(this, value, kX64Movl)) { return EmitIdentity(node); } diff --git a/v8/src/compiler/escape-analysis.cc b/v8/src/compiler/escape-analysis.cc index 7ff6ab684f..23dfb00184 100644 --- a/v8/src/compiler/escape-analysis.cc +++ b/v8/src/compiler/escape-analysis.cc @@ -5,10 +5,12 @@ #include "src/compiler/escape-analysis.h" #include "src/codegen/tick-counter.h" +#include "src/compiler/frame-states.h" #include "src/compiler/linkage.h" #include "src/compiler/node-matchers.h" #include "src/compiler/operator-properties.h" #include "src/compiler/simplified-operator.h" +#include "src/compiler/state-values-utils.h" #include "src/handles/handles-inl.h" #include "src/init/bootstrapper.h" #include "src/objects/map-inl.h" @@ -224,6 +226,11 @@ class EscapeAnalysisTracker : public ZoneObject { return tracker_->ResolveReplacement( NodeProperties::GetContextInput(current_node())); } + // Accessing the current node is fine for `FrameState nodes. + Node* CurrentNode() { + DCHECK_EQ(current_node()->opcode(), IrOpcode::kFrameState); + return current_node(); + } void SetReplacement(Node* replacement) { replacement_ = replacement; @@ -796,9 +803,30 @@ void ReduceNode(const Operator* op, EscapeAnalysisTracker::Scope* current, break; } case IrOpcode::kStateValues: - case IrOpcode::kFrameState: // These uses are always safe. break; + case IrOpcode::kFrameState: { + // We mark the receiver as escaping due to the non-standard `.getThis` + // API. + FrameState frame_state{current->CurrentNode()}; + FrameStateType type = frame_state.frame_state_info().type(); + // This needs to be kept in sync with the frame types supported in + // `OptimizedFrame::Summarize`. + if (type != FrameStateType::kUnoptimizedFunction && + type != FrameStateType::kJavaScriptBuiltinContinuation && + type != FrameStateType::kJavaScriptBuiltinContinuationWithCatch) { + break; + } + StateValuesAccess::iterator it = + StateValuesAccess(frame_state.parameters()).begin(); + if (!it.done()) { + if (Node* receiver = it.node()) { + current->SetEscaped(receiver); + } + current->SetEscaped(frame_state.function()); + } + break; + } default: { // For unknown nodes, treat all value inputs as escaping. int value_input_count = op->ValueInputCount(); diff --git a/v8/src/compiler/js-call-reducer.cc b/v8/src/compiler/js-call-reducer.cc index 43aa4a5990..46cd5a71a6 100644 --- a/v8/src/compiler/js-call-reducer.cc +++ b/v8/src/compiler/js-call-reducer.cc @@ -5924,11 +5924,12 @@ Reduction JSCallReducer::ReduceArrayIteratorPrototypeNext(Node* node) { Node* etrue = effect; Node* if_true = graph()->NewNode(common()->IfTrue(), branch); { - // We know that the {index} is range of the {length} now. + // This extra check exists to refine the type of {index} but also to break + // an exploitation technique that abuses typer mismatches. index = etrue = graph()->NewNode( - common()->TypeGuard( - Type::Range(0.0, length_access.type.Max() - 1.0, graph()->zone())), - index, etrue, if_true); + simplified()->CheckBounds(p.feedback(), + CheckBoundsFlag::kAbortOnOutOfBounds), + index, length, etrue, if_true); done_true = jsgraph()->FalseConstant(); if (iteration_kind == IterationKind::kKeys) { diff --git a/v8/src/compiler/js-heap-broker.cc b/v8/src/compiler/js-heap-broker.cc index 6cfd6d87c0..21b44fdeaf 100644 --- a/v8/src/compiler/js-heap-broker.cc +++ b/v8/src/compiler/js-heap-broker.cc @@ -335,13 +335,9 @@ bool PropertyCellData::Serialize(JSHeapBroker* broker) { } } - if (property_details.cell_type() == PropertyCellType::kConstant) { - Handle value_again = - broker->CanonicalPersistentHandle(cell->value(kAcquireLoad)); - if (*value != *value_again) { - DCHECK(!broker->IsMainThread()); - return false; - } + if (property_details.cell_type() == PropertyCellType::kInTransition) { + DCHECK(!broker->IsMainThread()); + return false; } ObjectData* value_data = broker->TryGetOrCreateData(value, false); diff --git a/v8/src/compiler/js-native-context-specialization.cc b/v8/src/compiler/js-native-context-specialization.cc index 7e96f0c9d7..747b4facf8 100644 --- a/v8/src/compiler/js-native-context-specialization.cc +++ b/v8/src/compiler/js-native-context-specialization.cc @@ -834,6 +834,12 @@ Reduction JSNativeContextSpecialization::ReduceGlobalAccess( return NoChange(); } else if (property_cell_type == PropertyCellType::kUndefined) { return NoChange(); + } else if (property_cell_type == PropertyCellType::kConstantType) { + // We rely on stability further below. + if (property_cell_value.IsHeapObject() && + !property_cell_value.AsHeapObject().map().is_stable()) { + return NoChange(); + } } } else if (access_mode == AccessMode::kHas) { DCHECK_EQ(receiver, lookup_start_object); @@ -950,17 +956,7 @@ Reduction JSNativeContextSpecialization::ReduceGlobalAccess( if (property_cell_value.IsHeapObject()) { MapRef property_cell_value_map = property_cell_value.AsHeapObject().map(); - if (property_cell_value_map.is_stable()) { - dependencies()->DependOnStableMap(property_cell_value_map); - } else { - // The value's map is already unstable. If this store were to go - // through the C++ runtime, it would transition the PropertyCell to - // kMutable. We don't want to change the cell type from generated - // code (to simplify concurrent heap access), however, so we keep - // it as kConstantType and do the store anyways (if the new value's - // map matches). This is safe because it merely prolongs the limbo - // state that we are in already. - } + dependencies()->DependOnStableMap(property_cell_value_map); // Check that the {value} is a HeapObject. value = effect = graph()->NewNode(simplified()->CheckHeapObject(), @@ -999,6 +995,7 @@ Reduction JSNativeContextSpecialization::ReduceGlobalAccess( break; } case PropertyCellType::kUndefined: + case PropertyCellType::kInTransition: UNREACHABLE(); } } diff --git a/v8/src/compiler/machine-operator-reducer.cc b/v8/src/compiler/machine-operator-reducer.cc index 2220cdb82f..8bf33d53a0 100644 --- a/v8/src/compiler/machine-operator-reducer.cc +++ b/v8/src/compiler/machine-operator-reducer.cc @@ -1697,11 +1697,21 @@ Reduction MachineOperatorReducer::ReduceWordNAnd(Node* node) { namespace { // Represents an operation of the form `(source & mask) == masked_value`. +// where each bit set in masked_value also has to be set in mask. struct BitfieldCheck { - Node* source; - uint32_t mask; - uint32_t masked_value; - bool truncate_from_64_bit; + Node* const source; + uint32_t const mask; + uint32_t const masked_value; + bool const truncate_from_64_bit; + + BitfieldCheck(Node* source, uint32_t mask, uint32_t masked_value, + bool truncate_from_64_bit) + : source(source), + mask(mask), + masked_value(masked_value), + truncate_from_64_bit(truncate_from_64_bit) { + CHECK_EQ(masked_value & ~mask, 0); + } static base::Optional Detect(Node* node) { // There are two patterns to check for here: @@ -1716,14 +1726,16 @@ struct BitfieldCheck { if (eq.left().IsWord32And()) { Uint32BinopMatcher mand(eq.left().node()); if (mand.right().HasResolvedValue() && eq.right().HasResolvedValue()) { - BitfieldCheck result{mand.left().node(), mand.right().ResolvedValue(), - eq.right().ResolvedValue(), false}; + uint32_t mask = mand.right().ResolvedValue(); + uint32_t masked_value = eq.right().ResolvedValue(); + if ((masked_value & ~mask) != 0) return {}; if (mand.left().IsTruncateInt64ToInt32()) { - result.truncate_from_64_bit = true; - result.source = - NodeProperties::GetValueInput(mand.left().node(), 0); + return BitfieldCheck( + NodeProperties::GetValueInput(mand.left().node(), 0), mask, + masked_value, true); + } else { + return BitfieldCheck(mand.left().node(), mask, masked_value, false); } - return result; } } } else { @@ -1815,17 +1827,20 @@ Reduction MachineOperatorReducer::ReduceWord64And(Node* node) { } Reduction MachineOperatorReducer::TryMatchWord32Ror(Node* node) { + // Recognize rotation, we are matching and transforming as follows: + // x << y | x >>> (32 - y) => x ror (32 - y) + // x << (32 - y) | x >>> y => x ror y + // x << y ^ x >>> (32 - y) => x ror (32 - y) if y & 31 != 0 + // x << (32 - y) ^ x >>> y => x ror y if y & 31 != 0 + // (As well as the commuted forms.) + // Note the side condition for XOR: the optimization doesn't hold for + // multiples of 32. + DCHECK(IrOpcode::kWord32Or == node->opcode() || IrOpcode::kWord32Xor == node->opcode()); Int32BinopMatcher m(node); Node* shl = nullptr; Node* shr = nullptr; - // Recognize rotation, we are matching: - // * x << y | x >>> (32 - y) => x ror (32 - y), i.e x rol y - // * x << (32 - y) | x >>> y => x ror y - // * x << y ^ x >>> (32 - y) => x ror (32 - y), i.e. x rol y - // * x << (32 - y) ^ x >>> y => x ror y - // as well as their commuted form. if (m.left().IsWord32Shl() && m.right().IsWord32Shr()) { shl = m.left().node(); shr = m.right().node(); @@ -1842,8 +1857,13 @@ Reduction MachineOperatorReducer::TryMatchWord32Ror(Node* node) { if (mshl.right().HasResolvedValue() && mshr.right().HasResolvedValue()) { // Case where y is a constant. - if (mshl.right().ResolvedValue() + mshr.right().ResolvedValue() != 32) + if (mshl.right().ResolvedValue() + mshr.right().ResolvedValue() != 32) { return NoChange(); + } + if (node->opcode() == IrOpcode::kWord32Xor && + (mshl.right().ResolvedValue() & 31) == 0) { + return NoChange(); + } } else { Node* sub = nullptr; Node* y = nullptr; @@ -1859,6 +1879,9 @@ Reduction MachineOperatorReducer::TryMatchWord32Ror(Node* node) { Int32BinopMatcher msub(sub); if (!msub.left().Is(32) || msub.right().node() != y) return NoChange(); + if (node->opcode() == IrOpcode::kWord32Xor) { + return NoChange(); // Can't guarantee y & 31 != 0. + } } node->ReplaceInput(0, mshl.left().node()); diff --git a/v8/src/compiler/simplified-operator-reducer.cc b/v8/src/compiler/simplified-operator-reducer.cc index b1d3f8b2f3..f31a6c9a03 100644 --- a/v8/src/compiler/simplified-operator-reducer.cc +++ b/v8/src/compiler/simplified-operator-reducer.cc @@ -75,7 +75,7 @@ Reduction SimplifiedOperatorReducer::Reduce(Node* node) { case IrOpcode::kChangeInt32ToTagged: { Int32Matcher m(node->InputAt(0)); if (m.HasResolvedValue()) return ReplaceNumber(m.ResolvedValue()); - if (m.IsChangeTaggedToInt32() || m.IsChangeTaggedSignedToInt32()) { + if (m.IsChangeTaggedSignedToInt32()) { return Replace(m.InputAt(0)); } break; diff --git a/v8/src/compiler/wasm-compiler.cc b/v8/src/compiler/wasm-compiler.cc index 8956e5e887..42b6911307 100644 --- a/v8/src/compiler/wasm-compiler.cc +++ b/v8/src/compiler/wasm-compiler.cc @@ -6568,9 +6568,12 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder { Node* jump_table_slot = gasm_->IntAdd(jump_table_start, jump_table_offset); args[0] = jump_table_slot; + Node* instance_node = gasm_->LoadFromObject( + MachineType::TaggedPointer(), function_data, + wasm::ObjectAccess::ToTagged(WasmExportedFunctionData::kInstanceOffset)); BuildWasmCall(sig_, VectorOf(args), VectorOf(rets), - wasm::kNoCodePosition, nullptr, kNoRetpoline, + wasm::kNoCodePosition, instance_node, kNoRetpoline, frame_state); } } diff --git a/v8/src/execution/arguments-inl.h b/v8/src/execution/arguments-inl.h index 0be2325837..2f69cd7adc 100644 --- a/v8/src/execution/arguments-inl.h +++ b/v8/src/execution/arguments-inl.h @@ -14,6 +14,15 @@ namespace v8 { namespace internal { +template +Arguments::ChangeValueScope::ChangeValueScope(Isolate* isolate, + Arguments* args, int index, + Object value) + : location_(args->address_of_arg_at(index)) { + old_value_ = handle(Object(*location_), isolate); + *location_ = value.ptr(); +} + template int Arguments::smi_at(int index) const { return Smi::ToInt(Object(*address_of_arg_at(index))); diff --git a/v8/src/execution/arguments.h b/v8/src/execution/arguments.h index 39877cf4d2..de70619d69 100644 --- a/v8/src/execution/arguments.h +++ b/v8/src/execution/arguments.h @@ -33,6 +33,18 @@ namespace internal { template class Arguments { public: + // Scope to temporarily change the value of an argument. + class ChangeValueScope { + public: + inline ChangeValueScope(Isolate* isolate, Arguments* args, int index, + Object value); + ~ChangeValueScope() { *location_ = old_value_->ptr(); } + + private: + Address* location_; + Handle old_value_; + }; + Arguments(int length, Address* arguments) : length_(length), arguments_(arguments) { DCHECK_GE(length_, 0); @@ -51,10 +63,6 @@ class Arguments { inline double number_at(int index) const; - inline void set_at(int index, Object value) { - *address_of_arg_at(index) = value.ptr(); - } - inline FullObjectSlot slot_at(int index) const { return FullObjectSlot(address_of_arg_at(index)); } diff --git a/v8/src/execution/isolate-inl.h b/v8/src/execution/isolate-inl.h index 96ea770e65..3486925aed 100644 --- a/v8/src/execution/isolate-inl.h +++ b/v8/src/execution/isolate-inl.h @@ -35,7 +35,7 @@ NativeContext Isolate::raw_native_context() { } Object Isolate::pending_exception() { - DCHECK(has_pending_exception()); + CHECK(has_pending_exception()); DCHECK(!thread_local_top()->pending_exception_.IsException(this)); return thread_local_top()->pending_exception_; } diff --git a/v8/src/heap/concurrent-marking.cc b/v8/src/heap/concurrent-marking.cc index eb1511f71d..085af90436 100644 --- a/v8/src/heap/concurrent-marking.cc +++ b/v8/src/heap/concurrent-marking.cc @@ -433,7 +433,7 @@ void ConcurrentMarking::Run(JobDelegate* delegate, isolate->PrintWithTimestamp("Starting concurrent marking task %d\n", task_id); } - bool ephemeron_marked = false; + bool another_ephemeron_iteration = false; { TimedScope scope(&time_ms); @@ -443,7 +443,7 @@ void ConcurrentMarking::Run(JobDelegate* delegate, while (weak_objects_->current_ephemerons.Pop(task_id, &ephemeron)) { if (visitor.ProcessEphemeron(ephemeron.key, ephemeron.value)) { - ephemeron_marked = true; + another_ephemeron_iteration = true; } } } @@ -484,6 +484,7 @@ void ConcurrentMarking::Run(JobDelegate* delegate, current_marked_bytes += visited_size; } } + if (objects_processed > 0) another_ephemeron_iteration = true; marked_bytes += current_marked_bytes; base::AsAtomicWord::Relaxed_Store(&task_state->marked_bytes, marked_bytes); @@ -499,7 +500,7 @@ void ConcurrentMarking::Run(JobDelegate* delegate, while (weak_objects_->discovered_ephemerons.Pop(task_id, &ephemeron)) { if (visitor.ProcessEphemeron(ephemeron.key, ephemeron.value)) { - ephemeron_marked = true; + another_ephemeron_iteration = true; } } } @@ -519,8 +520,8 @@ void ConcurrentMarking::Run(JobDelegate* delegate, base::AsAtomicWord::Relaxed_Store(&task_state->marked_bytes, 0); total_marked_bytes_ += marked_bytes; - if (ephemeron_marked) { - set_ephemeron_marked(true); + if (another_ephemeron_iteration) { + set_another_ephemeron_iteration(true); } } if (FLAG_trace_concurrent_marking) { diff --git a/v8/src/heap/concurrent-marking.h b/v8/src/heap/concurrent-marking.h index c685f5cca6..54f6057f58 100644 --- a/v8/src/heap/concurrent-marking.h +++ b/v8/src/heap/concurrent-marking.h @@ -91,10 +91,12 @@ class V8_EXPORT_PRIVATE ConcurrentMarking { size_t TotalMarkedBytes(); - void set_ephemeron_marked(bool ephemeron_marked) { - ephemeron_marked_.store(ephemeron_marked); + void set_another_ephemeron_iteration(bool another_ephemeron_iteration) { + another_ephemeron_iteration_.store(another_ephemeron_iteration); + } + bool another_ephemeron_iteration() { + return another_ephemeron_iteration_.load(); } - bool ephemeron_marked() { return ephemeron_marked_.load(); } private: struct TaskState { @@ -115,7 +117,7 @@ class V8_EXPORT_PRIVATE ConcurrentMarking { WeakObjects* const weak_objects_; TaskState task_state_[kMaxTasks + 1]; std::atomic total_marked_bytes_{0}; - std::atomic ephemeron_marked_{false}; + std::atomic another_ephemeron_iteration_{false}; }; } // namespace internal diff --git a/v8/src/heap/cppgc/marker.cc b/v8/src/heap/cppgc/marker.cc index d30bb0a8ec..304244502a 100644 --- a/v8/src/heap/cppgc/marker.cc +++ b/v8/src/heap/cppgc/marker.cc @@ -241,6 +241,7 @@ void MarkerBase::EnterAtomicPause(MarkingConfig::StackState stack_state) { } config_.stack_state = stack_state; config_.marking_type = MarkingConfig::MarkingType::kAtomic; + mutator_marking_state_.set_in_atomic_pause(); // Lock guards against changes to {Weak}CrossThreadPersistent handles, that // may conflict with marking. E.g., a WeakCrossThreadPersistent may be @@ -407,7 +408,9 @@ bool MarkerBase::ProcessWorklistsWithDeadline( size_t marked_bytes_deadline, v8::base::TimeTicks time_deadline) { StatsCollector::EnabledScope stats_scope( heap().stats_collector(), StatsCollector::kMarkTransitiveClosure); + bool saved_did_discover_new_ephemeron_pairs; do { + mutator_marking_state_.ResetDidDiscoverNewEphemeronPairs(); if ((config_.marking_type == MarkingConfig::MarkingType::kAtomic) || schedule_.ShouldFlushEphemeronPairs()) { mutator_marking_state_.FlushDiscoveredEphemeronPairs(); @@ -484,6 +487,8 @@ bool MarkerBase::ProcessWorklistsWithDeadline( } } + saved_did_discover_new_ephemeron_pairs = + mutator_marking_state_.DidDiscoverNewEphemeronPairs(); { StatsCollector::EnabledScope stats_scope( heap().stats_collector(), StatsCollector::kMarkProcessEphemerons); @@ -497,7 +502,8 @@ bool MarkerBase::ProcessWorklistsWithDeadline( return false; } } - } while (!mutator_marking_state_.marking_worklist().IsLocalAndGlobalEmpty()); + } while (!mutator_marking_state_.marking_worklist().IsLocalAndGlobalEmpty() || + saved_did_discover_new_ephemeron_pairs); return true; } diff --git a/v8/src/heap/cppgc/marking-state.h b/v8/src/heap/cppgc/marking-state.h index 6e08fc3e10..ca43372a96 100644 --- a/v8/src/heap/cppgc/marking-state.h +++ b/v8/src/heap/cppgc/marking-state.h @@ -9,6 +9,7 @@ #include "include/cppgc/trace-trait.h" #include "include/cppgc/visitor.h" +#include "src/base/logging.h" #include "src/heap/cppgc/compaction-worklists.h" #include "src/heap/cppgc/globals.h" #include "src/heap/cppgc/heap-object-header.h" @@ -111,6 +112,16 @@ class MarkingStateBase { movable_slots_worklist_.reset(); } + bool DidDiscoverNewEphemeronPairs() const { + return discovered_new_ephemeron_pairs_; + } + + void ResetDidDiscoverNewEphemeronPairs() { + discovered_new_ephemeron_pairs_ = false; + } + + void set_in_atomic_pause() { in_atomic_pause_ = true; } + protected: inline void MarkAndPush(HeapObjectHeader&, TraceDescriptor); @@ -144,6 +155,9 @@ class MarkingStateBase { movable_slots_worklist_; size_t marked_bytes_ = 0; + bool in_ephemeron_processing_ = false; + bool discovered_new_ephemeron_pairs_ = false; + bool in_atomic_pause_ = false; }; MarkingStateBase::MarkingStateBase(HeapBase& heap, @@ -270,10 +284,23 @@ void MarkingStateBase::ProcessWeakContainer(const void* object, void MarkingStateBase::ProcessEphemeron(const void* key, const void* value, TraceDescriptor value_desc, Visitor& visitor) { - // Filter out already marked keys. The write barrier for WeakMember - // ensures that any newly set value after this point is kept alive and does - // not require the callback. - if (HeapObjectHeader::FromPayload(key).IsMarked()) { + // ProcessEphemeron is not expected to find new ephemerons recursively, which + // would break the main marking loop. + DCHECK(!in_ephemeron_processing_); + in_ephemeron_processing_ = true; + // Keys are considered live even in incremental/concurrent marking settings + // because the write barrier for WeakMember ensures that any newly set value + // after this point is kept alive and does not require the callback. + const bool key_in_construction = HeapObjectHeader::FromPayload(key) + .IsInConstruction(); + const bool key_considered_as_live = + key_in_construction + ? in_atomic_pause_ + : HeapObjectHeader::FromPayload(key).IsMarked(); + DCHECK_IMPLIES( + key_in_construction && in_atomic_pause_, + HeapObjectHeader::FromPayload(key).IsMarked()); + if (key_considered_as_live) { if (value_desc.base_object_payload) { MarkAndPush(value_desc.base_object_payload, value_desc); } else { @@ -281,9 +308,11 @@ void MarkingStateBase::ProcessEphemeron(const void* key, const void* value, // should be immediately traced. value_desc.callback(&visitor, value); } - return; + } else { + discovered_ephemeron_pairs_worklist_.Push({key, value, value_desc}); + discovered_new_ephemeron_pairs_ = true; } - discovered_ephemeron_pairs_worklist_.Push({key, value, value_desc}); + in_ephemeron_processing_ = false; } void MarkingStateBase::AccountMarkedBytes(const HeapObjectHeader& header) { diff --git a/v8/src/heap/incremental-marking.cc b/v8/src/heap/incremental-marking.cc index a093835981..efedcdb32b 100644 --- a/v8/src/heap/incremental-marking.cc +++ b/v8/src/heap/incremental-marking.cc @@ -921,7 +921,8 @@ StepResult IncrementalMarking::Step(double max_step_size_in_ms, // This ignores that case where the embedder finds new V8-side objects. The // assumption is that large graphs are well connected and can mostly be // processed on their own. For small graphs, helping is not necessary. - v8_bytes_processed = collector_->ProcessMarkingWorklist(bytes_to_process); + std::tie(v8_bytes_processed, std::ignore) = + collector_->ProcessMarkingWorklist(bytes_to_process); StepResult v8_result = local_marking_worklists()->IsEmpty() ? StepResult::kNoImmediateWork : StepResult::kMoreWorkRemaining; diff --git a/v8/src/heap/mark-compact.cc b/v8/src/heap/mark-compact.cc index 756bf6e5e1..9e4f36d35c 100644 --- a/v8/src/heap/mark-compact.cc +++ b/v8/src/heap/mark-compact.cc @@ -1603,24 +1603,24 @@ void MarkCompactCollector::MarkDescriptorArrayFromWriteBarrier( descriptors, number_of_own_descriptors); } -void MarkCompactCollector::ProcessEphemeronsUntilFixpoint() { - bool work_to_do = true; +bool MarkCompactCollector::ProcessEphemeronsUntilFixpoint() { int iterations = 0; int max_iterations = FLAG_ephemeron_fixpoint_iterations; - while (work_to_do) { + bool another_ephemeron_iteration_main_thread; + + do { PerformWrapperTracing(); if (iterations >= max_iterations) { // Give up fixpoint iteration and switch to linear algorithm. - ProcessEphemeronsLinear(); - break; + return false; } // Move ephemerons from next_ephemerons into current_ephemerons to // drain them in this iteration. weak_objects_.current_ephemerons.Swap(weak_objects_.next_ephemerons); - heap()->concurrent_marking()->set_ephemeron_marked(false); + heap()->concurrent_marking()->set_another_ephemeron_iteration(false); { TRACE_GC(heap()->tracer(), @@ -1631,47 +1631,54 @@ void MarkCompactCollector::ProcessEphemeronsUntilFixpoint() { TaskPriority::kUserBlocking); } - work_to_do = ProcessEphemerons(); + another_ephemeron_iteration_main_thread = ProcessEphemerons(); FinishConcurrentMarking(); } CHECK(weak_objects_.current_ephemerons.IsEmpty()); CHECK(weak_objects_.discovered_ephemerons.IsEmpty()); - work_to_do = work_to_do || !local_marking_worklists()->IsEmpty() || - heap()->concurrent_marking()->ephemeron_marked() || - !local_marking_worklists()->IsEmbedderEmpty() || - !heap()->local_embedder_heap_tracer()->IsRemoteTracingDone(); ++iterations; - } + } while (another_ephemeron_iteration_main_thread || + heap()->concurrent_marking()->another_ephemeron_iteration() || + !local_marking_worklists()->IsEmpty() || + !local_marking_worklists()->IsEmbedderEmpty() || + !heap()->local_embedder_heap_tracer()->IsRemoteTracingDone()); CHECK(local_marking_worklists()->IsEmpty()); CHECK(weak_objects_.current_ephemerons.IsEmpty()); CHECK(weak_objects_.discovered_ephemerons.IsEmpty()); + return true; } bool MarkCompactCollector::ProcessEphemerons() { Ephemeron ephemeron; - bool ephemeron_marked = false; + bool another_ephemeron_iteration = false; // Drain current_ephemerons and push ephemerons where key and value are still // unreachable into next_ephemerons. while (weak_objects_.current_ephemerons.Pop(kMainThreadTask, &ephemeron)) { if (ProcessEphemeron(ephemeron.key, ephemeron.value)) { - ephemeron_marked = true; + another_ephemeron_iteration = true; } } // Drain marking worklist and push discovered ephemerons into // discovered_ephemerons. - DrainMarkingWorklist(); + size_t objects_processed; + std::tie(std::ignore, objects_processed) = ProcessMarkingWorklist(0); + + // As soon as a single object was processed and potentially marked another + // object we need another iteration. Otherwise we might miss to apply + // ephemeron semantics on it. + if (objects_processed > 0) another_ephemeron_iteration = true; // Drain discovered_ephemerons (filled in the drain MarkingWorklist-phase // before) and push ephemerons where key and value are still unreachable into // next_ephemerons. while (weak_objects_.discovered_ephemerons.Pop(kMainThreadTask, &ephemeron)) { if (ProcessEphemeron(ephemeron.key, ephemeron.value)) { - ephemeron_marked = true; + another_ephemeron_iteration = true; } } @@ -1679,7 +1686,7 @@ bool MarkCompactCollector::ProcessEphemerons() { weak_objects_.ephemeron_hash_tables.FlushToGlobal(kMainThreadTask); weak_objects_.next_ephemerons.FlushToGlobal(kMainThreadTask); - return ephemeron_marked; + return another_ephemeron_iteration; } void MarkCompactCollector::ProcessEphemeronsLinear() { @@ -1765,6 +1772,12 @@ void MarkCompactCollector::ProcessEphemeronsLinear() { ephemeron_marking_.newly_discovered.shrink_to_fit(); CHECK(local_marking_worklists()->IsEmpty()); + CHECK(weak_objects_.current_ephemerons.IsEmpty()); + CHECK(weak_objects_.discovered_ephemerons.IsEmpty()); + + // Flush local ephemerons for main task to global pool. + weak_objects_.ephemeron_hash_tables.FlushToGlobal(kMainThreadTask); + weak_objects_.next_ephemerons.FlushToGlobal(kMainThreadTask); } void MarkCompactCollector::PerformWrapperTracing() { @@ -1786,9 +1799,11 @@ void MarkCompactCollector::PerformWrapperTracing() { void MarkCompactCollector::DrainMarkingWorklist() { ProcessMarkingWorklist(0); } template -size_t MarkCompactCollector::ProcessMarkingWorklist(size_t bytes_to_process) { +std::pair MarkCompactCollector::ProcessMarkingWorklist( + size_t bytes_to_process) { HeapObject object; size_t bytes_processed = 0; + size_t objects_processed = 0; bool is_per_context_mode = local_marking_worklists()->IsPerContextMode(); Isolate* isolate = heap()->isolate(); while (local_marking_worklists()->Pop(&object) || @@ -1828,18 +1843,19 @@ size_t MarkCompactCollector::ProcessMarkingWorklist(size_t bytes_to_process) { map, object, visited_size); } bytes_processed += visited_size; + objects_processed++; if (bytes_to_process && bytes_processed >= bytes_to_process) { break; } } - return bytes_processed; + return std::make_pair(bytes_processed, objects_processed); } // Generate definitions for use in other files. -template size_t MarkCompactCollector::ProcessMarkingWorklist< +template std::pair MarkCompactCollector::ProcessMarkingWorklist< MarkCompactCollector::MarkingWorklistProcessingMode::kDefault>( size_t bytes_to_process); -template size_t MarkCompactCollector::ProcessMarkingWorklist< +template std::pair MarkCompactCollector::ProcessMarkingWorklist< MarkCompactCollector::MarkingWorklistProcessingMode:: kTrackNewlyDiscoveredObjects>(size_t bytes_to_process); @@ -1864,7 +1880,23 @@ void MarkCompactCollector::ProcessEphemeronMarking() { // buffer, flush it into global pool. weak_objects_.next_ephemerons.FlushToGlobal(kMainThreadTask); - ProcessEphemeronsUntilFixpoint(); + if (!ProcessEphemeronsUntilFixpoint()) { + // Fixpoint iteration needed too many iterations and was cancelled. Use the + // guaranteed linear algorithm. + ProcessEphemeronsLinear(); + } + +#ifdef VERIFY_HEAP + if (FLAG_verify_heap) { + Ephemeron ephemeron; + + weak_objects_.current_ephemerons.Swap(weak_objects_.next_ephemerons); + + while (weak_objects_.current_ephemerons.Pop(kMainThreadTask, &ephemeron)) { + CHECK(!ProcessEphemeron(ephemeron.key, ephemeron.value)); + } + } +#endif CHECK(local_marking_worklists()->IsEmpty()); CHECK(heap()->local_embedder_heap_tracer()->IsRemoteTracingDone()); @@ -2525,28 +2557,19 @@ void MarkCompactCollector::ClearJSWeakRefs() { RecordSlot(weak_cell, slot, HeapObject::cast(*slot)); } - HeapObject unregister_token = - HeapObject::cast(weak_cell.unregister_token()); + HeapObject unregister_token = weak_cell.unregister_token(); if (!non_atomic_marking_state()->IsBlackOrGrey(unregister_token)) { // The unregister token is dead. Remove any corresponding entries in the // key map. Multiple WeakCell with the same token will have all their // unregister_token field set to undefined when processing the first // WeakCell. Like above, we're modifying pointers during GC, so record the // slots. - HeapObject undefined = ReadOnlyRoots(isolate()).undefined_value(); JSFinalizationRegistry finalization_registry = JSFinalizationRegistry::cast(weak_cell.finalization_registry()); finalization_registry.RemoveUnregisterToken( JSReceiver::cast(unregister_token), isolate(), - [undefined](WeakCell matched_cell) { - matched_cell.set_unregister_token(undefined); - }, + JSFinalizationRegistry::kKeepMatchedCellsInRegistry, gc_notify_updated_slot); - // The following is necessary because in the case that weak_cell has - // already been popped and removed from the FinalizationRegistry, the call - // to JSFinalizationRegistry::RemoveUnregisterToken above will not find - // weak_cell itself to clear its unregister token. - weak_cell.set_unregister_token(undefined); } else { // The unregister_token is alive. ObjectSlot slot = weak_cell.RawField(WeakCell::kUnregisterTokenOffset); diff --git a/v8/src/heap/mark-compact.h b/v8/src/heap/mark-compact.h index 733588ae80..0674ce674f 100644 --- a/v8/src/heap/mark-compact.h +++ b/v8/src/heap/mark-compact.h @@ -588,7 +588,7 @@ class MarkCompactCollector final : public MarkCompactCollectorBase { // is drained until it is empty. template - size_t ProcessMarkingWorklist(size_t bytes_to_process); + std::pair ProcessMarkingWorklist(size_t bytes_to_process); private: void ComputeEvacuationHeuristics(size_t area_size, @@ -634,8 +634,9 @@ class MarkCompactCollector final : public MarkCompactCollectorBase { bool ProcessEphemeron(HeapObject key, HeapObject value); // Marks ephemerons and drains marking worklist iteratively - // until a fixpoint is reached. - void ProcessEphemeronsUntilFixpoint(); + // until a fixpoint is reached. Returns false if too many iterations have been + // tried and the linear approach should be used. + bool ProcessEphemeronsUntilFixpoint(); // Drains ephemeron and marking worklists. Single iteration of the // fixpoint iteration. diff --git a/v8/src/heap/marking-visitor-inl.h b/v8/src/heap/marking-visitor-inl.h index 55c37e535b..557d6248fc 100644 --- a/v8/src/heap/marking-visitor-inl.h +++ b/v8/src/heap/marking-visitor-inl.h @@ -326,7 +326,7 @@ int MarkingVisitorBase::VisitWeakCell( this->VisitMapPointer(weak_cell); WeakCell::BodyDescriptor::IterateBody(map, weak_cell, size, this); HeapObject target = weak_cell.relaxed_target(); - HeapObject unregister_token = HeapObject::cast(weak_cell.unregister_token()); + HeapObject unregister_token = weak_cell.relaxed_unregister_token(); concrete_visitor()->SynchronizePageAccess(target); concrete_visitor()->SynchronizePageAccess(unregister_token); if (concrete_visitor()->marking_state()->IsBlackOrGrey(target) && diff --git a/v8/src/ic/accessor-assembler.cc b/v8/src/ic/accessor-assembler.cc index 59a950b48f..2a9dc032a8 100644 --- a/v8/src/ic/accessor-assembler.cc +++ b/v8/src/ic/accessor-assembler.cc @@ -654,8 +654,8 @@ void AccessorAssembler::HandleLoadICSmiHandlerLoadNamedCase( Comment("module export"); TNode index = DecodeWord(handler_word); - TNode module = LoadObjectField( - CAST(p->receiver()), JSModuleNamespace::kModuleOffset); + TNode module = + LoadObjectField(CAST(holder), JSModuleNamespace::kModuleOffset); TNode exports = LoadObjectField(module, Module::kExportsOffset); TNode cell = CAST(LoadFixedArrayElement(exports, index)); diff --git a/v8/src/ic/ic.cc b/v8/src/ic/ic.cc index e607bb53a8..1fadcc1ead 100644 --- a/v8/src/ic/ic.cc +++ b/v8/src/ic/ic.cc @@ -857,8 +857,14 @@ Handle LoadIC::ComputeHandler(LookupIterator* lookup) { Smi::ToInt(lookup->name()->GetHash())); // We found the accessor, so the entry must exist. DCHECK(entry.is_found()); - int index = ObjectHashTable::EntryToValueIndex(entry); - return LoadHandler::LoadModuleExport(isolate(), index); + int value_index = ObjectHashTable::EntryToValueIndex(entry); + Handle smi_handler = + LoadHandler::LoadModuleExport(isolate(), value_index); + if (holder_is_lookup_start_object) { + return smi_handler; + } + return LoadHandler::LoadFromPrototype(isolate(), map, holder, + smi_handler); } Handle accessors = lookup->GetAccessors(); diff --git a/v8/src/libsampler/sampler.cc b/v8/src/libsampler/sampler.cc index e933c61864..f187aeaca6 100644 --- a/v8/src/libsampler/sampler.cc +++ b/v8/src/libsampler/sampler.cc @@ -60,7 +60,7 @@ using zx_thread_state_general_regs_t = zx_arm64_general_regs_t; #include "src/base/atomic-utils.h" #include "src/base/platform/platform.h" -#if V8_OS_ANDROID && !defined(__BIONIC_HAVE_UCONTEXT_T) +#if V8_OS_ANDROID && !defined(__BIONIC_HAVE_UCONTEXT_T) && !defined(OSOHOS) // Not all versions of Android's C library provide ucontext_t. // Detect this and provide custom but compatible definitions. Note that these diff --git a/v8/src/objects/js-weak-refs-inl.h b/v8/src/objects/js-weak-refs-inl.h index aa51ee18c8..0e39b00d13 100644 --- a/v8/src/objects/js-weak-refs-inl.h +++ b/v8/src/objects/js-weak-refs-inl.h @@ -71,16 +71,14 @@ bool JSFinalizationRegistry::Unregister( // key. Each WeakCell will be in the "active_cells" or "cleared_cells" list of // its FinalizationRegistry; remove it from there. return finalization_registry->RemoveUnregisterToken( - *unregister_token, isolate, - [isolate](WeakCell matched_cell) { - matched_cell.RemoveFromFinalizationRegistryCells(isolate); - }, + *unregister_token, isolate, kRemoveMatchedCellsFromRegistry, [](HeapObject, ObjectSlot, Object) {}); } -template +template bool JSFinalizationRegistry::RemoveUnregisterToken( - JSReceiver unregister_token, Isolate* isolate, MatchCallback match_callback, + JSReceiver unregister_token, Isolate* isolate, + RemoveUnregisterTokenMode removal_mode, GCNotifyUpdatedSlotCallback gc_notify_updated_slot) { // This method is called from both FinalizationRegistry#unregister and for // removing weakly-held dead unregister tokens. The latter is during GC so @@ -118,7 +116,16 @@ bool JSFinalizationRegistry::RemoveUnregisterToken( value = weak_cell.key_list_next(); if (weak_cell.unregister_token() == unregister_token) { // weak_cell has the same unregister token; remove it from the key list. - match_callback(weak_cell); + switch (removal_mode) { + case kRemoveMatchedCellsFromRegistry: + weak_cell.RemoveFromFinalizationRegistryCells(isolate); + break; + case kKeepMatchedCellsInRegistry: + // Do nothing. + break; + } + // Clear unregister token-related fields. + weak_cell.set_unregister_token(undefined); weak_cell.set_key_list_prev(undefined); weak_cell.set_key_list_next(undefined); was_present = true; @@ -163,6 +170,10 @@ HeapObject WeakCell::relaxed_target() const { return TaggedField::Relaxed_Load(*this, kTargetOffset); } +HeapObject WeakCell::relaxed_unregister_token() const { + return TaggedField::Relaxed_Load(*this, kUnregisterTokenOffset); +} + template void WeakCell::Nullify(Isolate* isolate, GCNotifyUpdatedSlotCallback gc_notify_updated_slot) { diff --git a/v8/src/objects/js-weak-refs.h b/v8/src/objects/js-weak-refs.h index 300673381a..88361ad1c0 100644 --- a/v8/src/objects/js-weak-refs.h +++ b/v8/src/objects/js-weak-refs.h @@ -53,10 +53,14 @@ class JSFinalizationRegistry : public JSObject { // it modifies slots in key_map and WeakCells and the normal write barrier is // disabled during GC, we need to tell the GC about the modified slots via the // gc_notify_updated_slot function. - template + enum RemoveUnregisterTokenMode { + kRemoveMatchedCellsFromRegistry, + kKeepMatchedCellsInRegistry + }; + template inline bool RemoveUnregisterToken( JSReceiver unregister_token, Isolate* isolate, - MatchCallback match_callback, + RemoveUnregisterTokenMode removal_mode, GCNotifyUpdatedSlotCallback gc_notify_updated_slot); // Returns true if the cleared_cells list is non-empty. @@ -93,6 +97,9 @@ class WeakCell : public TorqueGeneratedWeakCell { // Provide relaxed load access to target field. inline HeapObject relaxed_target() const; + // Provide relaxed load access to the unregister token field. + inline HeapObject relaxed_unregister_token() const; + // Nullify is called during GC and it modifies the pointers in WeakCell and // JSFinalizationRegistry. Thus we need to tell the GC about the modified // slots via the gc_notify_updated_slot function. The normal write barrier is diff --git a/v8/src/objects/js-weak-refs.tq b/v8/src/objects/js-weak-refs.tq index 9008f64290..3447e31b71 100644 --- a/v8/src/objects/js-weak-refs.tq +++ b/v8/src/objects/js-weak-refs.tq @@ -22,7 +22,7 @@ extern class JSFinalizationRegistry extends JSObject { extern class WeakCell extends HeapObject { finalization_registry: Undefined|JSFinalizationRegistry; target: Undefined|JSReceiver; - unregister_token: JSAny; + unregister_token: Undefined|JSReceiver; holdings: JSAny; // For storing doubly linked lists of WeakCells in JSFinalizationRegistry's diff --git a/v8/src/objects/objects.cc b/v8/src/objects/objects.cc index 96c84dab5f..0c1cc482de 100644 --- a/v8/src/objects/objects.cc +++ b/v8/src/objects/objects.cc @@ -196,6 +196,8 @@ std::ostream& operator<<(std::ostream& os, PropertyCellType type) { return os << "ConstantType"; case PropertyCellType::kMutable: return os << "Mutable"; + case PropertyCellType::kInTransition: + return os << "InTransition"; } UNREACHABLE(); } @@ -2527,6 +2529,12 @@ Maybe Object::SetPropertyInternal(LookupIterator* it, Maybe result = JSObject::SetPropertyWithInterceptor(it, should_throw, value); if (result.IsNothing() || result.FromJust()) return result; + // Assuming that the callback have side effects, we use + // Object::SetSuperProperty() which works properly regardless on + // whether the property was present on the receiver or not when + // storing to the receiver. + // Proceed lookup from the next state. + it->Next(); } else { Maybe maybe_attributes = JSObject::GetPropertyAttributesWithInterceptor(it); @@ -2534,11 +2542,21 @@ Maybe Object::SetPropertyInternal(LookupIterator* it, if ((maybe_attributes.FromJust() & READ_ONLY) != 0) { return WriteToReadOnlyProperty(it, value, should_throw); } - if (maybe_attributes.FromJust() == ABSENT) break; - *found = false; - return Nothing(); + // At this point we might have called interceptor's query or getter + // callback. Assuming that the callbacks have side effects, we use + // Object::SetSuperProperty() which works properly regardless on + // whether the property was present on the receiver or not when + // storing to the receiver. + if (maybe_attributes.FromJust() == ABSENT) { + // Proceed lookup from the next state. + it->Next(); + } else { + // Finish lookup in order to make Object::SetSuperProperty() store + // property to the receiver. + it->NotFound(); + } } - break; + return Object::SetSuperProperty(it, value, store_origin, should_throw); } case LookupIterator::ACCESSOR: { @@ -2601,6 +2619,26 @@ Maybe Object::SetPropertyInternal(LookupIterator* it, return Nothing(); } +bool Object::CheckContextualStoreToJSGlobalObject( + LookupIterator* it, Maybe should_throw) { + Isolate* isolate = it->isolate(); + + if (it->GetReceiver()->IsJSGlobalObject(isolate) && + (GetShouldThrow(isolate, should_throw) == ShouldThrow::kThrowOnError)) { + if (it->state() == LookupIterator::TRANSITION) { + // The property cell that we have created is garbage because we are going + // to throw now instead of putting it into the global dictionary. However, + // the cell might already have been stored into the feedback vector, so + // we must invalidate it nevertheless. + it->transition_cell()->ClearAndInvalidate(ReadOnlyRoots(isolate)); + } + isolate->Throw(*isolate->factory()->NewReferenceError( + MessageTemplate::kNotDefined, it->GetName())); + return false; + } + return true; +} + Maybe Object::SetProperty(LookupIterator* it, Handle value, StoreOrigin store_origin, Maybe should_throw) { @@ -2611,24 +2649,9 @@ Maybe Object::SetProperty(LookupIterator* it, Handle value, if (found) return result; } - // If the receiver is the JSGlobalObject, the store was contextual. In case - // the property did not exist yet on the global object itself, we have to - // throw a reference error in strict mode. In sloppy mode, we continue. - if (it->GetReceiver()->IsJSGlobalObject() && - (GetShouldThrow(it->isolate(), should_throw) == - ShouldThrow::kThrowOnError)) { - if (it->state() == LookupIterator::TRANSITION) { - // The property cell that we have created is garbage because we are going - // to throw now instead of putting it into the global dictionary. However, - // the cell might already have been stored into the feedback vector, so - // we must invalidate it nevertheless. - it->transition_cell()->ClearAndInvalidate(ReadOnlyRoots(it->isolate())); - } - it->isolate()->Throw(*it->isolate()->factory()->NewReferenceError( - MessageTemplate::kNotDefined, it->GetName())); + if (!CheckContextualStoreToJSGlobalObject(it, should_throw)) { return Nothing(); } - return AddDataProperty(it, value, NONE, should_throw, store_origin); } @@ -2695,6 +2718,9 @@ Maybe Object::SetSuperProperty(LookupIterator* it, Handle value, JSReceiver::GetOwnPropertyDescriptor(&own_lookup, &desc); MAYBE_RETURN(owned, Nothing()); if (!owned.FromJust()) { + if (!CheckContextualStoreToJSGlobalObject(&own_lookup, should_throw)) { + return Nothing(); + } return JSReceiver::CreateDataProperty(&own_lookup, value, should_throw); } @@ -6395,6 +6421,8 @@ PropertyCellType PropertyCell::UpdatedType(Isolate* isolate, V8_FALLTHROUGH; case PropertyCellType::kMutable: return PropertyCellType::kMutable; + case PropertyCellType::kInTransition: + UNREACHABLE(); } } @@ -6450,6 +6478,7 @@ bool PropertyCell::CheckDataIsCompatible(PropertyDetails details, Object value) { DisallowGarbageCollection no_gc; PropertyCellType cell_type = details.cell_type(); + CHECK_NE(cell_type, PropertyCellType::kInTransition); if (value.IsTheHole()) { CHECK_EQ(cell_type, PropertyCellType::kConstant); } else { @@ -6483,8 +6512,9 @@ bool PropertyCell::CanTransitionTo(PropertyDetails new_details, return new_details.cell_type() == PropertyCellType::kMutable || (new_details.cell_type() == PropertyCellType::kConstant && new_value.IsTheHole()); + case PropertyCellType::kInTransition: + UNREACHABLE(); } - UNREACHABLE(); } #endif // DEBUG @@ -6710,6 +6740,7 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap( JSFinalizationRegistry::cast(Object(raw_finalization_registry)); WeakCell weak_cell = WeakCell::cast(Object(raw_weak_cell)); DCHECK(!weak_cell.unregister_token().IsUndefined(isolate)); + HeapObject undefined = ReadOnlyRoots(isolate).undefined_value(); // Remove weak_cell from the linked list of other WeakCells with the same // unregister token and remove its unregister token from key_map if necessary @@ -6718,7 +6749,7 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap( if (weak_cell.key_list_prev().IsUndefined(isolate)) { SimpleNumberDictionary key_map = SimpleNumberDictionary::cast(finalization_registry.key_map()); - Object unregister_token = weak_cell.unregister_token(); + HeapObject unregister_token = weak_cell.unregister_token(); uint32_t key = Smi::ToInt(unregister_token.GetHash()); InternalIndex entry = key_map.FindEntry(isolate, key); DCHECK(entry.is_found()); @@ -6733,8 +6764,7 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap( // of the key in the hash table. WeakCell next = WeakCell::cast(weak_cell.key_list_next()); DCHECK_EQ(next.key_list_prev(), weak_cell); - next.set_key_list_prev(ReadOnlyRoots(isolate).undefined_value()); - weak_cell.set_key_list_next(ReadOnlyRoots(isolate).undefined_value()); + next.set_key_list_prev(undefined); key_map.ValueAtPut(entry, next); } } else { @@ -6746,6 +6776,12 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap( next.set_key_list_prev(weak_cell.key_list_prev()); } } + + // weak_cell is now removed from the unregister token map, so clear its + // unregister token-related fields. + weak_cell.set_unregister_token(undefined); + weak_cell.set_key_list_prev(undefined); + weak_cell.set_key_list_next(undefined); } } // namespace internal diff --git a/v8/src/objects/objects.h b/v8/src/objects/objects.h index c68445597f..93624b7b39 100644 --- a/v8/src/objects/objects.h +++ b/v8/src/objects/objects.h @@ -726,6 +726,8 @@ class Object : public TaggedImpl { V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT static MaybeHandle ConvertToIndex(Isolate* isolate, Handle input, MessageTemplate error_index); + V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT static bool CheckContextualStoreToJSGlobalObject( + LookupIterator* it, Maybe should_throw = Nothing()); }; V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os, const Object& obj); diff --git a/v8/src/objects/property-cell-inl.h b/v8/src/objects/property-cell-inl.h index 154dcab41f..266f1d1e03 100644 --- a/v8/src/objects/property-cell-inl.h +++ b/v8/src/objects/property-cell-inl.h @@ -57,6 +57,9 @@ void PropertyCell::Transition(PropertyDetails new_details, DCHECK(CanTransitionTo(new_details, *new_value)); // This code must be in sync with its counterpart in // PropertyCellData::Serialize. + PropertyDetails transition_marker = new_details; + transition_marker.set_cell_type(PropertyCellType::kInTransition); + set_property_details_raw(transition_marker.AsSmi(), kReleaseStore); set_value(*new_value, kReleaseStore); set_property_details_raw(new_details.AsSmi(), kReleaseStore); } diff --git a/v8/src/objects/property-details.h b/v8/src/objects/property-details.h index bab6e297e4..cdea8883d1 100644 --- a/v8/src/objects/property-details.h +++ b/v8/src/objects/property-details.h @@ -208,6 +208,9 @@ enum class PropertyCellType { kUndefined, // The PREMONOMORPHIC of property cells. kConstant, // Cell has been assigned only once. kConstantType, // Cell has been assigned only one type. + // Temporary value indicating an ongoing property cell state transition. Only + // observable by a background thread. + kInTransition, // Value for dictionaries not holding cells, must be 0: kNoCell = kMutable, }; @@ -347,8 +350,7 @@ class PropertyDetails { // Bit fields in value_ (type, shift, size). Must be public so the // constants can be embedded in generated code. using KindField = base::BitField; - using LocationField = KindField::Next; - using ConstnessField = LocationField::Next; + using ConstnessField = KindField::Next; using AttributesField = ConstnessField::Next; static const int kAttributesReadOnlyMask = (READ_ONLY << AttributesField::kShift); @@ -358,11 +360,12 @@ class PropertyDetails { (DONT_ENUM << AttributesField::kShift); // Bit fields for normalized/dictionary mode objects. - using PropertyCellTypeField = AttributesField::Next; + using PropertyCellTypeField = AttributesField::Next; using DictionaryStorageField = PropertyCellTypeField::Next; // Bit fields for fast objects. - using RepresentationField = AttributesField::Next; + using LocationField = AttributesField::Next; + using RepresentationField = LocationField::Next; using DescriptorPointer = RepresentationField::Next; using FieldIndexField = @@ -381,7 +384,6 @@ class PropertyDetails { STATIC_ASSERT(KindField::kLastUsedBit < 8); STATIC_ASSERT(ConstnessField::kLastUsedBit < 8); STATIC_ASSERT(AttributesField::kLastUsedBit < 8); - STATIC_ASSERT(LocationField::kLastUsedBit < 8); static const int kInitialIndex = 1; @@ -411,12 +413,12 @@ class PropertyDetails { // with an enumeration index of 0 as a single byte. uint8_t ToByte() { // We only care about the value of KindField, ConstnessField, and - // AttributesField. LocationField is also stored, but it will always be - // kField. We've statically asserted earlier that all those fields fit into - // a byte together. + // AttributesField. We've statically asserted earlier that these fields fit + // into a byte together. + + DCHECK_EQ(PropertyLocation::kField, location()); + STATIC_ASSERT(static_cast(PropertyLocation::kField) == 0); - // PropertyCellTypeField comes next, its value must be kNoCell == 0 for - // dictionary mode PropertyDetails anyway. DCHECK_EQ(PropertyCellType::kNoCell, cell_type()); STATIC_ASSERT(static_cast(PropertyCellType::kNoCell) == 0); @@ -430,16 +432,13 @@ class PropertyDetails { // Only to be used for bytes obtained by ToByte. In particular, only used for // non-global dictionary properties. static PropertyDetails FromByte(uint8_t encoded_details) { - // The 0-extension to 32bit sets PropertyCellType to kNoCell and - // enumeration index to 0, as intended. Everything else is obtained from - // |encoded_details|. - + // The 0-extension to 32bit sets PropertyLocation to kField, + // PropertyCellType to kNoCell, and enumeration index to 0, as intended. + // Everything else is obtained from |encoded_details|. PropertyDetails details(encoded_details); - - DCHECK_EQ(0, details.dictionary_index()); DCHECK_EQ(PropertyLocation::kField, details.location()); DCHECK_EQ(PropertyCellType::kNoCell, details.cell_type()); - + DCHECK_EQ(0, details.dictionary_index()); return details; } diff --git a/v8/src/regexp/regexp-utils.cc b/v8/src/regexp/regexp-utils.cc index 8bb243d611..bf8479c5ec 100644 --- a/v8/src/regexp/regexp-utils.cc +++ b/v8/src/regexp/regexp-utils.cc @@ -49,7 +49,8 @@ MaybeHandle RegExpUtils::SetLastIndex(Isolate* isolate, Handle value_as_object = isolate->factory()->NewNumberFromInt64(value); if (HasInitialRegExpMap(isolate, *recv)) { - JSRegExp::cast(*recv).set_last_index(*value_as_object, SKIP_WRITE_BARRIER); + JSRegExp::cast(*recv).set_last_index(*value_as_object, + UPDATE_WRITE_BARRIER); return recv; } else { return Object::SetProperty( diff --git a/v8/src/runtime/runtime-classes.cc b/v8/src/runtime/runtime-classes.cc index 088ae3d73d..6c3501334c 100644 --- a/v8/src/runtime/runtime-classes.cc +++ b/v8/src/runtime/runtime-classes.cc @@ -650,7 +650,12 @@ MaybeHandle DefineClass( Handle prototype = CreateClassPrototype(isolate); DCHECK_EQ(*constructor, args[ClassBoilerplate::kConstructorArgumentIndex]); - args.set_at(ClassBoilerplate::kPrototypeArgumentIndex, *prototype); + // Temporarily change ClassBoilerplate::kPrototypeArgumentIndex for the + // subsequent calls, but use a scope to make sure to change it back before + // returning, to not corrupt the caller's argument frame (in particular, for + // the interpreter, to not clobber the register frame). + RuntimeArguments::ChangeValueScope set_prototype_value_scope( + isolate, &args, ClassBoilerplate::kPrototypeArgumentIndex, *prototype); if (!InitClassConstructor(isolate, class_boilerplate, constructor_parent, constructor, args) || diff --git a/v8/test/cctest/test-api-interceptors.cc b/v8/test/cctest/test-api-interceptors.cc index af5858eaef..36ae3a4838 100644 --- a/v8/test/cctest/test-api-interceptors.cc +++ b/v8/test/cctest/test-api-interceptors.cc @@ -5490,10 +5490,10 @@ void DatabaseGetter(Local name, const v8::PropertyCallbackInfo& info) { ApiTestFuzzer::Fuzz(); auto context = info.GetIsolate()->GetCurrentContext(); - Local db = info.Holder() - ->GetRealNamedProperty(context, v8_str("db")) - .ToLocalChecked() - .As(); + v8::MaybeLocal maybe_db = + info.Holder()->GetRealNamedProperty(context, v8_str("db")); + if (maybe_db.IsEmpty()) return; + Local db = maybe_db.ToLocalChecked().As(); if (!db->Has(context, name).FromJust()) return; info.GetReturnValue().Set(db->Get(context, name).ToLocalChecked()); } diff --git a/v8/test/cctest/test-js-weak-refs.cc b/v8/test/cctest/test-js-weak-refs.cc index 1291283515..52cf8a529e 100644 --- a/v8/test/cctest/test-js-weak-refs.cc +++ b/v8/test/cctest/test-js-weak-refs.cc @@ -831,7 +831,7 @@ TEST(TestRemoveUnregisterToken) { Handle token1 = CreateKey("token1", isolate); Handle token2 = CreateKey("token2", isolate); - Handle undefined = + Handle undefined = handle(ReadOnlyRoots(isolate).undefined_value(), isolate); Handle weak_cell1a = FinalizationRegistryRegister( @@ -861,9 +861,7 @@ TEST(TestRemoveUnregisterToken) { finalization_registry->RemoveUnregisterToken( JSReceiver::cast(*token2), isolate, - [undefined](WeakCell matched_cell) { - matched_cell.set_unregister_token(*undefined); - }, + JSFinalizationRegistry::kKeepMatchedCellsInRegistry, [](HeapObject, ObjectSlot, Object) {}); // Both weak_cell2a and weak_cell2b remain on the weak cell chains. @@ -989,15 +987,17 @@ TEST(UnregisterTokenHeapVerifier) { v8::HandleScope outer_scope(isolate); { - // Make a new FinalizationRegistry and register an object with an unregister - // token that's unreachable after the IIFE returns. + // Make a new FinalizationRegistry and register two objects with the same + // unregister token that's unreachable after the IIFE returns. v8::HandleScope scope(isolate); CompileRun( "var token = {}; " "var registry = new FinalizationRegistry(function () {}); " "(function () { " - " let o = {}; " - " registry.register(o, {}, token); " + " let o1 = {}; " + " let o2 = {}; " + " registry.register(o1, {}, token); " + " registry.register(o2, {}, token); " "})();"); } @@ -1022,5 +1022,52 @@ TEST(UnregisterTokenHeapVerifier) { EmptyMessageQueues(isolate); } +TEST(UnregisteredAndUnclearedCellHeapVerifier) { + if (!FLAG_incremental_marking) return; + ManualGCScope manual_gc_scope; +#ifdef VERIFY_HEAP + FLAG_verify_heap = true; +#endif + + CcTest::InitializeVM(); + v8::Isolate* isolate = CcTest::isolate(); + Heap* heap = CcTest::heap(); + v8::HandleScope outer_scope(isolate); + + { + // Make a new FinalizationRegistry and register an object with a token. + v8::HandleScope scope(isolate); + CompileRun( + "var token = {}; " + "var registry = new FinalizationRegistry(function () {}); " + "registry.register({}, undefined, token);"); + } + + // Start incremental marking to activate the marking barrier. + heap::SimulateIncrementalMarking(heap, false); + + { + // Make a WeakCell list with length >1, then unregister with the token to + // the WeakCell from the registry. The linked list manipulation keeps the + // unregistered WeakCell alive (i.e. not put into cleared_cells) due to the + // marking barrier from incremental marking. Then make the original token + // collectible. + v8::HandleScope scope(isolate); + CompileRun( + "registry.register({}); " + "registry.unregister(token); " + "token = 0;"); + } + + // Trigger GC. + CcTest::CollectAllGarbage(); + CcTest::CollectAllGarbage(); + + // Pump message loop to run the finalizer task, then the incremental marking + // task. The verifier will verify that live WeakCells don't point to dead + // unregister tokens. + EmptyMessageQueues(isolate); +} + } // namespace internal } // namespace v8 diff --git a/v8/test/mjsunit/compiler/regress-crbug-1228407.js b/v8/test/mjsunit/compiler/regress-crbug-1228407.js new file mode 100644 index 0000000000..f01eafb80e --- /dev/null +++ b/v8/test/mjsunit/compiler/regress-crbug-1228407.js @@ -0,0 +1,24 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --interrupt-budget=100 + +function foo() { + return function bar() { + a.p = 42; + for (let i = 0; i < 100; i++) this.p(); + this.p = a; + }; +} + +var a = foo(); +var b = foo(); + +a.prototype = { p() {} }; +b.prototype = { p() { + this.q = new a(); + for (let i = 0; i < 200; i++) ; +}}; + +new b(); diff --git a/v8/test/mjsunit/compiler/regress-crbug-1234764.js b/v8/test/mjsunit/compiler/regress-crbug-1234764.js new file mode 100644 index 0000000000..eca9346d17 --- /dev/null +++ b/v8/test/mjsunit/compiler/regress-crbug-1234764.js @@ -0,0 +1,21 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +function foo(arg_true) { + let o = {c0: 0}; + let c0a = arg_true ? 0 : "x"; + let c0 = Math.max(c0a, 0) + c0a; + let v01 = 2**32 + (o.c0 & 1); + let ra = ((2**32 - 1) >>> c0) - v01; + let rb = (-1) << (32 - c0); + return (ra^rb) >> 31; +} + +%PrepareFunctionForOptimization(foo); +assertEquals(0, foo(true)); +assertEquals(0, foo(true)); +%OptimizeFunctionOnNextCall(foo); +assertEquals(0, foo(true)); diff --git a/v8/test/mjsunit/compiler/regress-crbug-1234770.js b/v8/test/mjsunit/compiler/regress-crbug-1234770.js new file mode 100644 index 0000000000..22f68db902 --- /dev/null +++ b/v8/test/mjsunit/compiler/regress-crbug-1234770.js @@ -0,0 +1,14 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +function foo(a) { + return ((a & 1) == 1) & ((a & 2) == 1); +} + +%PrepareFunctionForOptimization(foo); +assertEquals(0, foo(1)); +%OptimizeFunctionOnNextCall(foo); +assertEquals(0, foo(1)); diff --git a/v8/test/mjsunit/compiler/regress-crbug-1247763.js b/v8/test/mjsunit/compiler/regress-crbug-1247763.js new file mode 100644 index 0000000000..760fb92d08 --- /dev/null +++ b/v8/test/mjsunit/compiler/regress-crbug-1247763.js @@ -0,0 +1,30 @@ +// Copyright 2021 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --allow-natives-syntax + +class C extends Array {}; +%NeverOptimizeFunction(C); + +for (let i = 0; i < 3; i++) { + + function store_global() { global = new C(); }; + store_global(); + %PrepareFunctionForOptimization(store_global); + store_global(); + %OptimizeFunctionOnNextCall(store_global); + store_global(); + + new C(42); + + function load_global() { global.p1 = {}; global.p2 = {}; } + if (i) { + load_global(); + %PrepareFunctionForOptimization(load_global); + load_global(); + %OptimizeFunctionOnNextCall(load_global); + load_global(); + } + +} diff --git a/v8/test/unittests/api/interceptor-unittest.cc b/v8/test/unittests/api/interceptor-unittest.cc index 8a1db3f823..bc00462a29 100644 --- a/v8/test/unittests/api/interceptor-unittest.cc +++ b/v8/test/unittests/api/interceptor-unittest.cc @@ -170,8 +170,8 @@ TEST_F(InterceptorLoggingTest, DispatchTest) { EXPECT_EQ(Run("obj.foo"), "named getter"); EXPECT_EQ(Run("obj[42]"), "indexed getter"); - EXPECT_EQ(Run("obj.foo = null"), "named setter"); - EXPECT_EQ(Run("obj[42] = null"), "indexed setter"); + EXPECT_EQ(Run("obj.foo = null"), "named setter, named descriptor"); + EXPECT_EQ(Run("obj[42] = null"), "indexed setter, indexed descriptor"); EXPECT_EQ(Run("Object.getOwnPropertyDescriptor(obj, 'foo')"), "named descriptor"); diff --git a/v8/test/unittests/compiler/machine-operator-reducer-unittest.cc b/v8/test/unittests/compiler/machine-operator-reducer-unittest.cc index d54e927e83..ba9de255f1 100644 --- a/v8/test/unittests/compiler/machine-operator-reducer-unittest.cc +++ b/v8/test/unittests/compiler/machine-operator-reducer-unittest.cc @@ -956,16 +956,12 @@ TEST_F(MachineOperatorReducerTest, ReduceToWord32RorWithParameters) { // (x << y) ^ (x >>> (32 - y)) => x ror (32 - y) Node* node3 = graph()->NewNode(machine()->Word32Xor(), shl_l, shr_l); Reduction reduction3 = Reduce(node3); - EXPECT_TRUE(reduction3.Changed()); - EXPECT_EQ(reduction3.replacement(), node3); - EXPECT_THAT(reduction3.replacement(), IsWord32Ror(value, sub)); + EXPECT_FALSE(reduction3.Changed()); // (x >>> (32 - y)) ^ (x << y) => x ror (32 - y) Node* node4 = graph()->NewNode(machine()->Word32Xor(), shr_l, shl_l); Reduction reduction4 = Reduce(node4); - EXPECT_TRUE(reduction4.Changed()); - EXPECT_EQ(reduction4.replacement(), node4); - EXPECT_THAT(reduction4.replacement(), IsWord32Ror(value, sub)); + EXPECT_FALSE(reduction4.Changed()); // Testing rotate right. Node* shl_r = graph()->NewNode(machine()->Word32Shl(), value, sub); @@ -988,16 +984,12 @@ TEST_F(MachineOperatorReducerTest, ReduceToWord32RorWithParameters) { // (x << (32 - y)) ^ (x >>> y) => x ror y Node* node7 = graph()->NewNode(machine()->Word32Xor(), shl_r, shr_r); Reduction reduction7 = Reduce(node7); - EXPECT_TRUE(reduction7.Changed()); - EXPECT_EQ(reduction7.replacement(), node7); - EXPECT_THAT(reduction7.replacement(), IsWord32Ror(value, shift)); + EXPECT_FALSE(reduction7.Changed()); // (x >>> y) ^ (x << (32 - y)) => x ror y Node* node8 = graph()->NewNode(machine()->Word32Xor(), shr_r, shl_r); Reduction reduction8 = Reduce(node8); - EXPECT_TRUE(reduction8.Changed()); - EXPECT_EQ(reduction8.replacement(), node8); - EXPECT_THAT(reduction8.replacement(), IsWord32Ror(value, shift)); + EXPECT_FALSE(reduction8.Changed()); } TEST_F(MachineOperatorReducerTest, ReduceToWord32RorWithConstant) { diff --git a/v8/test/unittests/heap/cppgc/ephemeron-pair-unittest.cc b/v8/test/unittests/heap/cppgc/ephemeron-pair-unittest.cc index 32a5929fe4..f5d07c7fd5 100644 --- a/v8/test/unittests/heap/cppgc/ephemeron-pair-unittest.cc +++ b/v8/test/unittests/heap/cppgc/ephemeron-pair-unittest.cc @@ -239,5 +239,50 @@ TEST_F(EphemeronPairTest, EphemeronPairWithEmptyMixinValue) { FinishMarking(); } +namespace { + +class KeyWithCallback final : public GarbageCollected { + public: + template + explicit KeyWithCallback(Callback callback) { + callback(this); + } + void Trace(Visitor*) const {} +}; + +class EphemeronHolderForKeyWithCallback final + : public GarbageCollected { + public: + EphemeronHolderForKeyWithCallback(KeyWithCallback* key, GCed* value) + : ephemeron_pair_(key, value) {} + void Trace(cppgc::Visitor* visitor) const { visitor->Trace(ephemeron_pair_); } + + private: + const EphemeronPair ephemeron_pair_; +}; + +} // namespace + +TEST_F(EphemeronPairTest, EphemeronPairWithKeyInConstruction) { + GCed* value = MakeGarbageCollected(GetAllocationHandle()); + Persistent holder; + InitializeMarker(*Heap::From(GetHeap()), GetPlatformHandle().get()); + FinishSteps(); + MakeGarbageCollected( + GetAllocationHandle(), [this, &holder, value](KeyWithCallback* thiz) { + // The test doesn't use conservative stack scanning to retain key to + // avoid retaining value as a side effect. + EXPECT_TRUE(HeapObjectHeader::FromObject(thiz).TryMarkAtomic()); + holder = MakeGarbageCollected( + GetAllocationHandle(), thiz, value); + // Finishing marking at this point will leave an ephemeron pair + // reachable where the key is still in construction. The GC needs to + // mark the value for such pairs as live in the atomic pause as they key + // is considered live. + FinishMarking(); + }); + EXPECT_TRUE(HeapObjectHeader::FromObject(value).IsMarked()); +} + } // namespace internal } // namespace cppgc diff --git a/v8/test/unittests/heap/cppgc/marker-unittest.cc b/v8/test/unittests/heap/cppgc/marker-unittest.cc index 8f8191c6d0..3a9d56eb67 100644 --- a/v8/test/unittests/heap/cppgc/marker-unittest.cc +++ b/v8/test/unittests/heap/cppgc/marker-unittest.cc @@ -7,12 +7,14 @@ #include #include "include/cppgc/allocation.h" +#include "include/cppgc/ephemeron-pair.h" #include "include/cppgc/internal/pointer-policies.h" #include "include/cppgc/member.h" #include "include/cppgc/persistent.h" #include "include/cppgc/trace-trait.h" #include "src/heap/cppgc/heap-object-header.h" #include "src/heap/cppgc/marking-visitor.h" +#include "src/heap/cppgc/object-allocator.h" #include "src/heap/cppgc/stats-collector.h" #include "test/unittests/heap/cppgc/tests.h" #include "testing/gtest/include/gtest/gtest.h" @@ -44,6 +46,8 @@ class MarkerTest : public testing::TestWithHeap { Marker* marker() const { return marker_.get(); } + void ResetMarker() { marker_.reset(); } + private: std::unique_ptr marker_; }; @@ -346,6 +350,50 @@ TEST_F(MarkerTest, SentinelNotClearedOnWeakPersistentHandling) { EXPECT_EQ(kSentinelPointer, root->weak_child()); } +namespace { + +class SimpleObject final : public GarbageCollected { + public: + void Trace(Visitor*) const {} +}; + +class ObjectWithEphemeronPair final + : public GarbageCollected { + public: + explicit ObjectWithEphemeronPair(AllocationHandle& handle) + : ephemeron_pair_(MakeGarbageCollected(handle), + MakeGarbageCollected(handle)) {} + + void Trace(Visitor* visitor) const { + // First trace the ephemeron pair. The key is not yet marked as live, so the + // pair should be recorded for later processing. Then strongly mark the key. + // Marking the key will not trigger another worklist processing iteration, + // as it merely continues the same loop for regular objects and will leave + // the main marking worklist empty. If recording the ephemeron pair doesn't + // as well, we will get a crash when destroying the marker. + visitor->Trace(ephemeron_pair_); + visitor->Trace(const_cast(ephemeron_pair_.key.Get())); + } + + private: + const EphemeronPair ephemeron_pair_; +}; + +} // namespace + +TEST_F(MarkerTest, MarkerProcessesAllEphemeronPairs) { + static const Marker::MarkingConfig config = { + MarkingConfig::CollectionType::kMajor, + MarkingConfig::StackState::kNoHeapPointers, + MarkingConfig::MarkingType::kAtomic}; + Persistent obj = + MakeGarbageCollected(GetAllocationHandle(), + GetAllocationHandle()); + InitializeMarker(*Heap::From(GetHeap()), GetPlatformHandle().get(), config); + marker()->FinishMarking(MarkingConfig::StackState::kNoHeapPointers); + ResetMarker(); +} + // Incremental Marking class IncrementalMarkingTest : public testing::TestWithHeap {