[builtins] Add reference error for global object property access

Fixes V8 correctness failure when there's a proxy in the global object
prototype chain and unsuccessful attempt is made to access a property.

Bug: chromium:849024
Change-Id: I829e1a6c038982b7c7a77f8bdefb61facb4614f0
Reviewed-on: https://chromium-review.googlesource.com/1124446
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54237}

src/builtins/builtins-definitions.h

@@ -889,7 +889,7 @@ namespace internal {
   TFJ(ProxyConstructor, 2, kReceiver, kTarget, kHandler)                       \
   TFJ(ProxyRevocable, 2, kReceiver, kTarget, kHandler)                         \
   TFJ(ProxyRevoke, 0, kReceiver)                                               \
-  TFS(ProxyGetProperty, kProxy, kName, kReceiverValue)                         \
+  TFS(ProxyGetProperty, kProxy, kName, kReceiverValue, kOnNonExistent)         \
   TFS(ProxyHasProperty, kProxy, kName)                                         \
   TFS(ProxySetProperty, kProxy, kName, kValue, kReceiverValue, kLanguageMode)  \
                                                                                \

src/builtins/builtins-proxy-gen.cc

@@ -482,6 +482,7 @@ TF_BUILTIN(ProxyGetProperty, ProxiesCodeStubAssembler) {
   Node* proxy = Parameter(Descriptor::kProxy);
   Node* name = Parameter(Descriptor::kName);
   Node* receiver = Parameter(Descriptor::kReceiverValue);
+  Node* on_non_existent = Parameter(Descriptor::kOnNonExistent);
 
   CSA_ASSERT(this, IsJSProxy(proxy));
 
@@ -531,7 +532,7 @@ TF_BUILTIN(ProxyGetProperty, ProxiesCodeStubAssembler) {
     // 7.a. Return ? target.[[Get]](P, Receiver).
     // TODO(mslekova): Introduce GetPropertyWithReceiver stub
     Return(CallRuntime(Runtime::kGetPropertyWithReceiver, context, target, name,
-                       receiver));
+                       receiver, on_non_existent));
   }
 
   BIND(&throw_proxy_handler_revoked);

src/ic/accessor-assembler.cc

@@ -520,13 +520,16 @@ void AccessorAssembler::HandleLoadICSmiHandlerCase(
         to_name_failed(this, Label::kDeferred);
 
     if (support_elements == kSupportElements) {
+      DCHECK_NE(on_nonexistent, OnNonExistent::kThrowReferenceError);
+
       TryToName(p->name, &if_index, &var_index, &if_unique_name, &var_unique,
                 &to_name_failed);
 
       BIND(&if_unique_name);
       exit_point->ReturnCallStub(
           Builtins::CallableFor(isolate(), Builtins::kProxyGetProperty),
-          p->context, holder, var_unique.value(), p->receiver);
+          p->context, holder, var_unique.value(), p->receiver,
+          SmiConstant(on_nonexistent));
 
       BIND(&if_index);
       // TODO(mslekova): introduce TryToName that doesn't try to compute
@@ -535,12 +538,13 @@ void AccessorAssembler::HandleLoadICSmiHandlerCase(
 
       BIND(&to_name_failed);
       exit_point->ReturnCallRuntime(Runtime::kGetPropertyWithReceiver,
-                                    p->context, holder, p->name, p->receiver);
-
+                                    p->context, holder, p->name, p->receiver,
+                                    SmiConstant(on_nonexistent));
     } else {
       exit_point->ReturnCallStub(
           Builtins::CallableFor(isolate(), Builtins::kProxyGetProperty),
-          p->context, holder, p->name, p->receiver);
+          p->context, holder, p->name, p->receiver,
+          SmiConstant(on_nonexistent));
     }
   }
 
@@ -2196,7 +2200,7 @@ void AccessorAssembler::GenericPropertyLoad(Node* receiver, Node* receiver_map,
     direct_exit.ReturnCallStub(
         Builtins::CallableFor(isolate(), Builtins::kProxyGetProperty),
         p->context, receiver /*holder is the same as receiver*/, p->name,
-        receiver);
+        receiver, SmiConstant(OnNonExistent::kReturnUndefined));
   }
 }
 

src/ic/accessor-assembler.h

@@ -166,7 +166,6 @@ class AccessorAssembler : public CodeStubAssembler {
                              Label* if_miss, int min_feedback_capacity);
 
   // LoadIC implementation.
