[turbofan] Move SimplifiedOperatorBuilder into JSGraph.

This fixes the lifetime of nodes created by JSGlobalSpecialization that
contain a simplified operator. In the case where this reducer runs as
part of the inliner, the SimplifiedOperatorBuilder was instantiated with
the wrong zone. This led to use-after-free of simplified operators.

To avoid such situations in the future, we decided to move this operator
builder into the JSGraph and make the situation uniform with all other
operator builders.

R=bmeurer@chromium.org
BUG=chromium:543528
LOG=n

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

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

src/compiler/js-global-specialization.cc

@@ -30,8 +30,7 @@ JSGlobalSpecialization::JSGlobalSpecialization(
       jsgraph_(jsgraph),
       flags_(flags),
       global_object_(global_object),
-      dependencies_(dependencies),
-      simplified_(graph()->zone()) {}
+      dependencies_(dependencies) {}
 
 
 Reduction JSGlobalSpecialization::Reduce(Node* node) {
@@ -277,6 +276,11 @@ JSOperatorBuilder* JSGlobalSpecialization::javascript() const {
   return jsgraph()->javascript();
 }
 
+
+SimplifiedOperatorBuilder* JSGlobalSpecialization::simplified() const {
+  return jsgraph()->simplified();
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/js-global-specialization.h

@@ -62,7 +62,7 @@ class JSGlobalSpecialization final : public AdvancedReducer {
   Isolate* isolate() const;
   CommonOperatorBuilder* common() const;
   JSOperatorBuilder* javascript() const;
-  SimplifiedOperatorBuilder* simplified() { return &simplified_; }
+  SimplifiedOperatorBuilder* simplified() const;
   Flags flags() const { return flags_; }
   Handle<GlobalObject> global_object() const { return global_object_; }
   CompilationDependencies* dependencies() const { return dependencies_; }
@@ -71,7 +71,6 @@ class JSGlobalSpecialization final : public AdvancedReducer {
   Flags const flags_;
   Handle<GlobalObject> global_object_;
   CompilationDependencies* const dependencies_;
-  SimplifiedOperatorBuilder simplified_;
 
   DISALLOW_COPY_AND_ASSIGN(JSGlobalSpecialization);
 };

src/compiler/js-graph.h

@@ -17,19 +17,22 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
+class SimplifiedOperatorBuilder;
 class Typer;
 
 // Implements a facade on a Graph, enhancing the graph with JS-specific
-// notions, including a builder for for JS* operators, canonicalized global
+// notions, including various builders for operators, canonicalized global
 // constants, and various helper methods.
 class JSGraph : public ZoneObject {
  public:
   JSGraph(Isolate* isolate, Graph* graph, CommonOperatorBuilder* common,
-          JSOperatorBuilder* javascript, MachineOperatorBuilder* machine)
+          JSOperatorBuilder* javascript, SimplifiedOperatorBuilder* simplified,
+          MachineOperatorBuilder* machine)
       : isolate_(isolate),
         graph_(graph),
         common_(common),
         javascript_(javascript),
+        simplified_(simplified),
         machine_(machine),
         cache_(zone()) {
     for (int i = 0; i < kNumCachedNodes; i++) cached_nodes_[i] = nullptr;
@@ -117,8 +120,9 @@ class JSGraph : public ZoneObject {
   // Create a control node that serves as dependency for dead nodes.
   Node* Dead();
 
-  JSOperatorBuilder* javascript() const { return javascript_; }
   CommonOperatorBuilder* common() const { return common_; }
+  JSOperatorBuilder* javascript() const { return javascript_; }
+  SimplifiedOperatorBuilder* simplified() const { return simplified_; }
   MachineOperatorBuilder* machine() const { return machine_; }
   Graph* graph() const { return graph_; }
   Zone* zone() const { return graph()->zone(); }
@@ -147,6 +151,7 @@ class JSGraph : public ZoneObject {
   Graph* graph_;
   CommonOperatorBuilder* common_;
   JSOperatorBuilder* javascript_;
+  SimplifiedOperatorBuilder* simplified_;
   MachineOperatorBuilder* machine_;
   CommonNodeCache cache_;
   Node* cached_nodes_[kNumCachedNodes];

src/compiler/js-inlining.cc

@@ -351,7 +351,8 @@ Reduction JSInliner::ReduceJSCallFunction(Node* node,
 
   Graph graph(info.zone());
   JSGraph jsgraph(info.isolate(), &graph, jsgraph_->common(),
-                  jsgraph_->javascript(), jsgraph_->machine());
+                  jsgraph_->javascript(), jsgraph_->simplified(),
+                  jsgraph_->machine());
   AstGraphBuilder graph_builder(local_zone_, &info, &jsgraph);
   graph_builder.CreateGraph(false);
 

src/compiler/pipeline.cc

@@ -51,6 +51,7 @@
 #include "src/compiler/scheduler.h"
 #include "src/compiler/select-lowering.h"
 #include "src/compiler/simplified-lowering.h"
+#include "src/compiler/simplified-operator.h"
 #include "src/compiler/simplified-operator-reducer.h"
 #include "src/compiler/tail-call-optimization.h"
 #include "src/compiler/typer.h"
@@ -82,6 +83,7 @@ class PipelineData {
         graph_zone_(graph_zone_scope_.zone()),
         graph_(nullptr),
         loop_assignment_(nullptr),
+        simplified_(nullptr),
         machine_(nullptr),
         common_(nullptr),
         javascript_(nullptr),
@@ -98,13 +100,14 @@ class PipelineData {
     PhaseScope scope(pipeline_statistics, "init pipeline data");
     graph_ = new (graph_zone_) Graph(graph_zone_);
     source_positions_.Reset(new SourcePositionTable(graph_));
+    simplified_ = new (graph_zone_) SimplifiedOperatorBuilder(graph_zone_);
     machine_ = new (graph_zone_) MachineOperatorBuilder(
         graph_zone_, kMachPtr,
         InstructionSelector::SupportedMachineOperatorFlags());
     common_ = new (graph_zone_) CommonOperatorBuilder(graph_zone_);
     javascript_ = new (graph_zone_) JSOperatorBuilder(graph_zone_);
     jsgraph_ = new (graph_zone_)
-        JSGraph(isolate_, graph_, common_, javascript_, machine_);
+        JSGraph(isolate_, graph_, common_, javascript_, simplified_, machine_);
   }
 
   // For machine graph testing entry point.
@@ -122,6 +125,7 @@ class PipelineData {
         graph_(graph),
         source_positions_(new SourcePositionTable(graph_)),
         loop_assignment_(nullptr),
+        simplified_(nullptr),
         machine_(nullptr),
         common_(nullptr),
         javascript_(nullptr),
@@ -150,6 +154,7 @@ class PipelineData {
         graph_zone_(nullptr),
         graph_(nullptr),
         loop_assignment_(nullptr),
+        simplified_(nullptr),
         machine_(nullptr),
         common_(nullptr),
         javascript_(nullptr),
@@ -229,6 +234,7 @@ class PipelineData {
     graph_zone_ = nullptr;
     graph_ = nullptr;
     loop_assignment_ = nullptr;
+    simplified_ = nullptr;
     machine_ = nullptr;
     common_ = nullptr;
     javascript_ = nullptr;
@@ -296,6 +302,7 @@ class PipelineData {
   // TODO(dcarney): make this into a ZoneObject.
   base::SmartPointer<SourcePositionTable> source_positions_;
   LoopAssignmentAnalysis* loop_assignment_;
+  SimplifiedOperatorBuilder* simplified_;
   MachineOperatorBuilder* machine_;
   CommonOperatorBuilder* common_;
   JSOperatorBuilder* javascript_;

src/compiler/simplified-operator.h

@@ -125,7 +125,7 @@ ElementAccess const& ElementAccessOf(const Operator* op) WARN_UNUSED_RESULT;
 //   - Bool: a tagged pointer to either the canonical JS #false or
 //           the canonical JS #true object
 //   - Bit: an untagged integer 0 or 1, but word-sized
-class SimplifiedOperatorBuilder final {
+class SimplifiedOperatorBuilder final : public ZoneObject {
  public:
   explicit SimplifiedOperatorBuilder(Zone* zone);
 

test/cctest/compiler/test-changes-lowering.cc

@@ -34,7 +34,7 @@ class ChangesLoweringTester : public GraphBuilderTester<ReturnType> {
       : GraphBuilderTester<ReturnType>(p0),
         javascript(this->zone()),
         jsgraph(this->isolate(), this->graph(), this->common(), &javascript,
-                this->machine()),
+                nullptr, this->machine()),
         function(Handle<JSFunction>::null()) {}
 
   JSOperatorBuilder javascript;

test/cctest/compiler/test-js-constant-cache.cc

@@ -39,7 +39,7 @@ class JSConstantCacheTester : public HandleAndZoneScope,
   JSConstantCacheTester()
       : JSCacheTesterHelper(main_isolate(), main_zone()),
         JSGraph(main_isolate(), &main_graph_, &main_common_, &main_javascript_,
-                &main_machine_) {
+                nullptr, &main_machine_) {
     main_graph_.SetStart(main_graph_.NewNode(common()->Start(0)));
     main_graph_.SetEnd(
         main_graph_.NewNode(common()->End(1), main_graph_.start()));

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

@@ -23,7 +23,8 @@ class ContextSpecializationTester : public HandleAndZoneScope {
         javascript_(main_zone()),
         machine_(main_zone()),
         simplified_(main_zone()),
-        jsgraph_(main_isolate(), graph(), common(), &javascript_, &machine_),
+        jsgraph_(main_isolate(), graph(), common(), &javascript_, &simplified_,
+                 &machine_),
         reducer_(main_zone(), graph()),
         spec_(&reducer_, jsgraph(), MaybeHandle<Context>()) {}
 

test/cctest/compiler/test-js-typed-lowering.cc

@@ -85,7 +85,8 @@ class JSTypedLoweringTester : public HandleAndZoneScope {
   }
 
   Node* reduce(Node* node) {
-    JSGraph jsgraph(main_isolate(), &graph, &common, &javascript, &machine);
+    JSGraph jsgraph(main_isolate(), &graph, &common, &javascript, &simplified,
+                    &machine);
     // TODO(titzer): mock the GraphReducer here for better unit testing.
     GraphReducer graph_reducer(main_zone(), &graph);
     JSTypedLowering reducer(&graph_reducer, &jsgraph, main_zone());

test/cctest/compiler/test-loop-analysis.cc

@@ -39,7 +39,7 @@ class LoopFinderTester : HandleAndZoneScope {
       : isolate(main_isolate()),
         common(main_zone()),
         graph(main_zone()),
-        jsgraph(main_isolate(), &graph, &common, NULL, NULL),
+        jsgraph(main_isolate(), &graph, &common, nullptr, nullptr, nullptr),
         start(graph.NewNode(common.Start(1))),
         end(graph.NewNode(common.End(1), start)),
         p0(graph.NewNode(common.Parameter(0), start)),

test/cctest/compiler/test-machine-operator-reducer.cc

@@ -61,7 +61,7 @@ class ReducerTester : public HandleAndZoneScope {
         graph(main_zone()),
         javascript(main_zone()),
         typer(isolate, &graph),
-        jsgraph(isolate, &graph, &common, &javascript, &machine),
+        jsgraph(isolate, &graph, &common, &javascript, nullptr, &machine),
         maxuint32(Constant<int32_t>(kMaxUInt32)) {
     Node* s = graph.NewNode(common.Start(num_parameters));
     graph.SetStart(s);

test/cctest/compiler/test-osr.cc

@@ -48,7 +48,7 @@ class OsrDeconstructorTester : public HandleAndZoneScope {
       : isolate(main_isolate()),
         common(main_zone()),
         graph(main_zone()),
-        jsgraph(main_isolate(), &graph, &common, NULL, NULL),
+        jsgraph(main_isolate(), &graph, &common, nullptr, nullptr, nullptr),
         start(graph.NewNode(common.Start(1))),
         p0(graph.NewNode(common.Parameter(0), start)),
         end(graph.NewNode(common.End(1), start)),

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

@@ -27,7 +27,7 @@ class RepresentationChangerTester : public HandleAndZoneScope,
       : GraphAndBuilders(main_zone()),
         javascript_(main_zone()),
         jsgraph_(main_isolate(), main_graph_, &main_common_, &javascript_,
-                 &main_machine_),
+                 nullptr, &main_machine_),
         changer_(&jsgraph_, &main_simplified_, main_isolate()) {
     Node* s = graph()->NewNode(common()->Start(num_parameters));
     graph()->SetStart(s);

test/cctest/compiler/test-run-stubs.cc

@@ -35,7 +35,7 @@ TEST(RunOptimizedMathFloorStub) {
   CommonOperatorBuilder common(zone);
   JSOperatorBuilder javascript(zone);
   MachineOperatorBuilder machine(zone);
-  JSGraph js(isolate, &graph, &common, &javascript, &machine);
+  JSGraph js(isolate, &graph, &common, &javascript, nullptr, &machine);
 
   // FunctionTester (ab)uses a 2-argument function
   Node* start = graph.NewNode(common.Start(4));

test/cctest/compiler/test-simplified-lowering.cc

@@ -38,7 +38,7 @@ class SimplifiedLoweringTester : public GraphBuilderTester<ReturnType> {
         typer(this->isolate(), this->graph()),
         javascript(this->zone()),
         jsgraph(this->isolate(), this->graph(), this->common(), &javascript,
-                this->machine()),
+                nullptr, this->machine()),
         source_positions(jsgraph.graph()),
         lowering(&jsgraph, this->zone(), &source_positions) {}
 
@@ -676,7 +676,8 @@ class TestingGraph : public HandleAndZoneScope, public GraphAndBuilders {
       : GraphAndBuilders(main_zone()),
         typer(main_isolate(), graph()),
         javascript(main_zone()),
-        jsgraph(main_isolate(), graph(), common(), &javascript, machine()) {
+        jsgraph(main_isolate(), graph(), common(), &javascript, nullptr,
+                machine()) {
     start = graph()->NewNode(common()->Start(2));
     graph()->SetStart(start);
     ret =

test/unittests/compiler/bytecode-graph-builder-unittest.cc

@@ -51,8 +51,8 @@ Graph* BytecodeGraphBuilderTest::GetCompletedGraph() {
   CommonOperatorBuilder* common = new (zone()) CommonOperatorBuilder(zone());
   JSOperatorBuilder* javascript = new (zone()) JSOperatorBuilder(zone());
   Graph* graph = new (zone()) Graph(zone());
-  JSGraph* jsgraph =
-      new (zone()) JSGraph(isolate(), graph, common, javascript, machine);
+  JSGraph* jsgraph = new (zone())
+      JSGraph(isolate(), graph, common, javascript, nullptr, machine);
 
   Handle<String> name = factory()->NewStringFromStaticChars("test");
   Handle<String> script = factory()->NewStringFromStaticChars("test() {}");

test/unittests/compiler/change-lowering-unittest.cc

@@ -36,7 +36,8 @@ class ChangeLoweringTest : public TypedGraphTest {
   Reduction Reduce(Node* node) {
     MachineOperatorBuilder machine(zone(), WordRepresentation());
     JSOperatorBuilder javascript(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), &javascript, &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), &javascript, nullptr,
+                    &machine);
     ChangeLowering reducer(&jsgraph);
     return reducer.Reduce(node);
   }

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

@@ -26,7 +26,8 @@ class JSBuiltinReducerTest : public TypedGraphTest {
   Reduction Reduce(Node* node, MachineOperatorBuilder::Flags flags =
                                    MachineOperatorBuilder::Flag::kNoFlags) {
     MachineOperatorBuilder machine(zone(), kMachPtr, flags);
-    JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), javascript(), nullptr,
+                    &machine);
     // TODO(titzer): mock the GraphReducer here for better unit testing.
     GraphReducer graph_reducer(zone(), graph());
     JSBuiltinReducer reducer(&graph_reducer, &jsgraph);

test/unittests/compiler/js-context-relaxation-unittest.cc

@@ -20,7 +20,8 @@ class JSContextRelaxationTest : public GraphTest {
   Reduction Reduce(Node* node, MachineOperatorBuilder::Flags flags =
                                    MachineOperatorBuilder::kNoFlags) {
     MachineOperatorBuilder machine(zone(), kMachPtr, flags);
-    JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), javascript(), nullptr,
+                    &machine);
     // TODO(titzer): mock the GraphReducer here for better unit testing.
     GraphReducer graph_reducer(zone(), graph());
     JSContextRelaxation reducer;
@@ -29,7 +30,8 @@ class JSContextRelaxationTest : public GraphTest {
 
   Node* EmptyFrameState() {
     MachineOperatorBuilder machine(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), javascript(), nullptr,
+                    &machine);
     return jsgraph.EmptyFrameState();
   }
 

test/unittests/compiler/js-intrinsic-lowering-unittest.cc

@@ -32,7 +32,8 @@ class JSIntrinsicLoweringTest : public GraphTest {
   Reduction Reduce(Node* node, MachineOperatorBuilder::Flags flags =
                                    MachineOperatorBuilder::kNoFlags) {
     MachineOperatorBuilder machine(zone(), kMachPtr, flags);
-    JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), javascript(), nullptr,
+                    &machine);
     // TODO(titzer): mock the GraphReducer here for better unit testing.
     GraphReducer graph_reducer(zone(), graph());
     JSIntrinsicLowering reducer(&graph_reducer, &jsgraph,
@@ -42,7 +43,8 @@ class JSIntrinsicLoweringTest : public GraphTest {
 
   Node* EmptyFrameState() {
     MachineOperatorBuilder machine(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), javascript(), nullptr,
+                    &machine);
     return jsgraph.EmptyFrameState();
   }
 

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

@@ -40,7 +40,8 @@ class JSTypeFeedbackTest : public TypedGraphTest {
         isolate()->native_context()->global_object(), isolate());
 
     MachineOperatorBuilder machine(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), javascript(), nullptr,
+                    &machine);
     JSTypeFeedbackTable table(zone());
     // TODO(titzer): mock the GraphReducer here for better unit testing.
     GraphReducer graph_reducer(zone(), graph());
@@ -51,7 +52,8 @@ class JSTypeFeedbackTest : public TypedGraphTest {
 
   Node* EmptyFrameState() {
     MachineOperatorBuilder machine(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), javascript(), nullptr,
+                    &machine);
     return jsgraph.EmptyFrameState();
   }
 

test/unittests/compiler/js-typed-lowering-unittest.cc

@@ -80,7 +80,8 @@ class JSTypedLoweringTest : public TypedGraphTest {
  protected:
   Reduction Reduce(Node* node) {
     MachineOperatorBuilder machine(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), javascript(), nullptr,
+                    &machine);
     // TODO(titzer): mock the GraphReducer here for better unit testing.
     GraphReducer graph_reducer(zone(), graph());
     JSTypedLowering reducer(&graph_reducer, &jsgraph, zone());

test/unittests/compiler/liveness-analyzer-unittest.cc

@@ -25,7 +25,8 @@ class LivenessAnalysisTest : public GraphTest {
       : locals_count_(locals_count),
         machine_(zone(), kRepWord32),
         javascript_(zone()),
-        jsgraph_(isolate(), graph(), common(), &javascript_, &machine_),
+        jsgraph_(isolate(), graph(), common(), &javascript_, nullptr,
+                 &machine_),
         analyzer_(locals_count, zone()),
         empty_values_(graph()->NewNode(common()->StateValues(0), 0, nullptr)),
         next_checkpoint_id_(0),

test/unittests/compiler/machine-operator-reducer-unittest.cc

@@ -29,7 +29,8 @@ class MachineOperatorReducerTest : public TypedGraphTest {
  protected:
   Reduction Reduce(Node* node) {
     JSOperatorBuilder javascript(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), &javascript, &machine_);
+    JSGraph jsgraph(isolate(), graph(), common(), &javascript, nullptr,
+                    &machine_);
     MachineOperatorReducer reducer(&jsgraph);
     return reducer.Reduce(node);
   }

test/unittests/compiler/simplified-operator-reducer-unittest.cc

@@ -30,7 +30,8 @@ class SimplifiedOperatorReducerTest : public TypedGraphTest {
   Reduction Reduce(Node* node) {
     MachineOperatorBuilder machine(zone());
     JSOperatorBuilder javascript(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), &javascript, &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), &javascript, simplified(),
+                    &machine);
     SimplifiedOperatorReducer reducer(&jsgraph);
     return reducer.Reduce(node);
   }

test/unittests/compiler/state-values-utils-unittest.cc

@@ -95,7 +95,8 @@ TEST_F(StateValuesIteratorTest, TreeFromVector) {
   TRACED_FOREACH(int, count, sizes) {
     JSOperatorBuilder javascript(zone());
     MachineOperatorBuilder machine(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), &javascript, &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), &javascript, nullptr,
+                    &machine);
 
     // Generate the input vector.
     NodeVector inputs(zone());
@@ -124,7 +125,8 @@ TEST_F(StateValuesIteratorTest, BuildTreeIdentical) {
   TRACED_FOREACH(int, count, sizes) {
     JSOperatorBuilder javascript(zone());
     MachineOperatorBuilder machine(zone());
-    JSGraph jsgraph(isolate(), graph(), common(), &javascript, &machine);
+    JSGraph jsgraph(isolate(), graph(), common(), &javascript, nullptr,
+                    &machine);
 
     // Generate the input vector.
     NodeVector inputs(zone());