[debug] Report line numbers for Function constructor functions correctly

The spec says we have to insert some wrapper code with extra line breaks
in it, but this confuses users when they see stack traces as the line
numbers come from the code with the wrapper, instead of the original.

This CL sets line_offset on the script to indicate that line numbers
should be offset by the 2 extra line breaks when reading them out e.g.
for the purpose of stack traces.

Bug: chromium:109362
Change-Id: Ib608e1043c38b595b1466766f7592e993ee3b996
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1741660Reviewed-by: Simon Zünd <szuend@chromium.org>
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63127}

src/builtins/builtins-function.cc

@@ -93,6 +93,17 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
     function->shared().set_name_should_print_as_anonymous(true);
   }
 
+  // The spec says that we have to wrap code created via the function
+  // constructor in e.g. 'function anonymous(' as above, including with extra
+  // line breaks. Ths is confusing when reporting stack traces from the eval'd
+  // code as the line number of the error is always reported with 2 extra line
+  // breaks e.g. line 1 is reported as line 3. We fix this up here by setting
+  // line_offset which is read by stack trace code.
+  Handle<Script> script(Script::cast(function->shared().script()), isolate);
+  if (script->line_offset() == 0) {
+    script->set_line_offset(-2);
+  }
+
   // If new.target is equal to target then the function created
   // is already correctly setup and nothing else should be done
   // here. But if new.target is not equal to target then we are

test/cctest/test-api-stack-traces.cc

@@ -250,31 +250,31 @@ static void AnalyzeStackInNativeCode(
     v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(
         args.GetIsolate(), 5, v8::StackTrace::kOverview);
     CHECK_EQ(3, stackTrace->GetFrameCount());
-    checkStackFrame(nullptr, "function.name", 3, 1, true, false,
+    checkStackFrame(nullptr, "function.name", 1, 1, true, false,
                     stackTrace->GetFrame(isolate, 0));
   } else if (testGroup == kDisplayName) {
     v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(
         args.GetIsolate(), 5, v8::StackTrace::kOverview);
     CHECK_EQ(3, stackTrace->GetFrameCount());
-    checkStackFrame(nullptr, "function.displayName", 3, 1, true, false,
+    checkStackFrame(nullptr, "function.displayName", 1, 1, true, false,
                     stackTrace->GetFrame(isolate, 0));
   } else if (testGroup == kFunctionNameAndDisplayName) {
     v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(
         args.GetIsolate(), 5, v8::StackTrace::kOverview);
     CHECK_EQ(3, stackTrace->GetFrameCount());
-    checkStackFrame(nullptr, "function.displayName", 3, 1, true, false,
+    checkStackFrame(nullptr, "function.displayName", 1, 1, true, false,
                     stackTrace->GetFrame(isolate, 0));
   } else if (testGroup == kDisplayNameIsNotString) {
     v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(
         args.GetIsolate(), 5, v8::StackTrace::kOverview);
     CHECK_EQ(3, stackTrace->GetFrameCount());
-    checkStackFrame(nullptr, "function.name", 3, 1, true, false,
+    checkStackFrame(nullptr, "function.name", 1, 1, true, false,
                     stackTrace->GetFrame(isolate, 0));
   } else if (testGroup == kFunctionNameIsNotString) {
     v8::Local<v8::StackTrace> stackTrace = v8::StackTrace::CurrentStackTrace(
         args.GetIsolate(), 5, v8::StackTrace::kOverview);
     CHECK_EQ(3, stackTrace->GetFrameCount());
-    checkStackFrame(nullptr, "", 3, 1, true, false,
+    checkStackFrame(nullptr, "", 1, 1, true, false,
                     stackTrace->GetFrame(isolate, 0));
   }
 }

test/inspector/runtime/evaluate-new-function-error-expected.txt

+Tests that Runtime.evaluate has the correct error line number for 'new Function(...)'
+{
+    id : <messageId>
+    result : {
+        exceptionDetails : {
+            columnNumber : 3
+            exception : {
+                className : TypeError
+                description : TypeError: 0 is not a function     at eval (eval at <anonymous> (:1:1), <anonymous>:1:4)     at <anonymous>:1:22
+                objectId : <objectId>
+                subtype : error
+                type : object
+            }
+            exceptionId : <exceptionId>
+            lineNumber : 0
+            scriptId : <scriptId>
+            text : Uncaught
+        }
+        result : {
+            className : TypeError
+            description : TypeError: 0 is not a function     at eval (eval at <anonymous> (:1:1), <anonymous>:1:4)     at <anonymous>:1:22
+            objectId : <objectId>
+            subtype : error
+            type : object
+        }
+    }
+}

test/inspector/runtime/evaluate-new-function-error.js

+// Copyright 2019 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.
+
+let {session, contextGroup, Protocol} = InspectorTest.start("Tests that Runtime.evaluate has the correct error line number for 'new Function(...)'");
+
+var message = { expression: "new Function('(0)()')();" };
+
+Protocol.Runtime.evaluate(message)
+  .then(message => InspectorTest.logMessage(message))
+  .then(() => InspectorTest.completeTest());

test/mjsunit/regress/regress-crbug-109362.js

@@ -19,7 +19,7 @@ function test(expectation, f) {
 1 + reference_error //@ sourceURL=evaltest
 })
 */
-test("3:5", new Function(
+test("1:5", new Function(
     '1 + reference_error //@ sourceURL=evaltest'));
 /*
 (function(x
@@ -28,7 +28,7 @@ test("3:5", new Function(
  1 + reference_error //@ sourceURL=evaltest
 })
 */
-test("4:6", new Function(
+test("2:6", new Function(
     'x', '\n 1 + reference_error //@ sourceURL=evaltest'));
 /*
 (function(x
@@ -40,7 +40,7 @@ test("4:6", new Function(
  1 + reference_error //@ sourceURL=evaltest
 })
 */
-test("7:6", new Function(
+test("5:6", new Function(
     'x\n\n', "z//\n", "y", '\n 1 + reference_error //@ sourceURL=evaltest'));
 /*
 (function(x/\*,z//
@@ -49,7 +49,7 @@ test("7:6", new Function(
 1 + reference_error //@ sourceURL=evaltest
 })
 */
-test("4:5", new Function(
+test("2:5", new Function(
     'x/*', "z//\n", "y*/", '1 + reference_error //@ sourceURL=evaltest'));
 /*
 (function () {