-  enum class OnNonExistent { kThrowReferenceError, kReturnUndefined };
   void HandleLoadICHandlerCase(
       const LoadICParameters* p, TNode<Object> handler, Label* miss,
       ExitPoint* exit_point, ICMode ic_mode = ICMode::kNonGlobalIC,

src/objects.cc

@@ -1010,7 +1010,8 @@ Maybe<bool> JSReceiver::HasOwnProperty(Handle<JSReceiver> object,
 }
 
 // static
-MaybeHandle<Object> Object::GetProperty(LookupIterator* it) {
+MaybeHandle<Object> Object::GetProperty(LookupIterator* it,
+                                        OnNonExistent on_non_existent) {
   for (; it->IsFound(); it->Next()) {
     switch (it->state()) {
       case LookupIterator::NOT_FOUND:
@@ -1044,6 +1045,12 @@ MaybeHandle<Object> Object::GetProperty(LookupIterator* it) {
         return it->GetDataValue();
     }
   }
+
+  if (on_non_existent == OnNonExistent::kThrowReferenceError) {
+    THROW_NEW_ERROR(it->isolate(),
+                    NewReferenceError(MessageTemplate::kNotDefined, it->name()),
+                    Object);
+  }
   return it->isolate()->factory()->undefined_value();
 }
 

src/objects.h

@@ -667,6 +667,8 @@ enum class ComparisonResult {
 // (Returns false whenever {result} is kUndefined.)
 bool ComparisonResultToBool(Operation op, ComparisonResult result);
 
+enum class OnNonExistent { kThrowReferenceError, kReturnUndefined };
+
 class AbstractCode;
 class AccessorPair;
 class AccessCheckInfo;
@@ -1150,7 +1152,8 @@ class Object {
       Isolate* isolate, Handle<Object> object, Handle<Object> callable);
 
   V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT static MaybeHandle<Object>
-  GetProperty(LookupIterator* it);
+  GetProperty(LookupIterator* it,
+              OnNonExistent on_non_existent = OnNonExistent::kReturnUndefined);
 
   // ES6 [[Set]] (when passed kDontThrow)
   // Invariants for this and related functions (unless stated otherwise):

src/runtime/runtime-proxy.cc

@@ -41,10 +41,11 @@ RUNTIME_FUNCTION(Runtime_JSProxyGetTarget) {
 RUNTIME_FUNCTION(Runtime_GetPropertyWithReceiver) {
   HandleScope scope(isolate);
 
-  DCHECK_EQ(3, args.length());
+  DCHECK_EQ(4, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSReceiver, holder, 0);
   CONVERT_ARG_HANDLE_CHECKED(Object, key, 1);
   CONVERT_ARG_HANDLE_CHECKED(Object, receiver, 2);
+  CONVERT_ARG_HANDLE_CHECKED(Smi, on_non_existent, 3);
 
   bool success = false;
   LookupIterator it = LookupIterator::PropertyOrElement(isolate, receiver, key,
@@ -53,7 +54,10 @@ RUNTIME_FUNCTION(Runtime_GetPropertyWithReceiver) {
     DCHECK(isolate->has_pending_exception());
     return ReadOnlyRoots(isolate).exception();
   }
-  RETURN_RESULT_OR_FAILURE(isolate, Object::GetProperty(&it));
+
+  RETURN_RESULT_OR_FAILURE(
+      isolate, Object::GetProperty(
+                   &it, static_cast<OnNonExistent>(on_non_existent->value())));
 }
 
 RUNTIME_FUNCTION(Runtime_SetPropertyWithReceiver) {

src/runtime/runtime.h

@@ -394,7 +394,7 @@ namespace internal {
 #define FOR_EACH_INTRINSIC_PROXY(F)   \
   F(CheckProxyGetSetTrapResult, 2, 1) \
   F(CheckProxyHasTrap, 2, 1)          \
-  F(GetPropertyWithReceiver, 2, 1)    \
+  F(GetPropertyWithReceiver, 3, 1)    \
   F(IsJSProxy, 1, 1)                  \
   F(JSProxyGetHandler, 1, 1)          \
   F(JSProxyGetTarget, 1, 1)           \

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

+// Copyright 2018 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.
+
+// Flags: --expose-gc
+
+this.__proto__ = new Proxy({}, {});
+
+function test1() {
+  eval("bla");
+}
+
+assertThrows(test1, ReferenceError);
+assertThrows(test1, ReferenceError);
+assertThrows(test1, ReferenceError);
+
+function test2() {
+  gc();
+  eval("bla");
+}
+
+assertThrows(test2, ReferenceError);
+assertThrows(test2, ReferenceError);
+assertThrows(test2, ReferenceError);
+
+function foo() {
+  try {
+    eval("bla");
+  } catch(e) {
+    return;
+  }
+  throw 1337;
+}
+
+function test3() {
+  gc();
+  foo();
+  foo();
+}
+
+assertDoesNotThrow(test3);
+assertDoesNotThrow(test3);
+assertDoesNotThrow(test3);