[maglev] Fix exception phi for receiver in constructors

Our previous assumption that the receiver is immutable is incorrect in
constructors. Change the current logic (which never generates an
exception phi for receivers, but instead re-uses the parameter slot)
into forcing the receiver exception phi to be allocated (and spilled) in
the receiver parameter slot.

Bug: v8:7700
Change-Id: I1ba92b2e711dc0fcd7c818526b9c199cadcdd3bf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3948586
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83684}
This commit is contained in:
Leszek Swirski 2022-10-13 11:00:15 +02:00 committed by V8 LUCI CQ
parent 84c8c29136
commit a79dde2bce
4 changed files with 56 additions and 17 deletions

View File

@ -33,21 +33,7 @@ MergePointInterpreterFrameState::NewForCatchBlock(
} }
frame_state.ForEachParameter( frame_state.ForEachParameter(
unit, [&](ValueNode*& entry, interpreter::Register reg) { unit, [&](ValueNode*& entry, interpreter::Register reg) {
if (!is_inline && reg.is_receiver()) { entry = state->NewExceptionPhi(zone, reg, handler_offset);
// The receiver is a special case for a fairly silly reason:
// OptimizedFrame::Summarize requires the receiver (and the function)
// to be in a stack slot, since it's value must be available even
// though we're not deoptimizing (and thus register states are not
// available). Exception phis could be allocated in a register.
// Since the receiver is immutable, simply reuse its InitialValue
// node.
// For inlined functions / nested graph generation, this a) doesn't
// work (there's no receiver stack slot); and b) isn't necessary
// (Summarize only looks at noninlined functions).
entry = graph->parameters()[0];
} else {
entry = state->NewExceptionPhi(zone, reg, handler_offset);
}
}); });
frame_state.context(unit) = frame_state.context(unit) =
state->NewExceptionPhi(zone, context_register, handler_offset); state->NewExceptionPhi(zone, context_register, handler_offset);

View File

@ -394,7 +394,8 @@ void StraightForwardRegisterAllocator::AllocateRegisters() {
// (the first one by default) that is marked with the // (the first one by default) that is marked with the
// virtual_accumulator and force kReturnRegister0. This corresponds to // virtual_accumulator and force kReturnRegister0. This corresponds to
// the exception message object. // the exception message object.
Phi* phi = block->phis()->first(); Phi::List::Iterator phi_it = block->phis()->begin();
Phi* phi = *phi_it;
DCHECK_EQ(phi->input_count(), 0); DCHECK_EQ(phi->input_count(), 0);
if (phi->owner() == interpreter::Register::virtual_accumulator() && if (phi->owner() == interpreter::Register::virtual_accumulator() &&
!phi->is_dead()) { !phi->is_dead()) {
@ -405,6 +406,31 @@ void StraightForwardRegisterAllocator::AllocateRegisters() {
<< phi->result().operand() << std::endl; << phi->result().operand() << std::endl;
} }
} }
// The receiver is the next phi after the accumulator (or the first phi
// if there is no accumulator).
if (phi->owner() == interpreter::Register::virtual_accumulator()) {
++phi_it;
phi = *phi_it;
}
DCHECK(phi->owner().is_receiver());
// The receiver is a special case for a fairly silly reason:
// OptimizedFrame::Summarize requires the receiver (and the function)
// to be in a stack slot, since it's value must be available even
// though we're not deoptimizing (and thus register states are not
// available).
//
// TODO(leszeks):
// For inlined functions / nested graph generation, this a) doesn't
// work (there's no receiver stack slot); and b) isn't necessary
// (Summarize only looks at noninlined functions).
phi->Spill(compiler::AllocatedOperand(
compiler::AllocatedOperand::STACK_SLOT,
MachineRepresentation::kTagged,
(StandardFrameConstants::kExpressionsOffset -
UnoptimizedFrameConstants::kRegisterFileFromFp) /
kSystemPointerSize +
interpreter::Register::receiver().index()));
phi->result().SetAllocated(phi->spill_slot());
} }
// Secondly try to assign the phi to a free register. // Secondly try to assign the phi to a free register.
for (Phi* phi : *block->phis()) { for (Phi* phi : *block->phis()) {

View File

@ -11,7 +11,7 @@ let Class = class extends Base {
super(); super();
} }
}; };
for (let i = 0; i < 100; i++) { for (let i = 0; i < 10; i++) {
Class = class extends Class { Class = class extends Class {
constructor() { constructor() {
try { try {

View File

@ -0,0 +1,27 @@
// Copyright 2022 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: --maglev --allow-natives-syntax
class Base {
constructor() {
}
}
class Class extends Base {
constructor() {
try {
// Super initialises the "this" pointer.
super();
throw new Error();
} catch (e) {
// Access to "this" should be valid.
assertNotNull(this);
}
}
};
%PrepareFunctionForOptimization(Class);
new Class();
new Class();
%OptimizeMaglevOnNextCall(Class);
new Class();