[ubsan] Fix HeapObjectMatcher to avoid invalid casts.

BUG=v8:3809
LOG=n
R=svenpanne@chromium.org

Review URL: https://codereview.chromium.org/1196623002

Cr-Commit-Position: refs/heads/master@{#29147}

src/compiler/ast-graph-builder.cc

@@ -3590,7 +3590,7 @@ Node* AstGraphBuilder::BuildToBoolean(Node* input) {
     case IrOpcode::kNumberConstant:
       return jsgraph_->BooleanConstant(!NumberMatcher(input).Is(0));
     case IrOpcode::kHeapConstant: {
-      Handle<Object> object = HeapObjectMatcher<Object>(input).Value().handle();
+      Handle<HeapObject> object = HeapObjectMatcher(input).Value().handle();
       return jsgraph_->BooleanConstant(object->BooleanValue());
     }
     case IrOpcode::kJSEqual:

src/compiler/common-operator-reducer.cc

@@ -31,7 +31,7 @@ Decision DecideCondition(Node* const cond) {
       return mcond.Value() ? Decision::kTrue : Decision::kFalse;
     }
     case IrOpcode::kHeapConstant: {
-      HeapObjectMatcher<HeapObject> mcond(cond);
+      HeapObjectMatcher mcond(cond);
       return mcond.Value().handle()->BooleanValue() ? Decision::kTrue
                                                     : Decision::kFalse;
     }

src/compiler/js-builtin-reducer.cc

@@ -24,7 +24,7 @@ class JSCallReduction {
   // constant callee being a well-known builtin with a BuiltinFunctionId.
   bool HasBuiltinFunctionId() {
     if (node_->opcode() != IrOpcode::kJSCallFunction) return false;
-    HeapObjectMatcher<Object> m(NodeProperties::GetValueInput(node_, 0));
+    HeapObjectMatcher m(NodeProperties::GetValueInput(node_, 0));
     if (!m.HasValue() || !m.Value().handle()->IsJSFunction()) return false;
     Handle<JSFunction> function = Handle<JSFunction>::cast(m.Value().handle());
     return function->shared()->HasBuiltinFunctionId();
@@ -33,7 +33,7 @@ class JSCallReduction {
   // Retrieves the BuiltinFunctionId as described above.
   BuiltinFunctionId GetBuiltinFunctionId() {
     DCHECK_EQ(IrOpcode::kJSCallFunction, node_->opcode());
-    HeapObjectMatcher<Object> m(NodeProperties::GetValueInput(node_, 0));
+    HeapObjectMatcher m(NodeProperties::GetValueInput(node_, 0));
     Handle<JSFunction> function = Handle<JSFunction>::cast(m.Value().handle());
     return function->shared()->builtin_function_id();
   }

src/compiler/js-context-specialization.cc

@@ -5,9 +5,11 @@
 #include "src/compiler/js-context-specialization.h"
 
 #include "src/compiler/common-operator.h"
+#include "src/compiler/js-graph.h"
 #include "src/compiler/js-operator.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/node-properties.h"
+#include "src/contexts.h"
 
 namespace v8 {
 namespace internal {
@@ -27,7 +29,7 @@ Reduction JSContextSpecializer::Reduce(Node* node) {
 Reduction JSContextSpecializer::ReduceJSLoadContext(Node* node) {
   DCHECK_EQ(IrOpcode::kJSLoadContext, node->opcode());
 
-  HeapObjectMatcher<Context> m(NodeProperties::GetValueInput(node, 0));
+  HeapObjectMatcher m(NodeProperties::GetValueInput(node, 0));
   // If the context is not constant, no reduction can occur.
   if (!m.HasValue()) {
     return NoChange();
@@ -36,9 +38,9 @@ Reduction JSContextSpecializer::ReduceJSLoadContext(Node* node) {
   const ContextAccess& access = ContextAccessOf(node->op());
 
   // Find the right parent context.
-  Context* context = *m.Value().handle();
+  Handle<Context> context = Handle<Context>::cast(m.Value().handle());
   for (size_t i = access.depth(); i > 0; --i) {
-    context = context->previous();
+    context = handle(context->previous(), isolate());
   }
 
   // If the access itself is mutable, only fold-in the parent.
@@ -50,13 +52,11 @@ Reduction JSContextSpecializer::ReduceJSLoadContext(Node* node) {
     const Operator* op = jsgraph_->javascript()->LoadContext(
         0, access.index(), access.immutable());
     node->set_op(op);
-    Handle<Object> context_handle =
-        Handle<Object>(context, jsgraph_->isolate());
-    node->ReplaceInput(0, jsgraph_->Constant(context_handle));
+    node->ReplaceInput(0, jsgraph_->Constant(context));
     return Changed(node);
   }
-  Handle<Object> value = Handle<Object>(
-      context->get(static_cast<int>(access.index())), jsgraph_->isolate());
+  Handle<Object> value =
+      handle(context->get(static_cast<int>(access.index())), isolate());
 
   // Even though the context slot is immutable, the context might have escaped
   // before the function to which it belongs has initialized the slot.
@@ -78,7 +78,7 @@ Reduction JSContextSpecializer::ReduceJSLoadContext(Node* node) {
 Reduction JSContextSpecializer::ReduceJSStoreContext(Node* node) {
   DCHECK_EQ(IrOpcode::kJSStoreContext, node->opcode());
 
-  HeapObjectMatcher<Context> m(NodeProperties::GetValueInput(node, 0));
+  HeapObjectMatcher m(NodeProperties::GetValueInput(node, 0));
   // If the context is not constant, no reduction can occur.
   if (!m.HasValue()) {
     return NoChange();
@@ -92,20 +92,24 @@ Reduction JSContextSpecializer::ReduceJSStoreContext(Node* node) {
   }
 
   // Find the right parent context.
-  Context* context = *m.Value().handle();
+  Handle<Context> context = Handle<Context>::cast(m.Value().handle());
   for (size_t i = access.depth(); i > 0; --i) {
-    context = context->previous();
+    context = handle(context->previous(), isolate());
   }
 
-  const Operator* op = jsgraph_->javascript()->StoreContext(0, access.index());
-  node->set_op(op);
-  Handle<Object> new_context_handle =
-      Handle<Object>(context, jsgraph_->isolate());
-  node->ReplaceInput(0, jsgraph_->Constant(new_context_handle));
-
+  node->set_op(javascript()->StoreContext(0, access.index()));
+  node->ReplaceInput(0, jsgraph_->Constant(context));
   return Changed(node);
 }
 
+
+Isolate* JSContextSpecializer::isolate() const { return jsgraph()->isolate(); }
+
+
+JSOperatorBuilder* JSContextSpecializer::javascript() const {
+  return jsgraph()->javascript();
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/js-context-specialization.h

@@ -6,13 +6,16 @@
 #define V8_COMPILER_JS_CONTEXT_SPECIALIZATION_H_
 
 #include "src/compiler/graph-reducer.h"
-#include "src/compiler/js-graph.h"
-#include "src/contexts.h"
 
 namespace v8 {
 namespace internal {
 namespace compiler {
 
+// Forward declarations.
+class JSGraph;
+class JSOperatorBuilder;
+
+
 // Specializes a given JSGraph to a given context, potentially constant folding
 // some {LoadContext} nodes or strength reducing some {StoreContext} nodes.
 class JSContextSpecializer : public AdvancedReducer {
@@ -27,10 +30,17 @@ class JSContextSpecializer : public AdvancedReducer {
   Reduction ReduceJSStoreContext(Node* node);
 
  private:
-  JSGraph* jsgraph_;
+  Isolate* isolate() const;
+  JSOperatorBuilder* javascript() const;
+  JSGraph* jsgraph() const { return jsgraph_; }
+
+  JSGraph* const jsgraph_;
+
+  DISALLOW_COPY_AND_ASSIGN(JSContextSpecializer);
 };
-}
-}
-}  // namespace v8::internal::compiler
+
+}  // namespace compiler
+}  // namespace internal
+}  // namespace v8
 
 #endif  // V8_COMPILER_JS_CONTEXT_SPECIALIZATION_H_

src/compiler/js-inlining.cc

@@ -238,11 +238,12 @@ Reduction JSInliner::Reduce(Node* node) {
   if (node->opcode() != IrOpcode::kJSCallFunction) return NoChange();
 
   JSCallFunctionAccessor call(node);
-  HeapObjectMatcher<JSFunction> match(call.jsfunction());
+  HeapObjectMatcher match(call.jsfunction());
   if (!match.HasValue()) return NoChange();
 
-  Handle<JSFunction> function = match.Value().handle();
-  if (!function->IsJSFunction()) return NoChange();
+  if (!match.Value().handle()->IsJSFunction()) return NoChange();
+  Handle<JSFunction> function =
+      Handle<JSFunction>::cast(match.Value().handle());
   if (mode_ == kRestrictedInlining && !function->shared()->force_inline()) {
     return NoChange();
   }

src/compiler/js-intrinsic-lowering.cc

@@ -179,11 +179,12 @@ Reduction JSIntrinsicLowering::ReduceHeapObjectGetMap(Node* node) {
 
 Reduction JSIntrinsicLowering::ReduceIncrementStatsCounter(Node* node) {
   if (!FLAG_native_code_counters) return ChangeToUndefined(node);
-  HeapObjectMatcher<String> m(NodeProperties::GetValueInput(node, 0));
+  HeapObjectMatcher m(NodeProperties::GetValueInput(node, 0));
   if (!m.HasValue() || !m.Value().handle()->IsString()) {
     return ChangeToUndefined(node);
   }
-  SmartArrayPointer<char> name = m.Value().handle()->ToCString();
+  SmartArrayPointer<char> name =
+      Handle<String>::cast(m.Value().handle())->ToCString();
   StatsCounter counter(jsgraph()->isolate(), name.get());
   if (!counter.Enabled()) return ChangeToUndefined(node);
 

src/compiler/js-typed-lowering.cc

@@ -824,7 +824,7 @@ Reduction JSTypedLowering::ReduceJSLoadProperty(Node* node) {
   Node* key = NodeProperties::GetValueInput(node, 1);
   Node* base = NodeProperties::GetValueInput(node, 0);
   Type* key_type = NodeProperties::GetBounds(key).upper;
-  HeapObjectMatcher<Object> mbase(base);
+  HeapObjectMatcher mbase(base);
   if (mbase.HasValue() && mbase.Value().handle()->IsJSTypedArray()) {
     Handle<JSTypedArray> const array =
         Handle<JSTypedArray>::cast(mbase.Value().handle());
@@ -871,7 +871,7 @@ Reduction JSTypedLowering::ReduceJSStoreProperty(Node* node) {
   Node* value = NodeProperties::GetValueInput(node, 2);
   Type* key_type = NodeProperties::GetBounds(key).upper;
   Type* value_type = NodeProperties::GetBounds(value).upper;
-  HeapObjectMatcher<Object> mbase(base);
+  HeapObjectMatcher mbase(base);
   if (mbase.HasValue() && mbase.Value().handle()->IsJSTypedArray()) {
     Handle<JSTypedArray> const array =
         Handle<JSTypedArray>::cast(mbase.Value().handle());
@@ -1132,8 +1132,8 @@ Reduction JSTypedLowering::ReduceJSCreateClosure(Node* node) {
 
 Reduction JSTypedLowering::ReduceJSCreateLiteralArray(Node* node) {
   DCHECK_EQ(IrOpcode::kJSCreateLiteralArray, node->opcode());
-  HeapObjectMatcher<FixedArray> mconst(NodeProperties::GetValueInput(node, 2));
-  int length = mconst.Value().handle()->length();
+  HeapObjectMatcher mconst(NodeProperties::GetValueInput(node, 2));
+  int length = Handle<FixedArray>::cast(mconst.Value().handle())->length();
   int flags = OpParameter<int>(node->op());
 
   // Use the FastCloneShallowArrayStub only for shallow boilerplates up to the
@@ -1162,9 +1162,9 @@ Reduction JSTypedLowering::ReduceJSCreateLiteralArray(Node* node) {
 
 Reduction JSTypedLowering::ReduceJSCreateLiteralObject(Node* node) {
   DCHECK_EQ(IrOpcode::kJSCreateLiteralObject, node->opcode());
-  HeapObjectMatcher<FixedArray> mconst(NodeProperties::GetValueInput(node, 2));
+  HeapObjectMatcher mconst(NodeProperties::GetValueInput(node, 2));
   // Constants are pairs, see ObjectLiteral::properties_count().
-  int length = mconst.Value().handle()->length() / 2;
+  int length = Handle<FixedArray>::cast(mconst.Value().handle())->length() / 2;
   int flags = OpParameter<int>(node->op());
 
   // Use the FastCloneShallowObjectStub only for shallow boilerplates without
@@ -1227,9 +1227,10 @@ Reduction JSTypedLowering::ReduceJSCreateWithContext(Node* node) {
 Reduction JSTypedLowering::ReduceJSCreateBlockContext(Node* node) {
   DCHECK_EQ(IrOpcode::kJSCreateBlockContext, node->opcode());
   Node* const input = NodeProperties::GetValueInput(node, 0);
-  HeapObjectMatcher<ScopeInfo> minput(input);
+  HeapObjectMatcher minput(input);
   DCHECK(minput.HasValue());  // TODO(mstarzinger): Make ScopeInfo static.
-  int context_length = minput.Value().handle()->ContextLength();
+  int context_length =
+      Handle<ScopeInfo>::cast(minput.Value().handle())->ContextLength();
   if (FLAG_turbo_allocate && context_length < kBlockContextAllocationLimit) {
     // JSCreateBlockContext(s:scope[length < limit], f)
     Node* const effect = NodeProperties::GetEffectInput(node);

src/compiler/node-matchers.h

@@ -152,11 +152,10 @@ typedef FloatMatcher<double, IrOpcode::kNumberConstant> NumberMatcher;
 
 
 // A pattern matcher for heap object constants.
-template <typename T>
 struct HeapObjectMatcher final
-    : public ValueMatcher<Unique<T>, IrOpcode::kHeapConstant> {
+    : public ValueMatcher<Unique<HeapObject>, IrOpcode::kHeapConstant> {
   explicit HeapObjectMatcher(Node* node)
-      : ValueMatcher<Unique<T>, IrOpcode::kHeapConstant>(node) {}
+      : ValueMatcher<Unique<HeapObject>, IrOpcode::kHeapConstant>(node) {}
 };
 
 

test/cctest/compiler/test-js-context-specialization.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "src/compiler/js-context-specialization.h"
+#include "src/compiler/js-graph.h"
 #include "src/compiler/js-operator.h"
 #include "src/compiler/node-matchers.h"
 #include "src/compiler/node-properties.h"
@@ -91,7 +92,7 @@ TEST(ReduceJSLoadContext) {
     CHECK(r.Changed());
     Node* new_context_input = NodeProperties::GetValueInput(r.replacement(), 0);
     CHECK_EQ(IrOpcode::kHeapConstant, new_context_input->opcode());
-    HeapObjectMatcher<Context> match(new_context_input);
+    HeapObjectMatcher match(new_context_input);
     CHECK_EQ(*native, *match.Value().handle());
     ContextAccess access = OpParameter<ContextAccess>(r.replacement());
     CHECK_EQ(Context::GLOBAL_EVAL_FUN_INDEX, static_cast<int>(access.index()));
@@ -107,7 +108,7 @@ TEST(ReduceJSLoadContext) {
     CHECK(r.Changed());
     CHECK(r.replacement() != load);
 
-    HeapObjectMatcher<Object> match(r.replacement());
+    HeapObjectMatcher match(r.replacement());
     CHECK(match.HasValue());
     CHECK_EQ(*expected, *match.Value().handle());
   }
@@ -170,7 +171,7 @@ TEST(ReduceJSStoreContext) {
     CHECK(r.Changed());
     Node* new_context_input = NodeProperties::GetValueInput(r.replacement(), 0);
     CHECK_EQ(IrOpcode::kHeapConstant, new_context_input->opcode());
-    HeapObjectMatcher<Context> match(new_context_input);
+    HeapObjectMatcher match(new_context_input);
     CHECK_EQ(*native, *match.Value().handle());
     ContextAccess access = OpParameter<ContextAccess>(r.replacement());
     CHECK_EQ(Context::GLOBAL_EVAL_FUN_INDEX, static_cast<int>(access.index()));
@@ -245,7 +246,7 @@ TEST(SpecializeToContext) {
     CHECK_EQ(other_load, other_use->InputAt(0));
 
     Node* replacement = value_use->InputAt(0);
-    HeapObjectMatcher<Object> match(replacement);
+    HeapObjectMatcher match(replacement);
     CHECK(match.HasValue());
     CHECK_EQ(*expected, *match.Value().handle());
   }

test/cctest/compiler/test-representation-change.cc

@@ -69,7 +69,7 @@ class RepresentationChangerTester : public HandleAndZoneScope,
   }
 
   void CheckHeapConstant(Node* n, HeapObject* expected) {
-    HeapObjectMatcher<HeapObject> m(n);
+    HeapObjectMatcher m(n);
     CHECK(m.HasValue());
     CHECK_EQ(expected, *m.Value().handle());
   }

test/unittests/compiler/js-type-feedback-unittest.cc

@@ -275,7 +275,7 @@ TEST_F(JSTypeFeedbackTest, JSLoadNamedGlobalPropertyCellSmiWithDeoptimization) {
       access, CaptureEq(&cell_capture), graph()->start(), graph()->start());
   EXPECT_THAT(r.replacement(), load_field_match);
 
-  HeapObjectMatcher<PropertyCell> cell(cell_capture.value());
+  HeapObjectMatcher cell(cell_capture.value());
   EXPECT_TRUE(cell.HasValue());
   EXPECT_TRUE(cell.Value().handle()->IsPropertyCell());
 
@@ -327,7 +327,7 @@ TEST_F(JSTypeFeedbackTest,
       access, CaptureEq(&cell_capture), graph()->start(), graph()->start());
   EXPECT_THAT(r.replacement(), load_field_match);
 
-  HeapObjectMatcher<PropertyCell> cell(cell_capture.value());
+  HeapObjectMatcher cell(cell_capture.value());
   EXPECT_TRUE(cell.HasValue());
   EXPECT_TRUE(cell.Value().handle()->IsPropertyCell());