[api] fix incorrect parameter end position in CompileFunctionInContext

With --harmony-function-tostring enabled (now enabled by --harmony),
CompileFunctionInContext would produce incorrect results whenever called
with 1 or more argument parameters, due to specifying an incorrect end
position for the parameters.

BUG=v8:6190, v8:4958
R=littledan@chromium.org, adamk@chromium.org, jwolfe@igalia.com

Change-Id: Ied2bcba44116311ebcae3967963472b4e1058fd3
Reviewed-on: https://chromium-review.googlesource.com/465515
Commit-Queue: Caitlin Potter <caitp@igalia.com>
Reviewed-by: Daniel Ehrenberg <littledan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44340}

src/api.cc

@@ -2296,8 +2296,10 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
     }
     i::Handle<i::String> brackets;
     if (i::FLAG_harmony_function_tostring) {
+      // Append linefeed and signal that text beyond the linefeed is not part of
+      // the formal parameters.
       brackets = factory->NewStringFromStaticChars("\n) {");
-      parameters_end_pos = source_string->length() - 3;
+      parameters_end_pos = source_string->length() + 1;
     } else {
       brackets = factory->NewStringFromStaticChars("){");
     }

test/cctest/cctest.h

@@ -318,6 +318,9 @@ static inline v8::Local<v8::Value> v8_num(double x) {
   return v8::Number::New(v8::Isolate::GetCurrent(), x);
 }
 
+static inline v8::Local<v8::Integer> v8_int(int32_t x) {
+  return v8::Integer::New(v8::Isolate::GetCurrent(), x);
+}
 
 static inline v8::Local<v8::String> v8_str(const char* x) {
   return v8::String::NewFromUtf8(v8::Isolate::GetCurrent(), x,

test/cctest/test-compiler.cc

@@ -563,6 +563,82 @@ TEST(CompileFunctionInContextScriptOrigin) {
   CHECK_EQ(42 + strlen("throw "), static_cast<unsigned>(frame->GetColumn()));
 }
 
+TEST(CompileFunctionInContextHarmonyFunctionToString) {
+#define CHECK_NOT_CAUGHT(__local_context__, try_catch, __op__)                \
+  do {                                                                        \
+    const char* op = (__op__);                                                \
+    v8::Local<v8::Context> context = (__local_context__);                     \
+    if (try_catch.HasCaught()) {                                              \
+      v8::String::Utf8Value error(                                            \
+          try_catch.Exception()->ToString(context).ToLocalChecked());         \
+      V8_Fatal(__FILE__, __LINE__,                                            \
+               "Unexpected exception thrown during %s:\n\t%s\n", op, *error); \
+    }                                                                         \
+  } while (0)
+
+  auto previous_flag = v8::internal::FLAG_harmony_function_tostring;
+  v8::internal::FLAG_harmony_function_tostring = true;
+  {
+    CcTest::InitializeVM();
+    v8::HandleScope scope(CcTest::isolate());
+    LocalContext env;
+
+    // Regression test for v8:6190
+    {
+      v8::ScriptOrigin origin(v8_str("test"), v8_int(22), v8_int(41));
+      v8::ScriptCompiler::Source script_source(v8_str("return event"), origin);
+
+      v8::Local<v8::String> params[] = {v8_str("event")};
+      v8::TryCatch try_catch(CcTest::isolate());
+      v8::MaybeLocal<v8::Function> maybe_fun =
+          v8::ScriptCompiler::CompileFunctionInContext(
+              env.local(), &script_source, arraysize(params), params, 0,
+              nullptr);
+
+      CHECK_NOT_CAUGHT(env.local(), try_catch,
+                       "v8::ScriptCompiler::CompileFunctionInContext");
+
+      v8::Local<v8::Function> fun = maybe_fun.ToLocalChecked();
+      CHECK(!fun.IsEmpty());
+      CHECK(!try_catch.HasCaught());
+      v8::Local<v8::String> result =
+          fun->ToString(env.local()).ToLocalChecked();
+      v8::Local<v8::String> expected = v8_str(
+          "function anonymous(event\n"
+          ") {return event\n"
+          "}");
+      CHECK(expected->Equals(env.local(), result).FromJust());
+    }
+
+    // With no parameters:
+    {
+      v8::ScriptOrigin origin(v8_str("test"), v8_int(17), v8_int(31));
+      v8::ScriptCompiler::Source script_source(v8_str("return 0"), origin);
+
+      v8::TryCatch try_catch(CcTest::isolate());
+      v8::MaybeLocal<v8::Function> maybe_fun =
+          v8::ScriptCompiler::CompileFunctionInContext(
+              env.local(), &script_source, 0, nullptr, 0, nullptr);
+
+      CHECK_NOT_CAUGHT(env.local(), try_catch,
+                       "v8::ScriptCompiler::CompileFunctionInContext");
+
+      v8::Local<v8::Function> fun = maybe_fun.ToLocalChecked();
+      CHECK(!fun.IsEmpty());
+      CHECK(!try_catch.HasCaught());
+      v8::Local<v8::String> result =
+          fun->ToString(env.local()).ToLocalChecked();
+      v8::Local<v8::String> expected = v8_str(
+          "function anonymous(\n"
+          ") {return 0\n"
+          "}");
+      CHECK(expected->Equals(env.local(), result).FromJust());
+    }
+  }
+  v8::internal::FLAG_harmony_function_tostring = previous_flag;
+
+#undef CHECK_NOT_CAUGHT
+}
 
 #ifdef ENABLE_DISASSEMBLER
 static Handle<JSFunction> GetJSFunction(v8::Local<v8::Object> obj,