[v8-dev] Use baseline code to compute message locations. (issue 1331603002 by [email protected])

Tue, 08 Sep 2015 06:39:17 -0700 

Reviewers: Benedikt Meurer,

Description:
Use baseline code to compute message locations.

This switches Isolate::ComputeLocation to use baseline code when
computing message locations. This unifies locations between optimized
and non-optimized code by always going through the FrameSummary for
location computation.

[email protected]
TEST=message/regress/regress-4266
BUG=v8:4266
LOG=n

Please review this at https://codereview.chromium.org/1331603002/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+54, -28 lines):
  M src/ast-numbering.cc
  M src/compiler/ast-graph-builder.cc
  M src/compiler/linkage.cc
  M src/full-codegen/arm/full-codegen-arm.cc
  M src/full-codegen/arm64/full-codegen-arm64.cc
  M src/full-codegen/ia32/full-codegen-ia32.cc
  M src/full-codegen/mips/full-codegen-mips.cc
  M src/full-codegen/mips64/full-codegen-mips64.cc
  M src/full-codegen/ppc/full-codegen-ppc.cc
  M src/full-codegen/x64/full-codegen-x64.cc
  M src/full-codegen/x87/full-codegen-x87.cc
  M src/isolate.cc
  M src/scopes.h
  M src/scopes.cc
  A + test/message/regress/regress-4266.js
  A + test/message/regress/regress-4266.out
  A test/mjsunit/regress/regress-4266.js


