More LookupIterator fixes after r65078

(1) One more place in ic.cc must guard against "lookup->name()" calls
when the LookupIterator might be in indexed mode.

(2) Rather than burdening LookupIterator users with specifying
"kGuaranteedNoTypedArray", we can do the corresponding calculation in
the LookupIterator itself, which makes it robust towards any callers
that haven't been updated (specifically, in Object.values).

Bug: chromium:1027461,chromium:1028213
Change-Id: I76b5d08e309fc2a694955b537adbeb5a30e681f7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1936474
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65177}

src/ic/ic.cc

@@ -742,7 +742,7 @@ Handle<Object> LoadIC::ComputeHandler(LookupIterator* lookup) {
   // `in` cannot be called on strings, and will always return true for string
   // wrapper length and function prototypes. The latter two cases are given
   // LoadHandler::LoadNativeDataProperty below.
-  if (!IsAnyHas()) {
+  if (!IsAnyHas() && !lookup->IsElement()) {
     if (receiver->IsString() && *lookup->name() == roots.length_string()) {
       TRACE_HANDLER_STATS(isolate(), LoadIC_StringLength);
       return BUILTIN_CODE(isolate(), LoadIC_StringLength);

src/json/json-parser.cc

@@ -648,8 +648,7 @@ Handle<Object> JsonParser<Char>::BuildJsonObject(
     DCHECK(!key->AsArrayIndex(&index));
 #endif
     Handle<Object> value = property.value;
-    LookupIterator it(isolate_, object, key, object,
-                      LookupIterator::OWN_NO_TYPEDARRAY);
+    LookupIterator it(isolate_, object, key, object, LookupIterator::OWN);
     JSObject::DefineOwnPropertyIgnoreAttributes(&it, value, NONE).Check();
   }
 

src/objects/lookup-inl.h

@@ -42,7 +42,10 @@ LookupIterator::LookupIterator(Isolate* isolate, Handle<Object> receiver,
       index_(kInvalidIndex) {
 #ifdef DEBUG
   // Assert that the name is not an index.
-  if (configuration & kGuaranteedNotTypedArray) {
+  // If we're not looking at the prototype chain and the initial holder is
+  // not a typed array, then this means "array index", otherwise we need to
+  // ensure the full generality so that typed arrays are handled correctly.
+  if (!check_prototype_chain() && !holder->IsJSTypedArray()) {
     uint32_t index;
     DCHECK(!name->AsArrayIndex(&index));
   } else {

src/objects/lookup.h

@@ -22,18 +22,10 @@ class V8_EXPORT_PRIVATE LookupIterator final {
     // Configuration bits.
     kInterceptor = 1 << 0,
     kPrototypeChain = 1 << 1,
-    // For lookups on arbitrary objects, keys that are strings representing
-    // integers larger than kMaxArrayIndex but still in size_t range must
-    // be treated as "elements" lookups, even though for non-TypedArray holders
-    // such keys are converted back to strings. For efficiency, we allow
-    // annotating use cases where it is guaranteed that no TypedArrays will be
-    // encountered by the LookupIterator, so they can run in "property" mode.
-    kGuaranteedNotTypedArray = 1 << 2,
 
     // Convenience combinations of bits.
     OWN_SKIP_INTERCEPTOR = 0,
     OWN = kInterceptor,
-    OWN_NO_TYPEDARRAY = OWN | kGuaranteedNotTypedArray,
     PROTOTYPE_CHAIN_SKIP_INTERCEPTOR = kPrototypeChain,
     PROTOTYPE_CHAIN = kPrototypeChain | kInterceptor,
     DEFAULT = PROTOTYPE_CHAIN

src/runtime/runtime-literals.cc

@@ -417,15 +417,14 @@ Handle<JSObject> CreateObjectLiteral(
       if (value->IsUninitialized(isolate)) {
         value = handle(Smi::zero(), isolate);
       }
-      LookupIterator it(isolate, boilerplate, element_index, boilerplate,
-                        LookupIterator::OWN_NO_TYPEDARRAY);
-      JSObject::DefineOwnPropertyIgnoreAttributes(&it, value, NONE).Check();
+      JSObject::SetOwnElementIgnoreAttributes(boilerplate, element_index, value,
+                                              NONE)
+          .Check();
     } else {
       Handle<String> name = Handle<String>::cast(key);
       DCHECK(!name->AsArrayIndex(&element_index));
-      LookupIterator it(boilerplate, name, boilerplate,
-                        LookupIterator::OWN_NO_TYPEDARRAY);
-      JSObject::DefineOwnPropertyIgnoreAttributes(&it, value, NONE).Check();
+      JSObject::SetOwnPropertyIgnoreAttributes(boilerplate, name, value, NONE)
+          .Check();
     }
   }
 

test/mjsunit/regress/large-indices.js

@@ -58,3 +58,42 @@
   var target = {};
   Reflect.set(target, key, value, receiver);
 })();
+
+// crbug.com/1028213
+(function() {
+  function load(obj, key) {
+    return obj[key];
+  }
+  %PrepareFunctionForOptimization(load);
+  let obj = function() {};
+  obj.__proto__ = new Int8Array(1);
+  let key = Object(4294967297);
+  for (let i = 0; i < 3; i++) {
+    load(obj, key);
+  }
+})();
+(function() {
+  function load(obj, key) {
+    return obj[key];
+  }
+  %PrepareFunctionForOptimization(load);
+  let obj = new String("abc");
+  obj.__proto__ = new Int8Array(1);
+  let key = Object(4294967297);
+  for (let i = 0; i < 3; i++) {
+    load(obj, key);
+  }
+})();
+
+// crbug.com/1027461#c12
+(function() {
+  let arr = new Int32Array(2);
+  Object.defineProperty(arr, "foo", {get:function() { this.valueOf = 1; }});
+  arr[9007199254740991] = 1;
+  Object.values(arr);
+
+  let obj = [1, 2, 3];
+  Object.defineProperty(obj, 2, {get:function() { this.valueOf = 1; }});
+  obj[9007199254740991] = 1;
+  Object.values(obj);
+})();