Tests for GuardField length check along with bug fixes

runtime/vm/intermediate_language_arm.cc

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/globals.h"  // Needed here to get TARGET_ARCH_ARM.
 #if defined(TARGET_ARCH_ARM)
 
 #include "vm/intermediate_language.h"
 
 #include "vm/dart_entry.h"
@@ skipping 1425 matching lines @@
   }
 }
 
 
 LocationSummary* GuardFieldInstr::MakeLocationSummary() const {
   const intptr_t kNumInputs = 1;
   LocationSummary* summary =
       new LocationSummary(kNumInputs, 0, LocationSummary::kNoCall);
   summary->set_in(0, Location::RequiresRegister());
   const bool field_has_length = field().needs_length_check();
-  const bool need_value_temp_reg =
-      (field_has_length || ((value()->Type()->ToCid() == kDynamicCid) &&
-                            (field().guarded_cid() != kSmiCid)));
-  if (need_value_temp_reg) {
-    summary->AddTemp(Location::RequiresRegister());
-    summary->AddTemp(Location::RequiresRegister());
-  }
+  summary->AddTemp(Location::RequiresRegister());
+  summary->AddTemp(Location::RequiresRegister());
   const bool need_field_temp_reg =
       field_has_length || (field().guarded_cid() == kIllegalCid);
   if (need_field_temp_reg) {
     summary->AddTemp(Location::RequiresRegister());
   }
   return summary;
 }
 
 
 void GuardFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
   const intptr_t field_cid = field().guarded_cid();
   const intptr_t nullability = field().is_nullable() ? kNullCid : kIllegalCid;
   const intptr_t field_length = field().guarded_list_length();
   const bool field_has_length = field().needs_length_check();
-  const bool needs_value_temp_reg =
-      (field_has_length || ((value()->Type()->ToCid() == kDynamicCid) &&
-                            (field().guarded_cid() != kSmiCid)));
   const bool needs_field_temp_reg =
       field_has_length || (field().guarded_cid() == kIllegalCid);
   if (field_has_length) {
     // Currently, we should only see final fields that remember length.
     ASSERT(field().is_final());
   }
 
   if (field_cid == kDynamicCid) {
     ASSERT(!compiler->is_optimizing());
     return;  // Nothing to emit.
   }
 
   const intptr_t value_cid = value()->Type()->ToCid();
 
   Register value_reg = locs()->in(0).reg();
 
-  Register value_cid_reg = needs_value_temp_reg ?
-      locs()->temp(0).reg() : kNoRegister;
-  Register temp_reg = needs_value_temp_reg ?
-      locs()->temp(1).reg() : kNoRegister;
+  Register value_cid_reg = locs()->temp(0).reg();
+
+  Register temp_reg = locs()->temp(1).reg();
 
   Register field_reg = needs_field_temp_reg ?
       locs()->temp(locs()->temp_count() - 1).reg() : kNoRegister;
 
   Label ok, fail_label;
 
   Label* deopt = compiler->is_optimizing() ?
       compiler->AddDeoptStub(deopt_id(), kDeoptGuardField) : NULL;
 
   Label* fail = (deopt != NULL) ? deopt : &fail_label;
@@ skipping 11 matching lines @@
     }
 
     __ LoadObject(field_reg, Field::ZoneHandle(field().raw()));
 
     FieldAddress field_cid_operand(field_reg, Field::guarded_cid_offset());
     FieldAddress field_nullability_operand(
         field_reg, Field::is_nullable_offset());
     FieldAddress field_length_operand(
         field_reg, Field::guarded_list_length_offset());
 
