[compiler] Fix a (harmless) DCHECK failure

A frame state's outer frame state input can be kDeadValue. A DCHECK
did not take that into account. In release builds there was no issue
because we branch on the opcode anyways.

While fixing this bug, I'm strengthening the FrameState class such that
a FrameState node must have a kFrameState operator. I'm also
- changing the result type of outer_frame_state() from FrameState to
  Node* since it may in fact not be a kFrameState;
- removing has_outer_frame_state() because I find it unintuitive to
  have outer_frame_state() return non-NULL even when
  has_outer_frame_state() would return true.

Bug: chromium:1224758
Change-Id: I8ebed75c62e31f7eef71e2941fd18869d8a56af3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3001356Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75553}

src/compiler/backend/instruction-selector.cc

@@ -752,16 +752,14 @@ size_t InstructionSelector::AddInputsToFrameStateDescriptor(
     FrameStateDescriptor* descriptor, FrameState state, OperandGenerator* g,
     StateObjectDeduplicator* deduplicator, InstructionOperandVector* inputs,
     FrameStateInputKind kind, Zone* zone) {
-  DCHECK_EQ(IrOpcode::kFrameState, state->op()->opcode());
-
   size_t entries = 0;
   size_t initial_size = inputs->size();
   USE(initial_size);  // initial_size is only used for debug.
 
   if (descriptor->outer_state()) {
     entries += AddInputsToFrameStateDescriptor(
-        descriptor->outer_state(), state.outer_frame_state(), g, deduplicator,
-        inputs, kind, zone);
+        descriptor->outer_state(), FrameState{state.outer_frame_state()}, g,
+        deduplicator, inputs, kind, zone);
   }
 
   Node* parameters = state.parameters();