Index: src/ast-numbering.cc
diff --git a/src/ast-numbering.cc b/src/ast-numbering.cc
index 7338cc6724219678852d5a90c40155b5a62e53ad..9d048cd9c0a5773da6d426fa7bf70201c2c55ff7 100644 
--- a/src/ast-numbering.cc
+++ b/src/ast-numbering.cc
@@ -558,7 +558,7 @@ bool AstNumberingVisitor::Renumber(FunctionLiteral* node) { 
   Scope* scope = node->scope();

   if (scope->HasIllegalRedeclaration()) {
-    scope->VisitIllegalRedeclaration(this);
+    Visit(scope->GetIllegalRedeclaration());
     DisableOptimization(kFunctionWithIllegalRedeclaration);
     return Finish(node);
   }
Index: src/compiler/ast-graph-builder.cc
diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 8fee623d1b39e7d20b599518c54e63e45c9b6f35..5c67797b020c70d025da5fe2cbbbcbfba4382364 100644 
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -581,8 +581,7 @@ void AstGraphBuilder::CreateGraphBody(bool stack_check) { 

   // Visit illegal re-declaration and bail out if it exists.
   if (scope->HasIllegalRedeclaration()) {
-    AstEffectContext for_effect(this);
-    scope->VisitIllegalRedeclaration(this);
+    VisitForEffect(scope->GetIllegalRedeclaration());
     return;
   }

Index: src/compiler/linkage.cc
diff --git a/src/compiler/linkage.cc b/src/compiler/linkage.cc
index 53a4ecccb8f86f40b32865b477b052c46d6434b1..03c27a6bf1687fa78f520317e3d4016ca66d9cab 100644 
--- a/src/compiler/linkage.cc
+++ b/src/compiler/linkage.cc
@@ -218,10 +218,10 @@ int Linkage::FrameStateInputCount(Runtime::FunctionId function) { 
   switch (function) {
     case Runtime::kAllocateInTargetSpace:
     case Runtime::kDateField:
- case Runtime::kFinalizeClassDefinition: // TODO(conradw): Is it safe? case Runtime::kDefineClassMethod: // TODO(jarin): Is it safe? case Runtime::kDefineGetterPropertyUnchecked: // TODO(jarin): Is it safe? case Runtime::kDefineSetterPropertyUnchecked: // TODO(jarin): Is it safe? + case Runtime::kFinalizeClassDefinition: // TODO(conradw): Is it safe? 
     case Runtime::kForInDone:
     case Runtime::kForInStep:
     case Runtime::kGetOriginalConstructor:
@@ -243,13 +243,14 @@ int Linkage::FrameStateInputCount(Runtime::FunctionId function) { 
     case Runtime::kInlineGetCallerJSFunction:
     case Runtime::kInlineGetPrototype:
     case Runtime::kInlineRegExpExec:
+    case Runtime::kInlineSubString:
+    case Runtime::kInlineToName:
+    case Runtime::kInlineToNumber:
     case Runtime::kInlineToObject:
-    case Runtime::kInlineToPrimitive:
     case Runtime::kInlineToPrimitive_Number:
     case Runtime::kInlineToPrimitive_String:
-    case Runtime::kInlineToNumber:
+    case Runtime::kInlineToPrimitive:
     case Runtime::kInlineToString:
-    case Runtime::kInlineToName:
       return 1;
     case Runtime::kInlineDeoptimizeNow:
     case Runtime::kInlineThrowNotDateError:
Index: src/full-codegen/arm/full-codegen-arm.cc
diff --git a/src/full-codegen/arm/full-codegen-arm.cc b/src/full-codegen/arm/full-codegen-arm.cc index 24dc51608246b9210775e13be332ff905bd77c01..f9dc82c9f5549202aeb1a75dfe74d84f711b7655 100644 
--- a/src/full-codegen/arm/full-codegen-arm.cc
+++ b/src/full-codegen/arm/full-codegen-arm.cc
@@ -321,7 +321,7 @@ void FullCodeGenerator::Generate() {
   // redeclaration.
   if (scope()->HasIllegalRedeclaration()) {
     Comment cmnt(masm_, "[ Declarations");
-    scope()->VisitIllegalRedeclaration(this);
+    VisitForEffect(scope()->GetIllegalRedeclaration());

   } else {
     PrepareForBailoutForId(BailoutId::FunctionEntry(), NO_REGISTERS);
Index: src/full-codegen/arm64/full-codegen-arm64.cc
diff --git a/src/full-codegen/arm64/full-codegen-arm64.cc b/src/full-codegen/arm64/full-codegen-arm64.cc index 59d59202b4182d51a879c60536108752626de5e9..de2a88e08a3b0af9f24b1dc1b0fe649eef875972 100644 
--- a/src/full-codegen/arm64/full-codegen-arm64.cc
+++ b/src/full-codegen/arm64/full-codegen-arm64.cc
@@ -328,7 +328,7 @@ void FullCodeGenerator::Generate() {
   // redeclaration.
   if (scope()->HasIllegalRedeclaration()) {
     Comment cmnt(masm_, "[ Declarations");
-    scope()->VisitIllegalRedeclaration(this);
+    VisitForEffect(scope()->GetIllegalRedeclaration());

   } else {
     PrepareForBailoutForId(BailoutId::FunctionEntry(), NO_REGISTERS);
Index: src/full-codegen/ia32/full-codegen-ia32.cc
diff --git a/src/full-codegen/ia32/full-codegen-ia32.cc b/src/full-codegen/ia32/full-codegen-ia32.cc index 1f1c19e2d98698478fead6c2735bf53dd40cfe33..b25bb71a8e29faf83338aae3ed5e3e207d1c4342 100644 
--- a/src/full-codegen/ia32/full-codegen-ia32.cc
+++ b/src/full-codegen/ia32/full-codegen-ia32.cc
@@ -322,7 +322,7 @@ void FullCodeGenerator::Generate() {
   // redeclaration.
   if (scope()->HasIllegalRedeclaration()) {
     Comment cmnt(masm_, "[ Declarations");
-    scope()->VisitIllegalRedeclaration(this);
+    VisitForEffect(scope()->GetIllegalRedeclaration());

   } else {
     PrepareForBailoutForId(BailoutId::FunctionEntry(), NO_REGISTERS);
Index: src/full-codegen/mips/full-codegen-mips.cc
diff --git a/src/full-codegen/mips/full-codegen-mips.cc b/src/full-codegen/mips/full-codegen-mips.cc index 0a3e5bbb2e4f3df1ad16517317cac17d4f13614c..9ffc29c370c6c64d92aaa8121b23b460efb5932c 100644 
--- a/src/full-codegen/mips/full-codegen-mips.cc
+++ b/src/full-codegen/mips/full-codegen-mips.cc
@@ -339,7 +339,7 @@ void FullCodeGenerator::Generate() {
   // redeclaration.
   if (scope()->HasIllegalRedeclaration()) {
     Comment cmnt(masm_, "[ Declarations");
-    scope()->VisitIllegalRedeclaration(this);
+    VisitForEffect(scope()->GetIllegalRedeclaration());

   } else {
     PrepareForBailoutForId(BailoutId::FunctionEntry(), NO_REGISTERS);
Index: src/full-codegen/mips64/full-codegen-mips64.cc
diff --git a/src/full-codegen/mips64/full-codegen-mips64.cc b/src/full-codegen/mips64/full-codegen-mips64.cc index 10528179134c176d544ba45438d893c790abd2d6..aa51cb50a3f396ed9d8fed79b6b1aae8bede1a62 100644 
--- a/src/full-codegen/mips64/full-codegen-mips64.cc
+++ b/src/full-codegen/mips64/full-codegen-mips64.cc
@@ -335,7 +335,7 @@ void FullCodeGenerator::Generate() {
   // redeclaration.
   if (scope()->HasIllegalRedeclaration()) {
     Comment cmnt(masm_, "[ Declarations");
-    scope()->VisitIllegalRedeclaration(this);
+    VisitForEffect(scope()->GetIllegalRedeclaration());

   } else {
     PrepareForBailoutForId(BailoutId::FunctionEntry(), NO_REGISTERS);
Index: src/full-codegen/ppc/full-codegen-ppc.cc
diff --git a/src/full-codegen/ppc/full-codegen-ppc.cc b/src/full-codegen/ppc/full-codegen-ppc.cc index 3a24fd0c6c69c49a5a555d2544d5aeeadff1bef9..0a255549d67c91c4f5a05c3018f4dae50577846d 100644 
--- a/src/full-codegen/ppc/full-codegen-ppc.cc
+++ b/src/full-codegen/ppc/full-codegen-ppc.cc
@@ -333,7 +333,7 @@ void FullCodeGenerator::Generate() {
   // redeclaration.
   if (scope()->HasIllegalRedeclaration()) {
     Comment cmnt(masm_, "[ Declarations");
-    scope()->VisitIllegalRedeclaration(this);
+    VisitForEffect(scope()->GetIllegalRedeclaration());

   } else {
     PrepareForBailoutForId(BailoutId::FunctionEntry(), NO_REGISTERS);
Index: src/full-codegen/x64/full-codegen-x64.cc
diff --git a/src/full-codegen/x64/full-codegen-x64.cc b/src/full-codegen/x64/full-codegen-x64.cc index e6bb7ed40fe3036282d9594ad0a6cada9cfad8a3..6795f516a46096e1cd03cceb1efdbefaa919dcc9 100644 
--- a/src/full-codegen/x64/full-codegen-x64.cc
+++ b/src/full-codegen/x64/full-codegen-x64.cc
@@ -320,7 +320,7 @@ void FullCodeGenerator::Generate() {
   // redeclaration.
   if (scope()->HasIllegalRedeclaration()) {
     Comment cmnt(masm_, "[ Declarations");
-    scope()->VisitIllegalRedeclaration(this);
+    VisitForEffect(scope()->GetIllegalRedeclaration());

   } else {
     PrepareForBailoutForId(BailoutId::FunctionEntry(), NO_REGISTERS);
Index: src/full-codegen/x87/full-codegen-x87.cc
diff --git a/src/full-codegen/x87/full-codegen-x87.cc b/src/full-codegen/x87/full-codegen-x87.cc index 55c227ee3ff8b94788fafab057ae81d1830c3787..2e5768a1f03cb727b30683195a82e48242d4d134 100644 
--- a/src/full-codegen/x87/full-codegen-x87.cc
+++ b/src/full-codegen/x87/full-codegen-x87.cc
@@ -319,7 +319,7 @@ void FullCodeGenerator::Generate() {
   // redeclaration.
   if (scope()->HasIllegalRedeclaration()) {
     Comment cmnt(masm_, "[ Declarations");
-    scope()->VisitIllegalRedeclaration(this);
+    VisitForEffect(scope()->GetIllegalRedeclaration());

   } else {
     PrepareForBailoutForId(BailoutId::FunctionEntry(), NO_REGISTERS);
Index: src/isolate.cc
diff --git a/src/isolate.cc b/src/isolate.cc
index d60a481c201bd6428d7f1c6efbd713587a2970b5..445480a3eb46d860ded047f0ae9d8276172814e9 100644 
--- a/src/isolate.cc
+++ b/src/isolate.cc
@@ -1266,9 +1266,14 @@ bool Isolate::ComputeLocation(MessageLocation* target) { 
     Object* script = fun->shared()->script();
     if (script->IsScript() &&
         !(Script::cast(script)->source()->IsUndefined())) {
-      int pos = frame->LookupCode()->SourcePosition(frame->pc());
-      // Compute the location from the function and the reloc info.
       Handle<Script> casted_script(Script::cast(script));
+ // Compute the location from the function and the relocation info of the 
+      // baseline code. For optimized code this will use the deoptimization
+      // information to get canonical location information.
+      List<FrameSummary> frames(FLAG_max_inlining_levels + 1);
+      it.frame()->Summarize(&frames);
+      FrameSummary& summary = frames.last();
+      int pos = summary.code()->SourcePosition(summary.pc());
       *target = MessageLocation(casted_script, pos, pos + 1, handle(fun));
       return true;
     }
Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index 4bb9aa8f9ce73109f69b9a1a1547dd373b096e22..ba2efb2bcc9e1d7c742780f0f9d586b81a70c6f8 100644 
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -559,9 +559,9 @@ void Scope::SetIllegalRedeclaration(Expression* expression) { 
 }


-void Scope::VisitIllegalRedeclaration(AstVisitor* visitor) {
+Expression* Scope::GetIllegalRedeclaration() {
   DCHECK(HasIllegalRedeclaration());
-  illegal_redecl_->Accept(visitor);
+  return illegal_redecl_;
 }


Index: src/scopes.h
diff --git a/src/scopes.h b/src/scopes.h
index 021b7b3b17820a8aae3da5aa20096c846d7f056d..4b8fe4997b46a151439be29e70f9d30de261c240 100644 
--- a/src/scopes.h
+++ b/src/scopes.h
@@ -190,9 +190,9 @@ class Scope: public ZoneObject {
   // the additional requests will be silently ignored.
   void SetIllegalRedeclaration(Expression* expression);

-  // Visit the illegal redeclaration expression. Do not call if the
+  // Retrieve the illegal redeclaration expression. Do not call if the
   // scope doesn't have an illegal redeclaration node.
-  void VisitIllegalRedeclaration(AstVisitor* visitor);
+  Expression* GetIllegalRedeclaration();

   // Check if the scope has (at least) one illegal redeclaration.
   bool HasIllegalRedeclaration() const { return illegal_redecl_ != NULL; }
Index: test/message/regress/regress-4266.js
diff --git a/test/message/regress/regress-3995.js b/test/message/regress/regress-4266.js 
similarity index 77%
copy from test/message/regress/regress-3995.js
copy to test/message/regress/regress-4266.js
index ba84bc096527cc242b9ad43983b42eec0462c43c..552176bccf54e01f6c589ec420a06a0c4ba9e6c9 100644 
--- a/test/message/regress/regress-3995.js
+++ b/test/message/regress/regress-4266.js
@@ -3,5 +3,9 @@
 // found in the LICENSE file.

 (function() {
-  throw new Error("boom");
+  try {
+    [].foo();
+  } catch (e) {
+    throw e;
+  }
 })();
Index: test/message/regress/regress-4266.out
diff --git a/test/message/regress/regress-3995.out b/test/message/regress/regress-4266.out 
similarity index 50%
copy from test/message/regress/regress-3995.out
copy to test/message/regress/regress-4266.out
index e4f5b31d1f4b0979ac1b65f6a878a6115611cddd..d31541debe0a9530f0e4045bbdbd269086326e56 100644 
--- a/test/message/regress/regress-3995.out
+++ b/test/message/regress/regress-4266.out
@@ -2,9 +2,9 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.

-*%(basename)s:6: Error: boom
-  throw new Error("boom");
-  ^
-Error: boom
-    at *%(basename)s:6:9
-    at *%(basename)s:7:3
+*%(basename)s:9: TypeError: [].foo is not a function
+    throw e;
+    ^
+TypeError: [].foo is not a function
+    at *%(basename)s:7:8
+    at *%(basename)s:11:3
Index: test/mjsunit/regress/regress-4266.js
diff --git a/test/mjsunit/regress/regress-4266.js b/test/mjsunit/regress/regress-4266.js 
new file mode 100644
index 0000000000000000000000000000000000000000..f886250a87399acc62041246e995b48985d84a66 
--- /dev/null
+++ b/test/mjsunit/regress/regress-4266.js
@@ -0,0 +1,17 @@
+// Copyright 2015 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.
+
+// Flags: --turbo-filter=test --allow-natives-syntax
+
+function test() {
+  try {
+    [].foo();
+  } catch (e) {
+    return e.message;
+  }
+}
+
+assertEquals("[].foo is not a function", test());
+%OptimizeFunctionOnNextCall(test);
+assertEquals("[].foo is not a function", test());


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to