-    if (value_cid_reg == kNoRegister) {
-      ASSERT(!compiler->is_optimizing());
-      value_cid_reg = R3;
-      ASSERT((value_cid_reg != value_reg) && (field_reg != value_cid_reg));
-    }
+    ASSERT(value_cid_reg != kNoRegister);
+    ASSERT((value_cid_reg != value_reg) && (field_reg != value_cid_reg));
 
     if (value_cid == kDynamicCid) {
       LoadValueCid(compiler, value_cid_reg, value_reg);
       Label skip_length_check;
       __ ldr(IP, field_cid_operand);
       __ cmp(value_cid_reg, ShifterOperand(IP));
       __ b(&skip_length_check, NE);
       if (field_has_length) {
         ASSERT(temp_reg != kNoRegister);
         // Field guard may have remembered list length, check it.
         if ((field_cid == kArrayCid) || (field_cid == kImmutableArrayCid)) {
           __ ldr(temp_reg,
                  FieldAddress(value_reg, Array::length_offset()));
-          __ CompareImmediate(temp_reg, field_length);
+          __ CompareImmediate(temp_reg, Smi::RawValue(field_length));
         } else if (RawObject::IsTypedDataClassId(field_cid)) {
           __ ldr(temp_reg,
                  FieldAddress(value_reg, TypedData::length_offset()));
-          __ CompareImmediate(temp_reg, field_length);
+          __ CompareImmediate(temp_reg, Smi::RawValue(field_length));
         } else {
           ASSERT(field_cid == kIllegalCid);
+          ASSERT(field_length == Field::kUnknownFixedLength);
+          // At compile time we do not know the type of the field nor its
+          // length. At execution time we may have set the class id and
+          // list length so we compare the guarded length with the
+          // list length here, without this check the list length could change
+          // without triggering a deoptimization.
+          Label check_array, length_compared, no_fixed_length;
+          __ CompareImmediate(value_cid_reg, kNullCid);
+          __ b(&no_fixed_length, EQ);
+          // Check for typed data array.
+          __ CompareImmediate(value_cid_reg, kTypedDataFloat32x4ArrayCid);
+          __ b(&no_fixed_length, GT);
+          __ CompareImmediate(value_cid_reg, kTypedDataInt8ArrayCid);
+          // Could still be a regular array.
+          __ b(&check_array, LT);
+          __ ldr(temp_reg,
+                 FieldAddress(value_reg, TypedData::length_offset()));
+          __ ldr(IP, field_length_operand);
+          __ cmp(temp_reg, ShifterOperand(IP));
+          __ b(&length_compared);
+          // Check for regular array.
+          __ Bind(&check_array);
+          __ CompareImmediate(value_cid_reg, kImmutableArrayCid);
+          __ b(&no_fixed_length, GT);
+          __ CompareImmediate(value_cid_reg, kArrayCid);
+          __ b(&no_fixed_length, LT);
+          __ ldr(temp_reg,
+                 FieldAddress(value_reg, Array::length_offset()));
+          __ ldr(IP, field_length_operand);
+          __ cmp(temp_reg, ShifterOperand(IP));
+          __ b(&length_compared);
+          __ Bind(&no_fixed_length);
+          __ b(fail);
+          __ Bind(&length_compared);
           // Following branch cannot not occur, fall through.
         }
         __ b(fail, NE);
       }
       __ Bind(&skip_length_check);
       __ ldr(IP, field_nullability_operand);
       __ cmp(value_cid_reg, ShifterOperand(IP));
     } else if (value_cid == kNullCid) {
       __ ldr(value_cid_reg, field_nullability_operand);
       __ CompareImmediate(value_cid_reg, value_cid);
     } else {
       Label skip_length_check;
       __ ldr(value_cid_reg, field_cid_operand);
       __ CompareImmediate(value_cid_reg, value_cid);
       __ b(&skip_length_check, NE);
       if (field_has_length) {
         ASSERT(value_cid_reg != kNoRegister);
         ASSERT(temp_reg != kNoRegister);
-        if ((field_cid == kArrayCid) || (field_cid == kImmutableArrayCid)) {
+        if ((value_cid == kArrayCid) || (value_cid == kImmutableArrayCid)) {
           __ ldr(temp_reg,
                   FieldAddress(value_reg, Array::length_offset()));
-          __ CompareImmediate(temp_reg, field_length);
-        } else if (RawObject::IsTypedDataClassId(field_cid)) {
+          __ CompareImmediate(temp_reg, Smi::RawValue(field_length));
+        } else if (RawObject::IsTypedDataClassId(value_cid)) {
           __ ldr(temp_reg,
                   FieldAddress(value_reg, TypedData::length_offset()));
-          __ CompareImmediate(temp_reg, field_length);
+          __ CompareImmediate(temp_reg, Smi::RawValue(field_length));
+        } else if (field_cid != kIllegalCid) {
+          ASSERT(field_cid != value_cid);
+          ASSERT(field_length >= 0);
+          // Field has a known class id and length. At compile time it is
+          // known that the value's class id is not a fixed length list.
+          __ b(fail);
         } else {
           ASSERT(field_cid == kIllegalCid);
+          ASSERT(field_length == Field::kUnknownFixedLength);
           // Following jump cannot not occur, fall through.
         }
+        __ b(fail, NE);
       }
       // Not identical, possibly null.
       __ Bind(&skip_length_check);
     }
     __ b(&ok, EQ);
 
     __ ldr(IP, field_cid_operand);
     __ CompareImmediate(IP, kIllegalCid);
     __ b(fail, NE);
 
     if (value_cid == kDynamicCid) {
       __ str(value_cid_reg, field_cid_operand);
       __ str(value_cid_reg, field_nullability_operand);
       if (field_has_length) {
-        Label check_array, local_exit, local_fail;
+        Label check_array, length_set, no_fixed_length;
         __ CompareImmediate(value_cid_reg, kNullCid);
-        __ b(&local_fail, EQ);
+        __ b(&no_fixed_length, EQ);
         // Check for typed data array.
         __ CompareImmediate(value_cid_reg, kTypedDataFloat32x4ArrayCid);
-        __ b(&local_fail, GT);
+        __ b(&no_fixed_length, GT);
         __ CompareImmediate(value_cid_reg, kTypedDataInt8ArrayCid);
-        __ b(&check_array, LT);  // Could still be a regular array.
+        // Could still be a regular array.
+        __ b(&check_array, LT);
         // Destroy value_cid_reg (safe because we are finished with it).
         __ ldr(value_cid_reg,
                FieldAddress(value_reg, TypedData::length_offset()));
         __ str(value_cid_reg, field_length_operand);
-        __ b(&local_exit);  // Updated field length typed data array.
+        __ b(&length_set);  // Updated field length typed data array.
         // Check for regular array.
         __ Bind(&check_array);
         __ CompareImmediate(value_cid_reg, kImmutableArrayCid);
-        __ b(&local_fail, GT);
+        __ b(&no_fixed_length, GT);
         __ CompareImmediate(value_cid_reg, kArrayCid);
-        __ b(&local_fail, LT);
+        __ b(&no_fixed_length, LT);
         // Destroy value_cid_reg (safe because we are finished with it).
         __ ldr(value_cid_reg,
                FieldAddress(value_reg, Array::length_offset()));
         __ str(value_cid_reg, field_length_operand);
-        __ b(&local_exit);  // Updated field length from regular array.
-
-        __ Bind(&local_fail);
-        __ LoadImmediate(IP, Field::kNoFixedLength);
+        // Updated field length from regular array.
+        __ b(&length_set);
+        __ Bind(&no_fixed_length);
+        __ LoadImmediate(IP, Smi::RawValue(Field::kNoFixedLength));
         __ str(IP, field_length_operand);
-
-        __ Bind(&local_exit);
+        __ Bind(&length_set);
       }
     } else {
       __ LoadImmediate(IP, value_cid);
       __ str(IP, field_cid_operand);
       __ str(IP, field_nullability_operand);
-      if ((value_cid == kArrayCid) || (value_cid == kImmutableArrayCid)) {
-        // Destroy value_cid_reg (safe because we are finished with it).
-        __ ldr(value_cid_reg,
-               FieldAddress(value_reg, Array::length_offset()));
-        __ str(value_cid_reg, field_length_operand);
-      } else if (RawObject::IsTypedDataClassId(value_cid)) {
-        // Destroy value_cid_reg (safe because we are finished with it).
-        __ ldr(value_cid_reg,
-                FieldAddress(value_reg, TypedData::length_offset()));
-        __ str(value_cid_reg, field_length_operand);
-      } else {
-        __ LoadImmediate(IP, Field::kNoFixedLength);
-        __ str(IP, field_length_operand);
+      if (field_has_length) {
+        if ((value_cid == kArrayCid) || (value_cid == kImmutableArrayCid)) {
+          // Destroy value_cid_reg (safe because we are finished with it).
+          __ ldr(value_cid_reg,
+                 FieldAddress(value_reg, Array::length_offset()));
+          __ str(value_cid_reg, field_length_operand);
+        } else if (RawObject::IsTypedDataClassId(value_cid)) {
+          // Destroy value_cid_reg (safe because we are finished with it).
+          __ ldr(value_cid_reg,
+                  FieldAddress(value_reg, TypedData::length_offset()));
+          __ str(value_cid_reg, field_length_operand);
+        } else {
+          __ LoadImmediate(IP, Smi::RawValue(Field::kNoFixedLength));
+          __ str(IP, field_length_operand);
+        }
       }
     }
-
     if (!ok_is_fall_through) {
       __ b(&ok);
     }
   } else {
     if (field_reg != kNoRegister) {
       __ LoadObject(field_reg, Field::ZoneHandle(field().raw()));
     }
     if (value_cid == kDynamicCid) {
       // Field's guarded class id is fixed by value's class id is not known.
       __ tst(value_reg, ShifterOperand(kSmiTagMask));
@@ skipping 2985 matching lines @@
   compiler->GenerateCall(token_pos(),
                          &label,
                          PcDescriptors::kOther,
                          locs());
   __ Drop(2);  // Discard type arguments and receiver.
 }
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_ARM