[ignition/sparkplug] Fix folded interrupt check

Change the folded interrupt check to be on JumpLoop only, to avoid
calling it from Return. The call from Return could cause spurious stack
overflows, which interacted poorly with async functions that had already
resolved their promise.

Now the bytecode budget interrupt function is split into two functions,
one which does the stack check and one which doesn't. The former is
still called from JumpLoop, the latter is called from Return.

Bug: chromium:1231952, chromium:1232105
Change-Id: I8c4e2937f64b5f8fdbd6c1fcb2a76ec5f090ae3c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3049076
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75891}
This commit is contained in:
Leszek Swirski 2021-07-23 16:16:57 +02:00 committed by V8 LUCI CQ
parent 1f79309aaa
commit ef17601fa7
4 changed files with 99 additions and 74 deletions

View File

@ -558,7 +558,7 @@ void BaselineCompiler::UpdateInterruptBudgetAndJumpToLabel(
if (weight < 0) {
SaveAccumulatorScope accumulator_scope(&basm_);
CallRuntime(Runtime::kBytecodeBudgetInterruptFromBytecode,
CallRuntime(Runtime::kBytecodeBudgetInterruptWithStackCheckFromBytecode,
__ FunctionOperand());
}
}

View File

@ -1050,8 +1050,12 @@ void InterpreterAssembler::UpdateInterruptBudget(TNode<Int32T> weight,
Branch(condition, &ok, &interrupt_check);
BIND(&interrupt_check);
CallRuntime(Runtime::kBytecodeBudgetInterruptFromBytecode, GetContext(),
function);
// JumpLoop should do a stack check as part of the interrupt.
CallRuntime(
bytecode() == Bytecode::kJumpLoop
? Runtime::kBytecodeBudgetInterruptWithStackCheckFromBytecode
: Runtime::kBytecodeBudgetInterruptFromBytecode,
GetContext(), function);
Goto(&done);
BIND(&ok);

View File

@ -329,26 +329,10 @@ RUNTIME_FUNCTION(Runtime_StackGuardWithGap) {
return isolate->stack_guard()->HandleInterrupts();
}
RUNTIME_FUNCTION(Runtime_BytecodeBudgetInterruptFromBytecode) {
HandleScope scope(isolate);
DCHECK_EQ(1, args.length());
CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
TRACE_EVENT0("v8.execute", "V8.BytecodeBudgetInterrupt");
// Check for stack interrupts here so that we can fold the interrupt check
// into bytecode budget interrupts.
StackLimitCheck check(isolate);
if (check.JsHasOverflowed()) {
// TODO(leszeks): We shouldn't actually get StackOverflows here, consider
// DCHECKing instead.
return isolate->StackOverflow();
} else if (check.InterruptRequested()) {
Object return_value = isolate->stack_guard()->HandleInterrupts();
if (!return_value.IsUndefined(isolate)) {
return return_value;
}
}
namespace {
void BytecodeBudgetInterruptFromBytecode(Isolate* isolate,
Handle<JSFunction> function) {
function->SetInterruptBudget();
bool should_mark_for_optimization = function->has_feedback_vector();
if (!function->has_feedback_vector()) {
@ -377,6 +361,42 @@ RUNTIME_FUNCTION(Runtime_BytecodeBudgetInterruptFromBytecode) {
isolate->counters()->runtime_profiler_ticks()->Increment();
isolate->runtime_profiler()->MarkCandidatesForOptimizationFromBytecode();
}
}
} // namespace
RUNTIME_FUNCTION(Runtime_BytecodeBudgetInterruptWithStackCheckFromBytecode) {
HandleScope scope(isolate);
DCHECK_EQ(1, args.length());
CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
TRACE_EVENT0("v8.execute", "V8.BytecodeBudgetInterruptWithStackCheck");
// Check for stack interrupts here so that we can fold the interrupt check
// into bytecode budget interrupts.
StackLimitCheck check(isolate);
if (check.JsHasOverflowed()) {
// We ideally wouldn't actually get StackOverflows here, since we stack
// check on bytecode entry, but it's possible that this check fires due to
// the runtime function call being what overflows the stack.
// if our function entry
return isolate->StackOverflow();
} else if (check.InterruptRequested()) {
Object return_value = isolate->stack_guard()->HandleInterrupts();
if (!return_value.IsUndefined(isolate)) {
return return_value;
}
}
BytecodeBudgetInterruptFromBytecode(isolate, function);
return ReadOnlyRoots(isolate).undefined_value();
}
RUNTIME_FUNCTION(Runtime_BytecodeBudgetInterruptFromBytecode) {
HandleScope scope(isolate);
DCHECK_EQ(1, args.length());
CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
TRACE_EVENT0("v8.execute", "V8.BytecodeBudgetInterrupt");
BytecodeBudgetInterruptFromBytecode(isolate, function);
return ReadOnlyRoots(isolate).undefined_value();
}

View File

@ -206,58 +206,59 @@ namespace internal {
#define FOR_EACH_INTRINSIC_INTL(F, I)
#endif // V8_INTL_SUPPORT
#define FOR_EACH_INTRINSIC_INTERNAL(F, I) \
F(AccessCheck, 1, 1) \
F(AllocateByteArray, 1, 1) \
F(AllocateInYoungGeneration, 2, 1) \
F(AllocateInOldGeneration, 2, 1) \
F(AllocateSeqOneByteString, 1, 1) \
F(AllocateSeqTwoByteString, 1, 1) \
F(AllowDynamicFunction, 1, 1) \
I(CreateAsyncFromSyncIterator, 1, 1) \
F(CreateListFromArrayLike, 1, 1) \
F(DoubleToStringWithRadix, 2, 1) \
F(FatalProcessOutOfMemoryInAllocateRaw, 0, 1) \
F(FatalProcessOutOfMemoryInvalidArrayLength, 0, 1) \
F(GetAndResetRuntimeCallStats, -1 /* <= 2 */, 1) \
F(GetTemplateObject, 3, 1) \
F(IncrementUseCounter, 1, 1) \
F(BytecodeBudgetInterruptFromBytecode, 1, 1) \
F(BytecodeBudgetInterruptFromCode, 1, 1) \
F(NewError, 2, 1) \
F(NewReferenceError, 2, 1) \
F(NewSyntaxError, 2, 1) \
F(NewTypeError, -1 /* [1, 4] */, 1) \
F(OrdinaryHasInstance, 2, 1) \
F(PromoteScheduledException, 0, 1) \
F(ReportMessageFromMicrotask, 1, 1) \
F(ReThrow, 1, 1) \
F(RunMicrotaskCallback, 2, 1) \
F(PerformMicrotaskCheckpoint, 0, 1) \
F(StackGuard, 0, 1) \
F(StackGuardWithGap, 1, 1) \
F(Throw, 1, 1) \
F(ThrowApplyNonFunction, 1, 1) \
F(ThrowCalledNonCallable, 1, 1) \
F(ThrowConstructedNonConstructable, 1, 1) \
F(ThrowConstructorReturnedNonObject, 0, 1) \
F(ThrowInvalidStringLength, 0, 1) \
F(ThrowInvalidTypedArrayAlignment, 2, 1) \
F(ThrowIteratorError, 1, 1) \
F(ThrowSpreadArgError, 2, 1) \
F(ThrowIteratorResultNotAnObject, 1, 1) \
F(ThrowNotConstructor, 1, 1) \
F(ThrowPatternAssignmentNonCoercible, 1, 1) \
F(ThrowRangeError, -1 /* >= 1 */, 1) \
F(ThrowReferenceError, 1, 1) \
F(ThrowAccessedUninitializedVariable, 1, 1) \
F(ThrowStackOverflow, 0, 1) \
F(ThrowSymbolAsyncIteratorInvalid, 0, 1) \
F(ThrowSymbolIteratorInvalid, 0, 1) \
F(ThrowThrowMethodMissing, 0, 1) \
F(ThrowTypeError, -1 /* >= 1 */, 1) \
F(ThrowTypeErrorIfStrict, -1 /* >= 1 */, 1) \
F(Typeof, 1, 1) \
#define FOR_EACH_INTRINSIC_INTERNAL(F, I) \
F(AccessCheck, 1, 1) \
F(AllocateByteArray, 1, 1) \
F(AllocateInYoungGeneration, 2, 1) \
F(AllocateInOldGeneration, 2, 1) \
F(AllocateSeqOneByteString, 1, 1) \
F(AllocateSeqTwoByteString, 1, 1) \
F(AllowDynamicFunction, 1, 1) \
I(CreateAsyncFromSyncIterator, 1, 1) \
F(CreateListFromArrayLike, 1, 1) \
F(DoubleToStringWithRadix, 2, 1) \
F(FatalProcessOutOfMemoryInAllocateRaw, 0, 1) \
F(FatalProcessOutOfMemoryInvalidArrayLength, 0, 1) \
F(GetAndResetRuntimeCallStats, -1 /* <= 2 */, 1) \
F(GetTemplateObject, 3, 1) \
F(IncrementUseCounter, 1, 1) \
F(BytecodeBudgetInterruptFromBytecode, 1, 1) \
F(BytecodeBudgetInterruptWithStackCheckFromBytecode, 1, 1) \
F(BytecodeBudgetInterruptFromCode, 1, 1) \
F(NewError, 2, 1) \
F(NewReferenceError, 2, 1) \
F(NewSyntaxError, 2, 1) \
F(NewTypeError, -1 /* [1, 4] */, 1) \
F(OrdinaryHasInstance, 2, 1) \
F(PromoteScheduledException, 0, 1) \
F(ReportMessageFromMicrotask, 1, 1) \
F(ReThrow, 1, 1) \
F(RunMicrotaskCallback, 2, 1) \
F(PerformMicrotaskCheckpoint, 0, 1) \
F(StackGuard, 0, 1) \
F(StackGuardWithGap, 1, 1) \
F(Throw, 1, 1) \
F(ThrowApplyNonFunction, 1, 1) \
F(ThrowCalledNonCallable, 1, 1) \
F(ThrowConstructedNonConstructable, 1, 1) \
F(ThrowConstructorReturnedNonObject, 0, 1) \
F(ThrowInvalidStringLength, 0, 1) \
F(ThrowInvalidTypedArrayAlignment, 2, 1) \
F(ThrowIteratorError, 1, 1) \
F(ThrowSpreadArgError, 2, 1) \
F(ThrowIteratorResultNotAnObject, 1, 1) \
F(ThrowNotConstructor, 1, 1) \
F(ThrowPatternAssignmentNonCoercible, 1, 1) \
F(ThrowRangeError, -1 /* >= 1 */, 1) \
F(ThrowReferenceError, 1, 1) \
F(ThrowAccessedUninitializedVariable, 1, 1) \
F(ThrowStackOverflow, 0, 1) \
F(ThrowSymbolAsyncIteratorInvalid, 0, 1) \
F(ThrowSymbolIteratorInvalid, 0, 1) \
F(ThrowThrowMethodMissing, 0, 1) \
F(ThrowTypeError, -1 /* >= 1 */, 1) \
F(ThrowTypeErrorIfStrict, -1 /* >= 1 */, 1) \
F(Typeof, 1, 1) \
F(UnwindAndFindExceptionHandler, 0, 1)
#define FOR_EACH_INTRINSIC_LITERALS(F, I) \