From 2e82ead865d088890bbfd14abfb22b8055b35394 Mon Sep 17 00:00:00 2001 From: Georg Schmid Date: Fri, 12 Jul 2019 09:47:16 +0200 Subject: [PATCH] [turbofan] Add optional runtime checks for range types This CL adds the --assert-types flag to d8, which is intended to insert additional runtime checks after typed nodes, verifying the validity of our typing rules. So far, only range types are checked. Thanks to Neil Patil for suggesting something similar. R=neis@chromium.org, tebbi@chromium.org Change-Id: I5eb2c482235ec8cd07ee802ca7c12c86c2d3dc40 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1678372 Commit-Queue: Georg Schmid Reviewed-by: Georg Neis Reviewed-by: Tobias Tebbi Cr-Commit-Position: refs/heads/master@{#62664} --- BUILD.gn | 2 + src/builtins/base.tq | 41 +++++++++++++++ src/compiler/add-type-assertions-reducer.cc | 51 +++++++++++++++++++ src/compiler/add-type-assertions-reducer.h | 45 ++++++++++++++++ src/compiler/effect-control-linearizer.cc | 28 ++++++++++ src/compiler/graph-assembler.cc | 3 ++ src/compiler/graph-assembler.h | 1 + src/compiler/opcodes.h | 1 + src/compiler/pipeline.cc | 19 +++++++ src/compiler/simplified-lowering.cc | 3 ++ src/compiler/simplified-operator.cc | 7 +++ src/compiler/simplified-operator.h | 3 ++ src/compiler/typer.cc | 2 + src/compiler/verifier.cc | 1 + src/flags/flag-definitions.h | 3 ++ .../compiler/constant-fold-add-static.js | 2 +- 16 files changed, 211 insertions(+), 1 deletion(-) create mode 100644 src/compiler/add-type-assertions-reducer.cc create mode 100644 src/compiler/add-type-assertions-reducer.h diff --git a/BUILD.gn b/BUILD.gn index 3e1a64e6f4..a12ffeb20f 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1877,6 +1877,8 @@ v8_compiler_sources = [ "src/compiler/state-values-utils.h", "src/compiler/store-store-elimination.cc", "src/compiler/store-store-elimination.h", + "src/compiler/add-type-assertions-reducer.cc", + "src/compiler/add-type-assertions-reducer.h", "src/compiler/type-cache.cc", "src/compiler/type-cache.h", "src/compiler/type-narrowing-reducer.cc", diff --git a/src/builtins/base.tq b/src/builtins/base.tq index 13f148c16b..006a3211e1 100644 --- a/src/builtins/base.tq +++ b/src/builtins/base.tq @@ -1682,6 +1682,7 @@ extern operator '==' macro Word32Equal(bool, bool): bool; extern operator '!=' macro Word32NotEqual(bool, bool): bool; extern operator '+' macro Float64Add(float64, float64): float64; +extern operator '-' macro Float64Sub(float64, float64): float64; extern operator '+' macro NumberAdd(Number, Number): Number; extern operator '-' macro NumberSub(Number, Number): Number; @@ -1734,6 +1735,8 @@ extern macro TaggedIsNotSmi(Object): bool; extern macro TaggedIsPositiveSmi(Object): bool; extern macro IsValidPositiveSmi(intptr): bool; +extern macro IsInteger(HeapNumber): bool; + extern macro HeapObjectToJSDataView(HeapObject): JSDataView labels CastError; extern macro HeapObjectToJSProxy(HeapObject): JSProxy @@ -3070,3 +3073,41 @@ macro VerifiedUnreachable(): never { StaticAssert(false); unreachable; } + +macro Float64IsSomeInfinity(value: float64): bool { + if (value == V8_INFINITY) { + return true; + } + return value == (Convert(0) - V8_INFINITY); +} + +@export +macro IsIntegerOrSomeInfinity(o: Object): bool { + typeswitch (o) { + case (Smi): { + return true; + } + case (hn: HeapNumber): { + if (Float64IsSomeInfinity(Convert(hn))) { + return true; + } + return IsInteger(hn); + } + case (Object): { + return false; + } + } +} + +builtin CheckNumberInRange(implicit context: Context)( + value: Number, min: Number, max: Number): Undefined { + if (IsIntegerOrSomeInfinity(value) && min <= value && value <= max) { + return Undefined; + } else { + Print('Range type assertion failed! (value/min/max)'); + Print(value); + Print(min); + Print(max); + unreachable; + } +} diff --git a/src/compiler/add-type-assertions-reducer.cc b/src/compiler/add-type-assertions-reducer.cc new file mode 100644 index 0000000000..59d2fe6820 --- /dev/null +++ b/src/compiler/add-type-assertions-reducer.cc @@ -0,0 +1,51 @@ +// Copyright 2019 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. + +#include "src/compiler/add-type-assertions-reducer.h" + +#include "src/compiler/node-properties.h" + +namespace v8 { +namespace internal { +namespace compiler { + +AddTypeAssertionsReducer::AddTypeAssertionsReducer(Editor* editor, + JSGraph* jsgraph, Zone* zone) + : AdvancedReducer(editor), + jsgraph_(jsgraph), + visited_(jsgraph->graph()->NodeCount(), zone) {} + +AddTypeAssertionsReducer::~AddTypeAssertionsReducer() = default; + +Reduction AddTypeAssertionsReducer::Reduce(Node* node) { + if (node->opcode() == IrOpcode::kAssertType || + node->opcode() == IrOpcode::kPhi || !NodeProperties::IsTyped(node) || + visited_.Get(node)) { + return NoChange(); + } + visited_.Set(node, true); + + Type type = NodeProperties::GetType(node); + if (!type.IsRange()) { + return NoChange(); + } + + Node* assertion = graph()->NewNode(simplified()->AssertType(type), node); + NodeProperties::SetType(assertion, type); + + for (Edge edge : node->use_edges()) { + Node* const user = edge.from(); + DCHECK(!user->IsDead()); + if (NodeProperties::IsValueEdge(edge) && user != assertion) { + edge.UpdateTo(assertion); + Revisit(user); + } + } + + return NoChange(); +} + +} // namespace compiler +} // namespace internal +} // namespace v8 diff --git a/src/compiler/add-type-assertions-reducer.h b/src/compiler/add-type-assertions-reducer.h new file mode 100644 index 0000000000..36add040e1 --- /dev/null +++ b/src/compiler/add-type-assertions-reducer.h @@ -0,0 +1,45 @@ +// Copyright 2019 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. + +#ifndef V8_COMPILER_ADD_TYPE_ASSERTIONS_REDUCER_H_ +#define V8_COMPILER_ADD_TYPE_ASSERTIONS_REDUCER_H_ + +#include "src/common/globals.h" +#include "src/compiler/graph-reducer.h" +#include "src/compiler/js-graph.h" +#include "src/compiler/node-aux-data.h" +#include "src/compiler/simplified-operator.h" + +namespace v8 { +namespace internal { + +namespace compiler { + +class V8_EXPORT_PRIVATE AddTypeAssertionsReducer final + : public NON_EXPORTED_BASE(AdvancedReducer) { + public: + AddTypeAssertionsReducer(Editor* editor, JSGraph* jsgraph, Zone* zone); + ~AddTypeAssertionsReducer() final; + + const char* reducer_name() const override { + return "AddTypeAssertionsReducer"; + } + + Reduction Reduce(Node* node) final; + + private: + JSGraph* const jsgraph_; + NodeAuxData visited_; + + Graph* graph() { return jsgraph_->graph(); } + SimplifiedOperatorBuilder* simplified() { return jsgraph_->simplified(); } + + DISALLOW_COPY_AND_ASSIGN(AddTypeAssertionsReducer); +}; + +} // namespace compiler +} // namespace internal +} // namespace v8 + +#endif // V8_COMPILER_ADD_TYPE_ASSERTIONS_REDUCER_H_ diff --git a/src/compiler/effect-control-linearizer.cc b/src/compiler/effect-control-linearizer.cc index b79ec0206e..f66c50fac0 100644 --- a/src/compiler/effect-control-linearizer.cc +++ b/src/compiler/effect-control-linearizer.cc @@ -194,6 +194,7 @@ class EffectControlLinearizer { void LowerTransitionAndStoreNumberElement(Node* node); void LowerTransitionAndStoreNonNumberElement(Node* node); void LowerRuntimeAbort(Node* node); + Node* LowerAssertType(Node* node); Node* LowerConvertReceiver(Node* node); Node* LowerDateNow(Node* node); @@ -1267,6 +1268,9 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node, case IrOpcode::kRuntimeAbort: LowerRuntimeAbort(node); break; + case IrOpcode::kAssertType: + result = LowerAssertType(node); + break; case IrOpcode::kConvertReceiver: result = LowerConvertReceiver(node); break; @@ -5347,6 +5351,30 @@ void EffectControlLinearizer::LowerRuntimeAbort(Node* node) { __ Int32Constant(1), __ NoContextConstant()); } +Node* EffectControlLinearizer::LowerAssertType(Node* node) { + DCHECK_EQ(node->opcode(), IrOpcode::kAssertType); + Type type = OpParameter(node->op()); + DCHECK(type.IsRange()); + auto range = type.AsRange(); + + Node* const input = node->InputAt(0); + Node* const min = __ NumberConstant(range->Min()); + Node* const max = __ NumberConstant(range->Max()); + + { + Callable const callable = + Builtins::CallableFor(isolate(), Builtins::kCheckNumberInRange); + Operator::Properties const properties = node->op()->properties(); + CallDescriptor::Flags const flags = CallDescriptor::kNoFlags; + auto call_descriptor = Linkage::GetStubCallDescriptor( + graph()->zone(), callable.descriptor(), + callable.descriptor().GetStackParameterCount(), flags, properties); + __ Call(call_descriptor, __ HeapConstant(callable.code()), input, min, max, + __ NoContextConstant()); + return input; + } +} + Node* EffectControlLinearizer::LowerConvertReceiver(Node* node) { ConvertReceiverMode const mode = ConvertReceiverModeOf(node->op()); Node* value = node->InputAt(0); diff --git a/src/compiler/graph-assembler.cc b/src/compiler/graph-assembler.cc index f1ebf540d1..50f29d968b 100644 --- a/src/compiler/graph-assembler.cc +++ b/src/compiler/graph-assembler.cc @@ -52,6 +52,9 @@ Node* GraphAssembler::HeapConstant(Handle object) { return jsgraph()->HeapConstant(object); } +Node* GraphAssembler::NumberConstant(double value) { + return jsgraph()->Constant(value); +} Node* GraphAssembler::ExternalConstant(ExternalReference ref) { return jsgraph()->ExternalConstant(ref); diff --git a/src/compiler/graph-assembler.h b/src/compiler/graph-assembler.h index 5498ccaddf..e2c0005d15 100644 --- a/src/compiler/graph-assembler.h +++ b/src/compiler/graph-assembler.h @@ -200,6 +200,7 @@ class GraphAssembler { Node* Float64Constant(double value); Node* Projection(int index, Node* value); Node* HeapConstant(Handle object); + Node* NumberConstant(double value); Node* CEntryStubConstant(int result_size); Node* ExternalConstant(ExternalReference ref); diff --git a/src/compiler/opcodes.h b/src/compiler/opcodes.h index 0a0d310386..536fcbddcf 100644 --- a/src/compiler/opcodes.h +++ b/src/compiler/opcodes.h @@ -471,6 +471,7 @@ V(FindOrderedHashMapEntryForInt32Key) \ V(PoisonIndex) \ V(RuntimeAbort) \ + V(AssertType) \ V(DateNow) #define SIMPLIFIED_SPECULATIVE_BIGINT_BINOP_LIST(V) V(SpeculativeBigIntAdd) diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc index f1f5bb614a..a7f9cb1c98 100644 --- a/src/compiler/pipeline.cc +++ b/src/compiler/pipeline.cc @@ -16,6 +16,7 @@ #include "src/codegen/compiler.h" #include "src/codegen/optimized-compilation-info.h" #include "src/codegen/register-configuration.h" +#include "src/compiler/add-type-assertions-reducer.h" #include "src/compiler/backend/code-generator.h" #include "src/compiler/backend/frame-elider.h" #include "src/compiler/backend/instruction-selector.h" @@ -1421,6 +1422,19 @@ struct EscapeAnalysisPhase { } }; +struct TypeAssertionsPhase { + static const char* phase_name() { return "V8.TFTypeAssertions"; } + + void Run(PipelineData* data, Zone* temp_zone) { + GraphReducer graph_reducer(temp_zone, data->graph(), + data->jsgraph()->Dead()); + AddTypeAssertionsReducer type_assertions(&graph_reducer, data->jsgraph(), + temp_zone); + AddReducer(data, &graph_reducer, &type_assertions); + graph_reducer.ReduceGraph(); + } +}; + struct SimplifiedLoweringPhase { static const char* phase_name() { return "V8.TFSimplifiedLowering"; } @@ -2230,6 +2244,11 @@ bool PipelineImpl::OptimizeGraph(Linkage* linkage) { RunPrintAndVerify(EscapeAnalysisPhase::phase_name()); } + if (FLAG_assert_types) { + Run(); + RunPrintAndVerify(TypeAssertionsPhase::phase_name()); + } + // Perform simplified lowering. This has to run w/o the Typer decorator, // because we cannot compute meaningful types anyways, and the computed types // might even conflict with the representation/truncation logic. diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc index 623bd7bd3f..cd683d9a55 100644 --- a/src/compiler/simplified-lowering.cc +++ b/src/compiler/simplified-lowering.cc @@ -3465,6 +3465,9 @@ class RepresentationSelector { return SetOutput(node, MachineRepresentation::kNone); case IrOpcode::kStaticAssert: return VisitUnop(node, UseInfo::Any(), MachineRepresentation::kTagged); + case IrOpcode::kAssertType: + return VisitUnop(node, UseInfo::AnyTagged(), + MachineRepresentation::kTagged); default: FATAL( "Representation inference: unsupported opcode %i (%s), node #%i\n.", diff --git a/src/compiler/simplified-operator.cc b/src/compiler/simplified-operator.cc index 537d26e8b6..4878ff6af4 100644 --- a/src/compiler/simplified-operator.cc +++ b/src/compiler/simplified-operator.cc @@ -1231,6 +1231,13 @@ const Operator* SimplifiedOperatorBuilder::BigIntAsUintN(int bits) { "BigIntAsUintN", 1, 0, 0, 1, 0, 0, bits); } +const Operator* SimplifiedOperatorBuilder::AssertType(Type type) { + DCHECK(type.IsRange()); + return new (zone()) Operator1(IrOpcode::kAssertType, + Operator::kNoThrow | Operator::kNoDeopt, + "AssertType", 1, 0, 0, 1, 0, 0, type); +} + const Operator* SimplifiedOperatorBuilder::CheckIf( DeoptimizeReason reason, const VectorSlotPair& feedback) { if (!feedback.IsValid()) { diff --git a/src/compiler/simplified-operator.h b/src/compiler/simplified-operator.h index 19531b5b42..2df2397ce3 100644 --- a/src/compiler/simplified-operator.h +++ b/src/compiler/simplified-operator.h @@ -888,6 +888,9 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final // Abort (for terminating execution on internal error). const Operator* RuntimeAbort(AbortReason reason); + // Abort if the value input does not inhabit the given type + const Operator* AssertType(Type type); + const Operator* DateNow(); private: diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index a4921553c7..6f8dc21a4e 100644 --- a/src/compiler/typer.cc +++ b/src/compiler/typer.cc @@ -2348,6 +2348,8 @@ Type Typer::Visitor::TypeFindOrderedHashMapEntryForInt32Key(Node* node) { Type Typer::Visitor::TypeRuntimeAbort(Node* node) { UNREACHABLE(); } +Type Typer::Visitor::TypeAssertType(Node* node) { UNREACHABLE(); } + // Heap constants. Type Typer::Visitor::TypeConstant(Handle value) { diff --git a/src/compiler/verifier.cc b/src/compiler/verifier.cc index 32abf21d93..755418eec4 100644 --- a/src/compiler/verifier.cc +++ b/src/compiler/verifier.cc @@ -1544,6 +1544,7 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) { case IrOpcode::kCheckedTaggedToCompressedSigned: case IrOpcode::kCheckedTaggedToCompressedPointer: case IrOpcode::kCheckedTruncateTaggedToWord32: + case IrOpcode::kAssertType: break; case IrOpcode::kCheckFloat64Hole: diff --git a/src/flags/flag-definitions.h b/src/flags/flag-definitions.h index 8d01e5be6f..59be3927b3 100644 --- a/src/flags/flag-definitions.h +++ b/src/flags/flag-definitions.h @@ -318,6 +318,9 @@ DEFINE_BOOL(future, FUTURE_BOOL, DEFINE_IMPLICATION(future, write_protect_code_memory) +DEFINE_BOOL(assert_types, false, + "generate runtime type assertions to test the typer") + // Flags for experimental implementation features. DEFINE_BOOL(allocation_site_pretenuring, true, "pretenure with allocation sites") diff --git a/test/mjsunit/compiler/constant-fold-add-static.js b/test/mjsunit/compiler/constant-fold-add-static.js index c4c6f71891..e824cabda6 100644 --- a/test/mjsunit/compiler/constant-fold-add-static.js +++ b/test/mjsunit/compiler/constant-fold-add-static.js @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// Flags: --allow-natives-syntax +// Flags: --allow-natives-syntax --no-assert-types // Check that constant-folding of arithmetic results in identical nodes. (function() {