From 1a94bc20a12254875704c45262e2b87bf85b63f6 Mon Sep 17 00:00:00 2001
From: yangguo <yangguo@chromium.org>
Date: Tue, 13 Oct 2015 02:26:33 -0700
Subject: [PATCH] Fix Error object value lookups.

Looking up 'name' and 'message' properties at the same time and loading
the properties later can cause assertion failure if one of the properties
is an accessor and calling it changes the holder map. That may invalidate
the other lookup.

R=jkummerow@chromium.org
BUG=chromium:542101
LOG=N

Review URL: https://codereview.chromium.org/1403923002

Cr-Commit-Position: refs/heads/master@{#31229}
---
 src/messages.cc                              | 30 +++++++++-----------
 test/mjsunit/regress/regress-crbug-542101.js | 10 +++++++
 2 files changed, 24 insertions(+), 16 deletions(-)
 create mode 100644 test/mjsunit/regress/regress-crbug-542101.js

diff --git a/src/messages.cc b/src/messages.cc
index 640c2dff4e..9489071403 100644
--- a/src/messages.cc
+++ b/src/messages.cc
@@ -400,10 +400,6 @@ MaybeHandle<String> ErrorToStringHelper::Stringify(Isolate* isolate,
   Handle<String> name_string = isolate->factory()->name_string();
   LookupIterator internal_error_lookup(
       error, internal_key, LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
-  LookupIterator message_lookup(
-      error, message_string, LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
-  LookupIterator name_lookup(error, name_string,
-                             LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
 
   // Find out whether an internally created error object is on the prototype
   // chain. If the name property is found on a holder prior to the internally
@@ -412,24 +408,26 @@ MaybeHandle<String> ErrorToStringHelper::Stringify(Isolate* isolate,
   // Similar for the message property. If the message property shadows the
   // internally created error object, use that message property. Otherwise
   // use empty string as message.
-  if (internal_error_lookup.IsFound()) {
-    if (!ShadowsInternalError(isolate, &name_lookup, &internal_error_lookup)) {
-      Handle<JSObject> holder = internal_error_lookup.GetHolder<JSObject>();
-      name = Handle<String>(holder->constructor_name());
-    }
-    if (!ShadowsInternalError(isolate, &message_lookup,
-                              &internal_error_lookup)) {
-      message = isolate->factory()->empty_string();
-    }
-  }
-  if (name.is_null()) {
+  LookupIterator name_lookup(error, name_string,
+                             LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
+  if (internal_error_lookup.IsFound() &&
+      !ShadowsInternalError(isolate, &name_lookup, &internal_error_lookup)) {
+    Handle<JSObject> holder = internal_error_lookup.GetHolder<JSObject>();
+    name = Handle<String>(holder->constructor_name());
+  } else {
     ASSIGN_RETURN_ON_EXCEPTION(
         isolate, name,
         GetStringifiedProperty(isolate, &name_lookup,
                                isolate->factory()->Error_string()),
         String);
   }
-  if (message.is_null()) {
+
+  LookupIterator message_lookup(
+      error, message_string, LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
+  if (internal_error_lookup.IsFound() &&
+      !ShadowsInternalError(isolate, &message_lookup, &internal_error_lookup)) {
+    message = isolate->factory()->empty_string();
+  } else {
     ASSIGN_RETURN_ON_EXCEPTION(
         isolate, message,
         GetStringifiedProperty(isolate, &message_lookup,
diff --git a/test/mjsunit/regress/regress-crbug-542101.js b/test/mjsunit/regress/regress-crbug-542101.js
new file mode 100644
index 0000000000..1a4c2b37ce
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-542101.js
@@ -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.
+
+(function() {
+  Error.prototype.toString.call({
+    get name() { return { __proto__: this }; },
+    get message() { }
+  });
+})();