Make compilers agree on source position of thrown errors.

mstarzinger · Commit bot · commit 10dd9ce8beef · 2015-04-13T09:02:48.000Z
This makes the compilers agree on the source position of a message generated by "throw new Error()", it points to the beginning of the throw directive. R=titzer@chromium.org TEST=message/regress/regress-3995 BUG=v8:3995 LOG=N Review URL: https://codereview.chromium.org/1049703002 Cr-Commit-Position: refs/heads/master@{#27775}
diff --git a/src/assembler.cc b/src/assembler.cc
@@ -1615,19 +1615,20 @@ bool PositionsRecorder::WriteRecordedPositions() {
     EnsureSpace ensure_space(assembler_);
     assembler_->RecordRelocInfo(RelocInfo::STATEMENT_POSITION,
                                 state_.current_statement_position);
-    state_.written_statement_position = state_.current_statement_position;
     written = true;
   }
+  state_.written_statement_position = state_.current_statement_position;
 
   // Write the position if it is different from what was written last time and
-  // also different from the written statement position.
+  // also different from the statement position that was just written.
   if (state_.current_position != state_.written_position &&
-      state_.current_position != state_.written_statement_position) {
+      (state_.current_position != state_.written_statement_position ||
+       !written)) {
     EnsureSpace ensure_space(assembler_);
     assembler_->RecordRelocInfo(RelocInfo::POSITION, state_.current_position);
-    state_.written_position = state_.current_position;
     written = true;
   }
+  state_.written_position = state_.current_position;
 
   // Return whether something was written.
   return written;
diff --git a/src/full-codegen.cc b/src/full-codegen.cc
@@ -1639,6 +1639,7 @@ void FullCodeGenerator::VisitNativeFunctionLiteral(
 void FullCodeGenerator::VisitThrow(Throw* expr) {
   Comment cmnt(masm_, "[ Throw");
   VisitForStackValue(expr->exception());
+  SetSourcePosition(expr->position());
   __ CallRuntime(Runtime::kThrow, 1);
   // Never returns here.
 }
diff --git a/src/prettyprinter.cc b/src/prettyprinter.cc
@@ -325,7 +325,7 @@ void CallPrinter::VisitCall(Call* node) {
 
 
 void CallPrinter::VisitCallNew(CallNew* node) {
-  bool was_found = !found_ && node->expression()->position() == position_;
+  bool was_found = !found_ && node->position() == position_;
   if (was_found) found_ = true;
   Find(node->expression(), was_found);
   FindArguments(node->arguments());
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
@@ -5312,8 +5312,7 @@ void TryCatchMixedNestingCheck(v8::TryCatch* try_catch) {
   CHECK_EQ(0,
            strcmp(*v8::String::Utf8Value(message->Get()), "Uncaught Error: a"));
   CHECK_EQ(1, message->GetLineNumber());
-  // TODO(3995): Our compilers disagree about the position.
-  if (!i::FLAG_always_opt) CHECK_EQ(6, message->GetStartColumn());
+  CHECK_EQ(0, message->GetStartColumn());
 }
 
 
@@ -9795,11 +9794,7 @@ THREADED_TEST(ConstructorForObject) {
     value = CompileRun("new obj2(28)");
     CHECK(try_catch.HasCaught());
     String::Utf8Value exception_value1(try_catch.Exception());
-    // TODO(3995): Our compilers disagree about the position (and message).
-    if (!i::FLAG_always_opt) {
-      CHECK_EQ(0,
-               strcmp("TypeError: obj2 is not a function", *exception_value1));
-    }
+    CHECK_EQ(0, strcmp("TypeError: obj2 is not a function", *exception_value1));
     try_catch.Reset();
 
     Local<Value> args[] = {v8_num(29)};
@@ -15005,11 +15000,9 @@ void AnalyzeStackInNativeCode(const v8::FunctionCallbackInfo<v8::Value>& args) {
     checkStackFrame(origin, "foo", 6, 3, false, false,
                     stackTrace->GetFrame(1));
     // This is the source string inside the eval which has the call to foo.
-    checkStackFrame(NULL, "", 1, 5, false, false,
-                    stackTrace->GetFrame(2));
+    checkStackFrame(NULL, "", 1, 1, false, false, stackTrace->GetFrame(2));
     // The last frame is an anonymous function which has the initial eval call.
-    checkStackFrame(origin, "", 8, 7, false, false,
-                    stackTrace->GetFrame(3));
+    checkStackFrame(origin, "", 8, 7, false, false, stackTrace->GetFrame(3));
 
     CHECK(stackTrace->AsArray()->IsArray());
   } else if (testGroup == kDetailedTest) {
@@ -15022,11 +15015,9 @@ void AnalyzeStackInNativeCode(const v8::FunctionCallbackInfo<v8::Value>& args) {
                     stackTrace->GetFrame(1));
     bool is_eval = true;
     // This is the source string inside the eval which has the call to baz.
-    checkStackFrame(NULL, "", 1, 5, is_eval, false,
-                    stackTrace->GetFrame(2));
+    checkStackFrame(NULL, "", 1, 1, is_eval, false, stackTrace->GetFrame(2));
     // The last frame is an anonymous function which has the initial eval call.
-    checkStackFrame(origin, "", 10, 1, false, false,
-                    stackTrace->GetFrame(3));
+    checkStackFrame(origin, "", 10, 1, false, false, stackTrace->GetFrame(3));
 
     CHECK(stackTrace->AsArray()->IsArray());
   }
diff --git a/test/message/regress/regress-3995.js b/test/message/regress/regress-3995.js
@@ -0,0 +1,7 @@
+// 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.
+
+(function() {
+  throw new Error("boom");
+})();
diff --git a/test/message/regress/regress-3995.out b/test/message/regress/regress-3995.out
@@ -0,0 +1,10 @@
+# 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.
+
+*%(basename)s:6: Error: boom
+  throw new Error("boom");
+  ^
+Error: boom
+    at *%(basename)s:6:9
+    at *%(basename)s:7:3
diff --git a/test/mjsunit/debug-backtrace.js b/test/mjsunit/debug-backtrace.js
@@ -195,7 +195,7 @@ function listener(event, exec_state, event_data, data) {
       assertEquals("m", response.lookup(frame.func.ref).inferredName);
       assertFalse(frame.constructCall);
       assertEquals(35, frame.line);
-      assertEquals(6, frame.column);
+      assertEquals(2, frame.column);
       assertEquals(0, frame.arguments.length);
 
       json = '{"seq":0,"type":"request","command":"frame","arguments":{"number":3}}'

Original file line number	Diff line number	Diff line change
`@@ -1639,6 +1639,7 @@ void FullCodeGenerator::VisitNativeFunctionLiteral(`
`1639`	`1639`	`void FullCodeGenerator::VisitThrow(Throw* expr) {`
`1640`	`1640`	`Comment cmnt(masm_, "[ Throw");`
`1641`	`1641`	`VisitForStackValue(expr->exception());`
	`1642`	`+ SetSourcePosition(expr->position());`
`1642`	`1643`	`__ CallRuntime(Runtime::kThrow, 1);`
`1643`	`1644`	`// Never returns here.`
`1644`	`1645`	`}`