Propagate exceptions from JSFunction::SetName as needed

JSFunction::SetName can fail if it tries to create a string with
length > String::kMaxLength (either by prepending "set "/"get " or
by surrounding a Symbol descriptor with "["/"]").

This patch propagates that exception to the surrounding code rather
than CHECK-failing.

Bug: chromium:740398
Change-Id: I394943af481f3147387dd82ec5862d7071d57827
Reviewed-on: https://chromium-review.googlesource.com/566092Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46601}

src/compiler/linkage.cc

@@ -147,8 +147,6 @@ bool Linkage::NeedsFrameStateInput(Runtime::FunctionId function) {
     case Runtime::kAllocateInTargetSpace:
     case Runtime::kConvertReceiver:
     case Runtime::kCreateIterResultObject:
-    case Runtime::kDefineGetterPropertyUnchecked:  // TODO(jarin): Is it safe?
-    case Runtime::kDefineSetterPropertyUnchecked:  // TODO(jarin): Is it safe?
     case Runtime::kGeneratorGetContinuation:
     case Runtime::kIncBlockCounter:
     case Runtime::kIsFunction:

src/objects.cc

@@ -12925,21 +12925,27 @@ Handle<String> JSFunction::GetDebugName(Handle<JSFunction> function) {
   return JSFunction::GetName(function);
 }
 
-void JSFunction::SetName(Handle<JSFunction> function, Handle<Name> name,
+bool JSFunction::SetName(Handle<JSFunction> function, Handle<Name> name,
                          Handle<String> prefix) {
   Isolate* isolate = function->GetIsolate();
-  Handle<String> function_name = Name::ToFunctionName(name).ToHandleChecked();
+  Handle<String> function_name;
+  ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, function_name,
+                                   Name::ToFunctionName(name), false);
   if (prefix->length() > 0) {
     IncrementalStringBuilder builder(isolate);
     builder.AppendString(prefix);
     builder.AppendCharacter(' ');
     builder.AppendString(function_name);
-    function_name = builder.Finish().ToHandleChecked();
+    ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, function_name, builder.Finish(),
+                                     false);
   }
+  RETURN_ON_EXCEPTION_VALUE(
+      isolate,
       JSObject::DefinePropertyOrElementIgnoreAttributes(
           function, isolate->factory()->name_string(), function_name,
-      static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY))
-      .ToHandleChecked();
+          static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY)),
+      false);
+  return true;
 }
 
 namespace {

src/objects.h

@@ -5278,9 +5278,10 @@ class JSFunction: public JSObject {
 
   // ES6 section 9.2.11 SetFunctionName
   // Because of the way this abstract operation is used in the spec,
-  // it should never fail.
-  static void SetName(Handle<JSFunction> function, Handle<Name> name,
-                      Handle<String> prefix);
+  // it should never fail, but in practice it will fail if the generated
+  // function name's length exceeds String::kMaxLength.
+  static MUST_USE_RESULT bool SetName(Handle<JSFunction> function,
+                                      Handle<Name> name, Handle<String> prefix);
 
   // The function's displayName if it is set, otherwise name if it is
   // configured, otherwise shared function info

src/runtime/runtime-object.cc

@@ -758,8 +758,10 @@ RUNTIME_FUNCTION(Runtime_DefineDataPropertyInLiteral) {
 
   if (flags & DataPropertyInLiteralFlag::kSetFunctionName) {
     DCHECK(value->IsJSFunction());
-    JSFunction::SetName(Handle<JSFunction>::cast(value), name,
-                        isolate->factory()->empty_string());
+    if (!JSFunction::SetName(Handle<JSFunction>::cast(value), name,
+                             isolate->factory()->empty_string())) {
+      return isolate->heap()->exception();
+    }
   }
 
   LookupIterator it = LookupIterator::PropertyOrElement(
@@ -857,7 +859,9 @@ RUNTIME_FUNCTION(Runtime_DefineGetterPropertyUnchecked) {
   CONVERT_PROPERTY_ATTRIBUTES_CHECKED(attrs, 3);
 
   if (String::cast(getter->shared()->name())->length() == 0) {
-    JSFunction::SetName(getter, name, isolate->factory()->get_string());
+    if (!JSFunction::SetName(getter, name, isolate->factory()->get_string())) {
+      return isolate->heap()->exception();
+    }
   }
 
   RETURN_FAILURE_ON_EXCEPTION(
@@ -985,7 +989,9 @@ RUNTIME_FUNCTION(Runtime_DefineSetterPropertyUnchecked) {
   CONVERT_PROPERTY_ATTRIBUTES_CHECKED(attrs, 3);
 
   if (String::cast(setter->shared()->name())->length() == 0) {
-    JSFunction::SetName(setter, name, isolate->factory()->set_string());
+    if (!JSFunction::SetName(setter, name, isolate->factory()->set_string())) {
+      return isolate->heap()->exception();
+    }
   }
 
   RETURN_FAILURE_ON_EXCEPTION(

src/wasm/wasm-js.cc

@@ -839,7 +839,8 @@ Handle<JSFunction> InstallFunc(Isolate* isolate, Handle<JSObject> object,
   Handle<FunctionTemplateInfo> temp = NewTemplate(isolate, func);
   Handle<JSFunction> function =
       ApiNatives::InstantiateFunction(temp).ToHandleChecked();
-  JSFunction::SetName(function, name, isolate->factory()->empty_string());
+  CHECK(
+      JSFunction::SetName(function, name, isolate->factory()->empty_string()));
   function->shared()->set_length(length);
   PropertyAttributes attributes = static_cast<PropertyAttributes>(DONT_ENUM);
   JSObject::AddProperty(object, name, function, attributes);

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

+// Copyright 2017 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.
+
+var longString = (function() {
+  var str = "";
+  for (var i = 0; i < 24; i++) {
+    str += "abcdefgh12345678" + str;
+  }
+  return str;
+})();
+
+assertThrows(() => { return { get[longString]() { } } }, RangeError);
+assertThrows(() => { return { set[longString](v) { } } }, RangeError);
+assertThrows(() => { return { [Symbol(longString)]: () => {} } }, RangeError);