Fix type conversions in JSStackFrame accessors

We cannot assume that the receiver is a JSObject, nor can we assume
ToObject() completes successfully.

TBR=yangguo@chromium.org

Bug: chromium:739954
Change-Id: Id55571131ef8755e86f15cd2acb918ff0f1b7788
Reviewed-on: https://chromium-review.googlesource.com/632376Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47611}

src/messages.cc

@@ -341,11 +341,11 @@ Handle<Object> JSStackFrame::GetFunctionName() {
 
 namespace {
 
-bool CheckMethodName(Isolate* isolate, Handle<JSObject> obj, Handle<Name> name,
-                     Handle<JSFunction> fun,
+bool CheckMethodName(Isolate* isolate, Handle<JSReceiver> receiver,
+                     Handle<Name> name, Handle<JSFunction> fun,
                      LookupIterator::Configuration config) {
   LookupIterator iter =
-      LookupIterator::PropertyOrElement(isolate, obj, name, config);
+      LookupIterator::PropertyOrElement(isolate, receiver, name, config);
   if (iter.state() == LookupIterator::DATA) {
     return iter.GetDataValue().is_identical_to(fun);
   } else if (iter.state() == LookupIterator::ACCESSOR) {
@@ -376,13 +376,14 @@ Handle<Object> JSStackFrame::GetMethodName() {
     return isolate_->factory()->null_value();
   }
 
-  Handle<JSReceiver> receiver =
-      Object::ToObject(isolate_, receiver_).ToHandleChecked();
-  if (!receiver->IsJSObject()) {
+  Handle<JSReceiver> receiver;
+  if (!Object::ToObject(isolate_, receiver_).ToHandle(&receiver)) {
+    DCHECK(isolate_->has_pending_exception());
+    isolate_->clear_pending_exception();
+    isolate_->set_external_caught_exception(false);
     return isolate_->factory()->null_value();
   }
 
-  Handle<JSObject> obj = Handle<JSObject>::cast(receiver);
   Handle<String> name(function_->shared()->name(), isolate_);
   // ES2015 gives getters and setters name prefixes which must
   // be stripped to find the property name.
@@ -390,15 +391,15 @@ Handle<Object> JSStackFrame::GetMethodName() {
       name->IsUtf8EqualTo(CStrVector("set "), true)) {
     name = isolate_->factory()->NewProperSubString(name, 4, name->length());
   }
-  if (CheckMethodName(isolate_, obj, name, function_,
+  if (CheckMethodName(isolate_, receiver, name, function_,
                       LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR)) {
     return name;
   }
 
   HandleScope outer_scope(isolate_);
   Handle<Object> result;
-  for (PrototypeIterator iter(isolate_, obj, kStartAtReceiver); !iter.IsAtEnd();
-       iter.Advance()) {
+  for (PrototypeIterator iter(isolate_, receiver, kStartAtReceiver);
+       !iter.IsAtEnd(); iter.Advance()) {
     Handle<Object> current = PrototypeIterator::GetCurrent(iter);
     if (!current->IsJSObject()) break;
     Handle<JSObject> current_obj = Handle<JSObject>::cast(current);
@@ -426,14 +427,21 @@ Handle<Object> JSStackFrame::GetTypeName() {
   // TODO(jgruber): Check for strict/constructor here as in
   // CallSitePrototypeGetThis.
 
-  if (receiver_->IsNullOrUndefined(isolate_))
+  if (receiver_->IsNullOrUndefined(isolate_)) {
     return isolate_->factory()->null_value();
+  } else if (receiver_->IsJSProxy()) {
+    return isolate_->factory()->Proxy_string();
+  }
 
-  if (receiver_->IsJSProxy()) return isolate_->factory()->Proxy_string();
+  Handle<JSReceiver> receiver;
+  if (!Object::ToObject(isolate_, receiver_).ToHandle(&receiver)) {
+    DCHECK(isolate_->has_pending_exception());
+    isolate_->clear_pending_exception();
+    isolate_->set_external_caught_exception(false);
+    return isolate_->factory()->null_value();
+  }
 
-  Handle<JSReceiver> receiver_object =
-      Object::ToObject(isolate_, receiver_).ToHandleChecked();
-  return JSReceiver::GetConstructorName(receiver_object);
+  return JSReceiver::GetConstructorName(receiver);
 }
 
 int JSStackFrame::GetLineNumber() {
@@ -570,8 +578,10 @@ void AppendMethodCall(Isolate* isolate, JSStackFrame* call_site,
       }
     }
   } else {
-    builder->AppendString(Handle<String>::cast(type_name));
-    builder->AppendCharacter('.');
+    if (IsNonEmptyString(type_name)) {
+      builder->AppendString(Handle<String>::cast(type_name));
+      builder->AppendCharacter('.');
+    }
     if (IsNonEmptyString(method_name)) {
       builder->AppendString(Handle<String>::cast(method_name));
     } else {

test/mjsunit/regress/regress-3718.js

@@ -18,4 +18,5 @@ assertEquals("String", getTypeName(""));
 assertEquals("Boolean", getTypeName(false));
 assertEquals("Object", getTypeName({}));
 assertEquals("Array", getTypeName([]));
+assertEquals("Proxy", getTypeName(new Proxy({},{})));
 assertEquals("Custom", getTypeName(new (function Custom(){})()));

test/mjsunit/stack-traces-custom.js

@@ -24,3 +24,28 @@ try {
   assertEquals("k", frames[4].getMethodName());
   assertEquals(null, frames[5].getMethodName());
 }
+
+function testMethodName(f, frameNumber, expectedName) {
+  try {
+    Error.prepareStackTrace = function(e, frames) { return frames; }
+    f();
+    assertUnreachable();
+  } catch (e) {
+    const frame = e.stack[frameNumber];
+    assertEquals(expectedName, frame.getMethodName());
+  } finally {
+    Error.prepareStackTrace = undefined;
+  }
+}
+
+const thrower = { valueOf: () => { throw new Error(); }};
+
+{
+  const str = "";
+  testMethodName(() => { str.indexOf(str, thrower); }, 1, "indexOf");
+}
+
+{
+  const nr = 42;
+  testMethodName(() => { nr.toString(thrower); }, 1, "toString");
+}