From 70e302eeddfe8b078b257975b0ad7b9748974f00 Mon Sep 17 00:00:00 2001 From: danno Date: Thu, 2 Jun 2016 14:24:24 -0700 Subject: [PATCH] [turbofan] Fix assert caused by bogus merging of out-of-scope CodeAssembler variables Previously, CodeAssembler Variables declared in an explicit C++ scope would continue to be merged into future labels beyond that scope, causing asserts. This CL ensures that Variables are properly ignored when they go out of scope. Review-Url: https://codereview.chromium.org/2035683002 Cr-Commit-Position: refs/heads/master@{#36690} --- src/compiler/code-assembler.cc | 6 +++-- src/compiler/code-assembler.h | 4 +++- test/cctest/test-code-stub-assembler.cc | 29 +++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/compiler/code-assembler.cc b/src/compiler/code-assembler.cc index b9d11af487..9bc40258c8 100644 --- a/src/compiler/code-assembler.cc +++ b/src/compiler/code-assembler.cc @@ -600,10 +600,12 @@ class CodeAssembler::Variable::Impl : public ZoneObject { CodeAssembler::Variable::Variable(CodeAssembler* assembler, MachineRepresentation rep) - : impl_(new (assembler->zone()) Impl(rep)) { - assembler->variables_.push_back(impl_); + : impl_(new (assembler->zone()) Impl(rep)), assembler_(assembler) { + assembler->variables_.insert(impl_); } +CodeAssembler::Variable::~Variable() { assembler_->variables_.erase(impl_); } + void CodeAssembler::Variable::Bind(Node* value) { impl_->value_ = value; } Node* CodeAssembler::Variable::value() const { diff --git a/src/compiler/code-assembler.h b/src/compiler/code-assembler.h index ab04e7e58e..2a00b040c7 100644 --- a/src/compiler/code-assembler.h +++ b/src/compiler/code-assembler.h @@ -166,6 +166,7 @@ class CodeAssembler { class Variable { public: explicit Variable(CodeAssembler* assembler, MachineRepresentation rep); + ~Variable(); void Bind(Node* value); Node* value() const; MachineRepresentation rep() const; @@ -175,6 +176,7 @@ class CodeAssembler { friend class CodeAssembler; class Impl; Impl* impl_; + CodeAssembler* assembler_; }; enum AllocationFlag : uint8_t { @@ -362,7 +364,7 @@ class CodeAssembler { Code::Flags flags_; const char* name_; bool code_generated_; - ZoneVector variables_; + ZoneSet variables_; DISALLOW_COPY_AND_ASSIGN(CodeAssembler); }; diff --git a/test/cctest/test-code-stub-assembler.cc b/test/cctest/test-code-stub-assembler.cc index 01c99d43aa..18d51ff0f0 100644 --- a/test/cctest/test-code-stub-assembler.cc +++ b/test/cctest/test-code-stub-assembler.cc @@ -1090,5 +1090,34 @@ TEST(DeferredCodePhiHints) { CHECK(!m.GenerateCode().is_null()); } +TEST(TestOutOfScopeVariable) { + typedef CodeStubAssembler::Label Label; + typedef CodeStubAssembler::Variable Variable; + Isolate* isolate(CcTest::InitIsolateOnce()); + VoidDescriptor descriptor(isolate); + CodeStubAssemblerTester m(isolate, descriptor); + Label block1(&m); + Label block2(&m); + Label block3(&m); + Label block4(&m); + m.Branch(m.WordEqual(m.Parameter(0), m.IntPtrConstant(0)), &block1, &block4); + m.Bind(&block4); + { + Variable var_object(&m, MachineRepresentation::kTagged); + m.Branch(m.WordEqual(m.Parameter(0), m.IntPtrConstant(0)), &block2, + &block3); + + m.Bind(&block2); + var_object.Bind(m.IntPtrConstant(55)); + m.Goto(&block1); + + m.Bind(&block3); + var_object.Bind(m.IntPtrConstant(66)); + m.Goto(&block1); + } + m.Bind(&block1); + CHECK(!m.GenerateCode().is_null()); +} + } // namespace internal } // namespace v8