@@ -3343,9 +3341,9 @@ FrameStateDescriptor* GetFrameStateDescriptorInternal(Zone* zone,
   int stack = state_info.type() == FrameStateType::kUnoptimizedFunction ? 1 : 0;
 
   FrameStateDescriptor* outer_state = nullptr;
-  if (state.has_outer_frame_state()) {
-    outer_state =
-        GetFrameStateDescriptorInternal(zone, state.outer_frame_state());
+  if (state.outer_frame_state()->opcode() == IrOpcode::kFrameState) {
+    outer_state = GetFrameStateDescriptorInternal(
+        zone, FrameState{state.outer_frame_state()});
   }
 
 #if V8_ENABLE_WEBASSEMBLY

src/compiler/common-operator.h

@@ -617,12 +617,7 @@ class CommonNodeWrapperBase : public NodeWrapper {
 class FrameState : public CommonNodeWrapperBase {
  public:
   explicit constexpr FrameState(Node* node) : CommonNodeWrapperBase(node) {
-    // TODO(jgruber): Disallow kStart (needed for PromiseConstructorBasic unit
-    // test, among others). Also, outer_frame_state points at the start node
-    // for non-inlined functions. This could be avoided by checking
-    // has_outer_frame_state() before casting to FrameState.
-    DCHECK(node->opcode() == IrOpcode::kFrameState ||
-           node->opcode() == IrOpcode::kStart);
+    DCHECK_EQ(node->opcode(), IrOpcode::kFrameState);
   }
 
   FrameStateInfo frame_state_info() const {
@@ -657,15 +652,13 @@ class FrameState : public CommonNodeWrapperBase {
   Node* function() const { return node()->InputAt(kFrameStateFunctionInput); }
 
   // An outer frame state exists for inlined functions; otherwise it points at
-  // the start node.
-  bool has_outer_frame_state() const {
-    Node* maybe_outer_frame_state = node()->InputAt(kFrameStateOuterStateInput);
-    DCHECK(maybe_outer_frame_state->opcode() == IrOpcode::kFrameState ||
-           maybe_outer_frame_state->opcode() == IrOpcode::kStart);
-    return maybe_outer_frame_state->opcode() == IrOpcode::kFrameState;
-  }
-  FrameState outer_frame_state() const {
-    return FrameState{node()->InputAt(kFrameStateOuterStateInput)};
+  // the start node. Could also be dead.
+  Node* outer_frame_state() const {
+    Node* result = node()->InputAt(kFrameStateOuterStateInput);
+    DCHECK(result->opcode() == IrOpcode::kFrameState ||
+           result->opcode() == IrOpcode::kStart ||
+           result->opcode() == IrOpcode::kDeadValue);
+    return result;
   }
 };
 

src/compiler/graph-assembler.cc

@@ -861,11 +861,11 @@ Node* GraphAssembler::DynamicCheckMapsWithDeoptUnless(Node* condition,
                                                       Node* value, Node* map,
                                                       Node* feedback_vector,
                                                       FrameState frame_state) {
-  return AddNode(graph()->NewNode(common()->DynamicCheckMapsWithDeoptUnless(
-                                      frame_state.has_outer_frame_state()),
-                                  condition, slot_index, value, map,
-                                  feedback_vector, frame_state, effect(),
-                                  control()));
+  return AddNode(graph()->NewNode(
+      common()->DynamicCheckMapsWithDeoptUnless(
+          frame_state.outer_frame_state()->opcode() == IrOpcode::kFrameState),
+      condition, slot_index, value, map, feedback_vector, frame_state, effect(),
+      control()));
 }
 
 TNode<Object> GraphAssembler::Call(const CallDescriptor* call_descriptor,

src/compiler/js-call-reducer.cc

@@ -4072,7 +4072,7 @@ JSCallReducer::ReduceCallOrConstructWithArrayLikeOrSpreadOfCreateArguments(
       arraylike_or_spread_index - JSCallOrConstructNode::FirstArgumentIndex();
   // Check if are spreading to inlined arguments or to the arguments of
   // the outermost function.
-  if (!frame_state.has_outer_frame_state()) {
+  if (frame_state.outer_frame_state()->opcode() != IrOpcode::kFrameState) {
     Operator const* op;
     if (IsCallWithArrayLikeOrSpread(node)) {
       static constexpr int kTargetAndReceiver = 2;
@@ -4087,10 +4087,10 @@ JSCallReducer::ReduceCallOrConstructWithArrayLikeOrSpreadOfCreateArguments(
     NodeProperties::ChangeOp(node, op);
     return Changed(node);
   }
-  FrameState outer_state = frame_state.outer_frame_state();
   // Get to the actual frame state from which to extract the arguments;
   // we can only optimize this in case the {node} was already inlined into
   // some other function (and same for the {arg_array}).
+  FrameState outer_state{frame_state.outer_frame_state()};
   FrameStateInfo outer_info = outer_state.frame_state_info();
   if (outer_info.type() == FrameStateType::kArgumentsAdaptor) {
     // Need to take the parameters from the arguments adaptor.

src/compiler/js-create-lowering.cc

@@ -156,7 +156,7 @@ Reduction JSCreateLowering::ReduceJSCreateArguments(Node* node) {
 
   // Use the ArgumentsAccessStub for materializing both mapped and unmapped
   // arguments object, but only for non-inlined (i.e. outermost) frames.
-  if (!frame_state.has_outer_frame_state()) {
+  if (frame_state.outer_frame_state()->opcode() != IrOpcode::kFrameState) {
     switch (type) {
       case CreateArgumentsType::kMappedArguments: {
         // TODO(turbofan): Duplicate parameters are not handled yet.

src/compiler/js-generic-lowering.cc

@@ -283,7 +283,7 @@ void JSGenericLowering::LowerJSLoadProperty(Node* node) {
   JSLoadPropertyNode n(node);
   const PropertyAccess& p = n.Parameters();
   FrameState frame_state = n.frame_state();
-  FrameState outer_state = frame_state.outer_frame_state();
+  Node* outer_state = frame_state.outer_frame_state();
   STATIC_ASSERT(n.FeedbackVectorIndex() == 2);
   if (outer_state->opcode() != IrOpcode::kFrameState) {
     n->RemoveInput(n.FeedbackVectorIndex());
@@ -307,7 +307,7 @@ void JSGenericLowering::LowerJSLoadNamed(Node* node) {
   JSLoadNamedNode n(node);
   NamedAccess const& p = n.Parameters();
   FrameState frame_state = n.frame_state();
-  FrameState outer_state = frame_state.outer_frame_state();
+  Node* outer_state = frame_state.outer_frame_state();
   STATIC_ASSERT(n.FeedbackVectorIndex() == 1);
   if (!p.feedback().IsValid()) {
     n->RemoveInput(n.FeedbackVectorIndex());
@@ -365,7 +365,7 @@ void JSGenericLowering::LowerJSLoadGlobal(Node* node) {
   const LoadGlobalParameters& p = n.Parameters();
   CallDescriptor::Flags flags = FrameStateFlagForCall(node);
   FrameState frame_state = n.frame_state();
-  FrameState outer_state = frame_state.outer_frame_state();
+  Node* outer_state = frame_state.outer_frame_state();
   STATIC_ASSERT(n.FeedbackVectorIndex() == 0);
   if (outer_state->opcode() != IrOpcode::kFrameState) {
     n->RemoveInput(n.FeedbackVectorIndex());
@@ -412,7 +412,7 @@ void JSGenericLowering::LowerJSStoreProperty(Node* node) {
   JSStorePropertyNode n(node);
   const PropertyAccess& p = n.Parameters();
   FrameState frame_state = n.frame_state();
-  FrameState outer_state = frame_state.outer_frame_state();
+  Node* outer_state = frame_state.outer_frame_state();
   STATIC_ASSERT(n.FeedbackVectorIndex() == 3);
   if (outer_state->opcode() != IrOpcode::kFrameState) {
     n->RemoveInput(n.FeedbackVectorIndex());
@@ -430,7 +430,7 @@ void JSGenericLowering::LowerJSStoreNamed(Node* node) {
   JSStoreNamedNode n(node);
   NamedAccess const& p = n.Parameters();
   FrameState frame_state = n.frame_state();
-  FrameState outer_state = frame_state.outer_frame_state();
+  Node* outer_state = frame_state.outer_frame_state();
   STATIC_ASSERT(n.FeedbackVectorIndex() == 2);
   if (!p.feedback().IsValid()) {
     n->RemoveInput(n.FeedbackVectorIndex());
@@ -455,7 +455,7 @@ void JSGenericLowering::LowerJSStoreNamedOwn(Node* node) {
   CallDescriptor::Flags flags = FrameStateFlagForCall(node);
   StoreNamedOwnParameters const& p = n.Parameters();
   FrameState frame_state = n.frame_state();
-  FrameState outer_state = frame_state.outer_frame_state();
+  Node* outer_state = frame_state.outer_frame_state();
   STATIC_ASSERT(n.FeedbackVectorIndex() == 2);
   if (outer_state->opcode() != IrOpcode::kFrameState) {
     n->RemoveInput(n.FeedbackVectorIndex());
@@ -477,7 +477,7 @@ void JSGenericLowering::LowerJSStoreGlobal(Node* node) {
   JSStoreGlobalNode n(node);
   const StoreGlobalParameters& p = n.Parameters();
   FrameState frame_state = n.frame_state();
-  FrameState outer_state = frame_state.outer_frame_state();
+  Node* outer_state = frame_state.outer_frame_state();
   STATIC_ASSERT(n.FeedbackVectorIndex() == 1);
   if (outer_state->opcode() != IrOpcode::kFrameState) {
     n->RemoveInput(n.FeedbackVectorIndex());

src/compiler/js-inlining.cc

@@ -497,9 +497,9 @@ Reduction JSInliner::ReduceJSCall(Node* node) {
   // To ensure inlining always terminates, we have an upper limit on inlining
   // the nested calls.
   int nesting_level = 0;
-  for (FrameState frame_state = FrameState{call.frame_state()};
+  for (Node* frame_state = call.frame_state();
        frame_state->opcode() == IrOpcode::kFrameState;
-       frame_state = frame_state.outer_frame_state()) {
+       frame_state = FrameState{frame_state}.outer_frame_state()) {
     nesting_level++;
     if (nesting_level > kMaxDepthForInlining) {
       TRACE("Not inlining "

test/unittests/compiler/js-call-reducer-unittest.cc

@@ -120,6 +120,14 @@ class JSCallReducerTest : public TypedGraphTest {
                               CallFeedbackRelation::kTarget);
   }
 
+  Node* DummyFrameState() {
+    return graph()->NewNode(
+        common()->FrameState(BytecodeOffset{42},
+                             OutputFrameStateCombine::Ignore(), nullptr),
+        graph()->start(), graph()->start(), graph()->start(), graph()->start(),
+        graph()->start(), graph()->start());
+  }
+
  private:
   JSOperatorBuilder javascript_;
   CompilationDependencies deps_;
@@ -131,7 +139,7 @@ TEST_F(JSCallReducerTest, PromiseConstructorNoArgs) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* feedback = UndefinedConstant();
 
   Node* construct = graph()->NewNode(
@@ -151,7 +159,7 @@ TEST_F(JSCallReducerTest, PromiseConstructorSubclass) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* feedback = UndefinedConstant();
 
   Node* executor = UndefinedConstant();
@@ -170,7 +178,7 @@ TEST_F(JSCallReducerTest, PromiseConstructorBasic) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* feedback = UndefinedConstant();
 
   Node* executor = UndefinedConstant();
@@ -190,7 +198,7 @@ TEST_F(JSCallReducerTest, PromiseConstructorWithHook) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* feedback = UndefinedConstant();
 
   Node* executor = UndefinedConstant();
@@ -223,7 +231,7 @@ TEST_F(JSCallReducerTest, MathUnaryWithNumber) {
     Node* effect = graph()->start();
     Node* control = graph()->start();
     Node* context = UndefinedConstant();
-    Node* frame_state = graph()->start();
+    Node* frame_state = DummyFrameState();
     Node* jsfunction = MathFunction(fnc);
     Node* p0 = Parameter(Type::Any(), 0);
     Node* feedback = UndefinedConstant();
@@ -253,7 +261,7 @@ TEST_F(JSCallReducerTest, MathBinaryWithNumber) {
     Node* effect = graph()->start();
     Node* control = graph()->start();
     Node* context = UndefinedConstant();
-    Node* frame_state = graph()->start();
+    Node* frame_state = DummyFrameState();
     Node* p0 = Parameter(Type::Any(), 0);
     Node* p1 = Parameter(Type::Any(), 0);
     Node* feedback = UndefinedConstant();
@@ -276,7 +284,7 @@ TEST_F(JSCallReducerTest, MathClz32WithUnsigned32) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
 
   Node* p0 = Parameter(Type::Unsigned32(), 0);
   Node* feedback = UndefinedConstant();
@@ -295,7 +303,7 @@ TEST_F(JSCallReducerTest, MathClz32WithUnsigned32NoArg) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
 
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -316,7 +324,7 @@ TEST_F(JSCallReducerTest, MathImulWithUnsigned32) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Unsigned32(), 0);
   Node* p1 = Parameter(Type::Unsigned32(), 1);
   Node* feedback = UndefinedConstant();
@@ -338,7 +346,7 @@ TEST_F(JSCallReducerTest, MathMinWithNoArguments) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* feedback = UndefinedConstant();
   Node* call =
       graph()->NewNode(Call(0), jsfunction, UndefinedConstant(), feedback,
@@ -354,7 +362,7 @@ TEST_F(JSCallReducerTest, MathMinWithNumber) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -371,7 +379,7 @@ TEST_F(JSCallReducerTest, MathMinWithTwoArguments) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* p1 = Parameter(Type::Any(), 1);
   Node* feedback = UndefinedConstant();
@@ -394,7 +402,7 @@ TEST_F(JSCallReducerTest, MathMaxWithNoArguments) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* feedback = UndefinedConstant();
   Node* call =
       graph()->NewNode(Call(0), jsfunction, UndefinedConstant(), feedback,
@@ -410,7 +418,7 @@ TEST_F(JSCallReducerTest, MathMaxWithNumber) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -428,7 +436,7 @@ TEST_F(JSCallReducerTest, MathMaxWithTwoArguments) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* p1 = Parameter(Type::Any(), 1);
   Node* feedback = UndefinedConstant();
@@ -451,7 +459,7 @@ TEST_F(JSCallReducerTest, StringFromSingleCharCodeWithNumber) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -470,7 +478,7 @@ TEST_F(JSCallReducerTest, StringFromSingleCharCodeWithPlainPrimitive) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::PlainPrimitive(), 0);
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -492,7 +500,7 @@ TEST_F(JSCallReducerTest, NumberIsFinite) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -513,7 +521,7 @@ TEST_F(JSCallReducerTest, NumberIsIntegerWithNumber) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -534,7 +542,7 @@ TEST_F(JSCallReducerTest, NumberIsNaNWithNumber) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -555,7 +563,7 @@ TEST_F(JSCallReducerTest, NumberIsSafeIntegerWithIntegral32) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -576,7 +584,7 @@ TEST_F(JSCallReducerTest, GlobalIsFiniteWithNumber) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -597,7 +605,7 @@ TEST_F(JSCallReducerTest, GlobalIsNaN) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* feedback = UndefinedConstant();
   Node* call =
@@ -618,7 +626,7 @@ TEST_F(JSCallReducerTest, NumberParseInt) {
   Node* effect = graph()->start();
   Node* control = graph()->start();
   Node* context = UndefinedConstant();
-  Node* frame_state = graph()->start();
+  Node* frame_state = DummyFrameState();
   Node* p0 = Parameter(Type::Any(), 0);
   Node* p1 = Parameter(Type::Any(), 1);
   Node* feedback = UndefinedConstant();