From 6b115d762dfeece2f3951a65b520d3a64213e2cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Wed, 31 Mar 2021 13:38:30 +0200 Subject: [PATCH] deps: patch V8 to 8.4.371.23 Refs: https://github.com/v8/v8/compare/8.4.371.19...8.4.371.23 PR-URL: https://github.com/nodejs/node/pull/38001 Reviewed-By: Richard Lau Reviewed-By: Michael Dawson Reviewed-By: Colin Ihrig Reviewed-By: Jiawen Geng --- deps/v8/include/v8-version.h | 2 +- .../builtins/promise-all-element-closure.tq | 45 ++++++++++++++---- deps/v8/src/heap/mark-compact.cc | 5 ++ .../wasm/baseline/arm/liftoff-assembler-arm.h | 11 +++-- deps/v8/src/wasm/baseline/liftoff-assembler.h | 9 ---- deps/v8/test/cctest/test-js-weak-refs.cc | 47 +++++++++++++++++++ .../mjsunit/regress/wasm/regress-1101304.js | 29 ++++++++++++ deps/v8/tools/whitespace.txt | 1 + 8 files changed, 126 insertions(+), 23 deletions(-) create mode 100644 deps/v8/test/mjsunit/regress/wasm/regress-1101304.js diff --git a/deps/v8/include/v8-version.h b/deps/v8/include/v8-version.h index cee7990e4bc193..d31f2a53076b7c 100644 --- a/deps/v8/include/v8-version.h +++ b/deps/v8/include/v8-version.h @@ -11,7 +11,7 @@ #define V8_MAJOR_VERSION 8 #define V8_MINOR_VERSION 4 #define V8_BUILD_NUMBER 371 -#define V8_PATCH_LEVEL 19 +#define V8_PATCH_LEVEL 23 // Use 1 for candidates and 0 otherwise. // (Boolean macro values are not supported by all preprocessors.) diff --git a/deps/v8/src/builtins/promise-all-element-closure.tq b/deps/v8/src/builtins/promise-all-element-closure.tq index 0b870ea3b185bc..4dfafec1c92555 100644 --- a/deps/v8/src/builtins/promise-all-element-closure.tq +++ b/deps/v8/src/builtins/promise-all-element-closure.tq @@ -82,7 +82,8 @@ const kPropertyArrayHashFieldMax: constexpr int31 transitioning macro PromiseAllResolveElementClosure( implicit context: Context)( - value: JSAny, function: JSFunction, wrapResultFunctor: F): JSAny { + value: JSAny, function: JSFunction, wrapResultFunctor: F, + hasResolveAndRejectClosures: constexpr bool): JSAny { // We use the {function}s context as the marker to remember whether this // resolve element closure was already called. It points to the resolve // element context (which is a FunctionContext) until it was called the @@ -98,10 +99,6 @@ transitioning macro PromiseAllResolveElementClosure( const nativeContext = LoadNativeContext(context); function.context = nativeContext; - // Update the value depending on whether Promise.all or - // Promise.allSettled is called. - const updatedValue = wrapResultFunctor.Call(nativeContext, value); - // Determine the index from the {function}. assert(kPropertyArrayNoHashSentinel == 0); const identityHash = @@ -116,13 +113,41 @@ transitioning macro PromiseAllResolveElementClosure( const elements = UnsafeCast(valuesArray.elements); const valuesLength = Convert(valuesArray.length); if (index < valuesLength) { - // The {index} is in bounds of the {values_array}, - // just store the {value} and continue. + // The {index} is in bounds of the {values_array}, check if this element has + // already been resolved, and store the {value} if not. + // + // Promise.allSettled, for each input element, has both a resolve and a + // reject closure that share an [[AlreadyCalled]] boolean. That is, the + // input element can only be settled once: after resolve is called, reject + // returns early, and vice versa. Using {function}'s context as the marker + // only tracks per-closure instead of per-element. When the second + // resolve/reject closure is called on the same index, values.object[index] + // will already exist and will not be the hole value. In that case, return + // early. Everything up to this point is not yet observable to user code. + // This is not a problem for Promise.all since Promise.all has a single + // resolve closure (no reject) per element. + if (hasResolveAndRejectClosures) { + if (elements.objects[index] != TheHole) deferred { + return Undefined; + } + } + + // Update the value depending on whether Promise.all or + // Promise.allSettled is called. + const updatedValue = wrapResultFunctor.Call(nativeContext, value); elements.objects[index] = updatedValue; } else { // Check if we need to grow the backing store. + // + // There's no need to check if this element has already been resolved for + // Promise.allSettled if {values_array} has not yet grown to the index. const newLength = index + 1; const elementsLength = elements.length_intptr; + + // Update the value depending on whether Promise.all or + // Promise.allSettled is called. + const updatedValue = wrapResultFunctor.Call(nativeContext, value); + if (index < elementsLength) { // The {index} is within bounds of the {elements} backing store, so // just store the {value} and update the "length" of the {values_array}. @@ -166,7 +191,7 @@ PromiseAllResolveElementClosure( js-implicit context: Context, receiver: JSAny, target: JSFunction)(value: JSAny): JSAny { return PromiseAllResolveElementClosure( - value, target, PromiseAllWrapResultAsFulfilledFunctor{}); + value, target, PromiseAllWrapResultAsFulfilledFunctor{}, false); } transitioning javascript builtin @@ -174,7 +199,7 @@ PromiseAllSettledResolveElementClosure( js-implicit context: Context, receiver: JSAny, target: JSFunction)(value: JSAny): JSAny { return PromiseAllResolveElementClosure( - value, target, PromiseAllSettledWrapResultAsFulfilledFunctor{}); + value, target, PromiseAllSettledWrapResultAsFulfilledFunctor{}, true); } transitioning javascript builtin @@ -182,6 +207,6 @@ PromiseAllSettledRejectElementClosure( js-implicit context: Context, receiver: JSAny, target: JSFunction)(value: JSAny): JSAny { return PromiseAllResolveElementClosure( - value, target, PromiseAllSettledWrapResultAsRejectedFunctor{}); + value, target, PromiseAllSettledWrapResultAsRejectedFunctor{}, true); } } diff --git a/deps/v8/src/heap/mark-compact.cc b/deps/v8/src/heap/mark-compact.cc index 53c215e2349a9a..d6368b9bdb8d7d 100644 --- a/deps/v8/src/heap/mark-compact.cc +++ b/deps/v8/src/heap/mark-compact.cc @@ -2531,6 +2531,11 @@ void MarkCompactCollector::ClearJSWeakRefs() { matched_cell.set_unregister_token(undefined); }, 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/deps/v8/src/wasm/baseline/arm/liftoff-assembler-arm.h b/deps/v8/src/wasm/baseline/arm/liftoff-assembler-arm.h index d69797886a30d4..eb91b79ea55a95 100644 --- a/deps/v8/src/wasm/baseline/arm/liftoff-assembler-arm.h +++ b/deps/v8/src/wasm/baseline/arm/liftoff-assembler-arm.h @@ -122,11 +122,16 @@ template inline void I64Binop(LiftoffAssembler* assm, LiftoffRegister dst, LiftoffRegister lhs, LiftoffRegister rhs) { - DCHECK_NE(dst.low_gp(), lhs.high_gp()); - DCHECK_NE(dst.low_gp(), rhs.high_gp()); - (assm->*op)(dst.low_gp(), lhs.low_gp(), rhs.low_gp(), SetCC, al); + Register dst_low = dst.low_gp(); + if (dst_low == lhs.high_gp() || dst_low == rhs.high_gp()) { + dst_low = assm->GetUnusedRegister( + kGpReg, LiftoffRegList::ForRegs(lhs, rhs, dst.high_gp())) + .gp(); + } + (assm->*op)(dst_low, lhs.low_gp(), rhs.low_gp(), SetCC, al); (assm->*op_with_carry)(dst.high_gp(), lhs.high_gp(), Operand(rhs.high_gp()), LeaveCC, al); + if (dst_low != dst.low_gp()) assm->mov(dst.low_gp(), dst_low); } template high.code()) { - // Establish the invariant that the register of the low word always has - // a lower code than the register of the high word. This guarantees that - // if a register pair of an input is reused for the result, the low - // word and high word registers are not swapped, i.e. the low word - // register of the result is not the high word register of the input, - // and vice versa. - std::swap(low, high); - } return LiftoffRegister::ForPair(low, high); } else if (kNeedS128RegPair && rc == kFpRegPair) { // kFpRegPair specific logic here because we need adjacent registers, not diff --git a/deps/v8/test/cctest/test-js-weak-refs.cc b/deps/v8/test/cctest/test-js-weak-refs.cc index d6c09a1fd4aa2a..4a5db34eafa7df 100644 --- a/deps/v8/test/cctest/test-js-weak-refs.cc +++ b/deps/v8/test/cctest/test-js-weak-refs.cc @@ -968,5 +968,52 @@ TEST(JSWeakRefTenuredInWorklist) { CHECK(weak_ref->target().IsUndefined(isolate)); } +TEST(UnregisterTokenHeapVerifier) { + FLAG_harmony_weak_refs = true; + 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 an 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); " + "})();"); + } + + // GC so the WeakCell corresponding to o is moved from the active_cells to + // cleared_cells. + CcTest::CollectAllGarbage(); + CcTest::CollectAllGarbage(); + + { + // Override the unregister token to make the original object collectible. + v8::HandleScope scope(isolate); + CompileRun("token = 0;"); + } + + heap::SimulateIncrementalMarking(heap, true); + + // Pump message loop to run the finalizer task, then the incremental marking + // task. The finalizer task will pop the WeakCell from the cleared list. This + // should make the unregister_token slot undefined. That slot is iterated as a + // custom weak pointer, so if it is not made undefined, the verifier as part + // of the incremental marking task will crash. + EmptyMessageQueues(isolate); +} + } // namespace internal } // namespace v8 diff --git a/deps/v8/test/mjsunit/regress/wasm/regress-1101304.js b/deps/v8/test/mjsunit/regress/wasm/regress-1101304.js new file mode 100644 index 00000000000000..36331d094ab806 --- /dev/null +++ b/deps/v8/test/mjsunit/regress/wasm/regress-1101304.js @@ -0,0 +1,29 @@ +// Copyright 2020 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. + +load('test/mjsunit/wasm/wasm-module-builder.js'); + +const builder = new WasmModuleBuilder(); +builder.addType(makeSig( + [kWasmI64, kWasmI32, kWasmF64, kWasmI64, kWasmI64, kWasmI32, kWasmI64], + [])); +builder.addFunction(undefined, 0 /* sig */).addBody([ + kExprI32Const, 0, // i32.const + kExprIf, kWasmStmt, // if @3 + kExprI32Const, 1, // i32.const + kExprIf, kWasmStmt, // if @7 + kExprNop, // nop + kExprElse, // else @10 + kExprUnreachable, // unreachable + kExprEnd, // end @12 + kExprLocalGet, 0x06, // local.get + kExprLocalSet, 0x00, // local.set + kExprLocalGet, 0x03, // local.get + kExprLocalGet, 0x00, // local.get + kExprI64Sub, // i64.sub + kExprDrop, // drop + kExprUnreachable, // unreachable + kExprEnd // end @24 +]); +builder.toModule(); diff --git a/deps/v8/tools/whitespace.txt b/deps/v8/tools/whitespace.txt index e246f93f14d83a..4668daa518fe78 100644 --- a/deps/v8/tools/whitespace.txt +++ b/deps/v8/tools/whitespace.txt @@ -14,3 +14,4 @@ I'm starting to think that just adding trailing whitespaces might not be bad. Because whitespaces are not that funny..... Today's answer to life the universe and everything is 12950! Today's answer to life the universe and everything is 6728! ++1 \ No newline at end of file