[v8-dev] Fix lost arguments dropping in HLeaveInlined. (issue 10938016)

Tue, 18 Sep 2012 09:20:25 -0700 

Reviewers: Jakob,

Description:
Fix lost arguments dropping in HLeaveInlined.

This fixes HleaveInlined to correctly drop pushed arguments on all code
paths and addresses a corner case where the arguments stack height
mismatched at an OSR entry point.

[email protected]
BUG=chromium:150545
TEST=mjsunit/regress/regress-crbug-150545

Please review this at https://chromiumcodereview.appspot.com/10938016/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/arm/lithium-arm.cc
  M src/hydrogen-instructions.h
  M src/hydrogen.cc
  M src/ia32/lithium-ia32.cc
  M src/mips/lithium-mips.cc
  M src/x64/lithium-x64.cc
  A + test/mjsunit/regress/regress-crbug-150545.js


Index: src/arm/lithium-arm.cc
diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc
index b7a513cc0cc1a2d98caabbf6c57dc95cbc9720e6..1b313c153951ee6ec18b2a1e3bb3ef12be6c0e60 100644 
--- a/src/arm/lithium-arm.cc
+++ b/src/arm/lithium-arm.cc
@@ -2125,6 +2125,7 @@ LInstruction* LChunkBuilder::DoDeleteProperty(HDeleteProperty* instr) { 


 LInstruction* LChunkBuilder::DoOsrEntry(HOsrEntry* instr) {
+  ASSERT(argument_count_ == 0);
   allocator_->MarkAsOsrEntry();
   current_block_->last_environment()->set_ast_id(instr->ast_id());
   return AssignEnvironment(new(zone()) LOsrEntry);
@@ -2264,7 +2265,7 @@ LInstruction* LChunkBuilder::DoLeaveInlined(HLeaveInlined* instr) { 

   HEnvironment* env = current_block_->last_environment();

-  if (instr->arguments_pushed()) {
+  if (env->entry()->arguments_pushed()) {
     int argument_count = env->arguments_environment()->parameter_count();
     pop = new(zone()) LDrop(argument_count);
     argument_count_ -= argument_count;
Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index ebc86a97f18cb6c61a3f5c9d94c25ca59e2da4ee..c87bb553b11b96e2790b63d96e8d428f853abc6d 100644 
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -1472,21 +1472,13 @@ class HEnterInlined: public HTemplateInstruction<0> { 

 class HLeaveInlined: public HTemplateInstruction<0> {
  public:
-  explicit HLeaveInlined(bool arguments_pushed)
-      : arguments_pushed_(arguments_pushed) { }
+  HLeaveInlined() { }

   virtual Representation RequiredInputRepresentation(int index) {
     return Representation::None();
   }

-  bool arguments_pushed() {
-    return arguments_pushed_;
-  }
-
   DECLARE_CONCRETE_INSTRUCTION(LeaveInlined)
-
- private:
-  bool arguments_pushed_;
 };


Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index c30a88bf1224c711be718430530fa5d8292bbd67..44f9b8a64be51c79fd50bb24d569d57c5730ebc2 100644 
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -168,10 +168,9 @@ void HBasicBlock::Finish(HControlInstruction* end) {
 void HBasicBlock::Goto(HBasicBlock* block, FunctionState* state) {
   bool drop_extra = state != NULL &&
       state->inlining_kind() == DROP_EXTRA_ON_RETURN;
-  bool arguments_pushed = state != NULL && state->arguments_pushed();

   if (block->IsInlineReturnTarget()) {
-    AddInstruction(new(zone()) HLeaveInlined(arguments_pushed));
+    AddInstruction(new(zone()) HLeaveInlined());
     last_environment_ = last_environment()->DiscardInlined(drop_extra);
   }
@@ -185,11 +184,10 @@ void HBasicBlock::AddLeaveInlined(HValue* return_value, 
                                   FunctionState* state) {
   HBasicBlock* target = state->function_return();
   bool drop_extra = state->inlining_kind() == DROP_EXTRA_ON_RETURN;
-  bool arguments_pushed = state->arguments_pushed();

   ASSERT(target->IsInlineReturnTarget());
   ASSERT(return_value != NULL);
-  AddInstruction(new(zone()) HLeaveInlined(arguments_pushed));
+  AddInstruction(new(zone()) HLeaveInlined());
   last_environment_ = last_environment()->DiscardInlined(drop_extra);
   last_environment()->Push(return_value);
   AddSimulate(BailoutId::None());
Index: src/ia32/lithium-ia32.cc
diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc
index f843f3bc4eb4919ce96d41ca2c33cc970b79286d..38e7480d3df87c3222972c67dbb26ae1f590eb06 100644 
--- a/src/ia32/lithium-ia32.cc
+++ b/src/ia32/lithium-ia32.cc
@@ -2238,6 +2238,7 @@ LInstruction* LChunkBuilder::DoDeleteProperty(HDeleteProperty* instr) { 


 LInstruction* LChunkBuilder::DoOsrEntry(HOsrEntry* instr) {
+  ASSERT(argument_count_ == 0);
   allocator_->MarkAsOsrEntry();
   current_block_->last_environment()->set_ast_id(instr->ast_id());
   return AssignEnvironment(new(zone()) LOsrEntry);
@@ -2385,7 +2386,7 @@ LInstruction* LChunkBuilder::DoLeaveInlined(HLeaveInlined* instr) { 

   HEnvironment* env = current_block_->last_environment();

-  if (instr->arguments_pushed()) {
+  if (env->entry()->arguments_pushed()) {
     int argument_count = env->arguments_environment()->parameter_count();
     pop = new(zone()) LDrop(argument_count);
     argument_count_ -= argument_count;
Index: src/mips/lithium-mips.cc
diff --git a/src/mips/lithium-mips.cc b/src/mips/lithium-mips.cc
index 2ad64524826d5e685e7fb354a7df0f0d196743eb..e9edd48b9ef04381f5530f27459639ee328d44a0 100644 
--- a/src/mips/lithium-mips.cc
+++ b/src/mips/lithium-mips.cc
@@ -2069,6 +2069,7 @@ LInstruction* LChunkBuilder::DoDeleteProperty(HDeleteProperty* instr) { 


 LInstruction* LChunkBuilder::DoOsrEntry(HOsrEntry* instr) {
+  ASSERT(argument_count_ == 0);
   allocator_->MarkAsOsrEntry();
   current_block_->last_environment()->set_ast_id(instr->ast_id());
   return AssignEnvironment(new(zone()) LOsrEntry);
@@ -2208,7 +2209,7 @@ LInstruction* LChunkBuilder::DoLeaveInlined(HLeaveInlined* instr) { 

   HEnvironment* env = current_block_->last_environment();

-  if (instr->arguments_pushed()) {
+  if (env->entry()->arguments_pushed()) {
     int argument_count = env->arguments_environment()->parameter_count();
     pop = new(zone()) LDrop(argument_count);
     argument_count_ -= argument_count;
Index: src/x64/lithium-x64.cc
diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc
index 177e7349ade6e91ad0fa237d228b2755bb78eb8d..dd3054f2da620dd245db260826510a6d9b69d940 100644 
--- a/src/x64/lithium-x64.cc
+++ b/src/x64/lithium-x64.cc
@@ -2128,6 +2128,7 @@ LInstruction* LChunkBuilder::DoDeleteProperty(HDeleteProperty* instr) { 


 LInstruction* LChunkBuilder::DoOsrEntry(HOsrEntry* instr) {
+  ASSERT(argument_count_ == 0);
   allocator_->MarkAsOsrEntry();
   current_block_->last_environment()->set_ast_id(instr->ast_id());
   return AssignEnvironment(new(zone()) LOsrEntry);
@@ -2267,7 +2268,7 @@ LInstruction* LChunkBuilder::DoLeaveInlined(HLeaveInlined* instr) { 

   HEnvironment* env = current_block_->last_environment();

-  if (instr->arguments_pushed()) {
+  if (env->entry()->arguments_pushed()) {
     int argument_count = env->arguments_environment()->parameter_count();
     pop = new(zone()) LDrop(argument_count);
     argument_count_ -= argument_count;
Index: test/mjsunit/regress/regress-crbug-150545.js
diff --git a/test/mjsunit/regress/regress-crbug-135066.js b/test/mjsunit/regress/regress-crbug-150545.js 
similarity index 74%
copy from test/mjsunit/regress/regress-crbug-135066.js
copy to test/mjsunit/regress/regress-crbug-150545.js
index 1aeca8b1a32d678ba7274c60230a77fdda97f6aa..a748af14cd0247836783e221d5d1bb9bdf1ad080 100644 
--- a/test/mjsunit/regress/regress-crbug-135066.js
+++ b/test/mjsunit/regress/regress-crbug-150545.js
@@ -25,29 +25,28 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-// Filler long enough to trigger lazy parsing.
-var filler = "//" + new Array(1024).join('x');
+// Test that we do not generate OSR entry points that have an arguments
+// stack height different from zero. The OSR machinery cannot generate
+// frames for that.

-// Test strict eval in global context.
-eval(
-  "'use strict';" +
-  "var x = 23;" +
-  "var f = function bozo1() {" +
-  "  return x;" +
-  "};" +
-  "assertSame(23, f());" +
-  filler
-);
-
-// Test default eval in strict context.
 (function() {
   "use strict";
-  eval(
-    "var y = 42;" +
-    "var g = function bozo2() {" +
-    "  return y;" +
-    "};" +
-    "assertSame(42, g());" +
-    filler
-  );
+
+  var instantReturn = false;
+  function inner() {
+    if (instantReturn) return;
+    assertSame(3, arguments.length);
+    assertSame(1, arguments[0]);
+    assertSame(2, arguments[1]);
+    assertSame(3, arguments[2]);
+  }
+
+  function outer(iterations) {
+    inner(1,2,3);
+    while (--iterations > 0);
+    assertSame(0, iterations);
+  }
+
+  // This function should be optimized via OSR.
+  outer(100000);
 })();


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to