Optimizing code generator expects that every AST node is traversed once, i.e., the nodes are not ...

runtime/vm/opt_code_generator_ia32.cc

 // Copyright (c) 2011, 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_IA32.
 #if defined(TARGET_ARCH_IA32)
 
 #include "vm/opt_code_generator.h"
 
 #include "vm/assembler_macros.h"
@@ skipping 2465 matching lines @@
   }
   CodeGenerator::VisitLoadIndexedNode(node);
 }
 
 
 void OptimizingCodeGenerator::VisitStoreIndexedNode(StoreIndexedNode* node) {
   if (FLAG_enable_type_checks) {
     CodeGenerator::VisitStoreIndexedNode(node);
     return;
   }
-  node->array()->Visit(this);
+  Class& known_array_class = Class::Handle();
+  // Load array and release its CodeGenInfo as value may refer to the same
+  // array (e.g. in a[x] += 3). Fixes issue 1570.
+  {
+    CodeGenInfo array_info(node->array());
+    node->array()->Visit(this);
+    known_array_class = array_info.is_class()->raw();

[hausner, 2012/02/09 21:51:48]

Not sure why you call this the "known" array class. Isn't the known class the
one that the VM knows about, i.e. object_array_class?

[srdjan, 2012/02/10 07:13:11]

Changing it to class_of_this_array to avoid confusion. That class is computed
using type analysis and can be any of the classes including ObjectArray,
GrowableObjectArray, ImmutableArray or any user class. 

+  }
   // TODO(srdjan): Use VisitLoadTwo and check if index is smi (CodeGenInfo).
   ObjectStore* object_store = Isolate::Current()->object_store();
   const Class& object_array_class =
       Class::ZoneHandle(object_store->array_class());
   const ICData& ic_data = node->ICDataAtId(node->id());
   if (ic_data.NumberOfChecks() == 0) {
     VisitLoadTwo(node->index_expr(), node->value(), EBX, ECX);
     DeoptimizationBlob* deopt_blob =
         AddDeoptimizationBlob(node, EBX, ECX, kDeoptNoTypeFeedback);
     __ jmp(deopt_blob->label());
     return;
   }
 
-  CodeGenInfo array_info(node->array());
-  CodeGenInfo index_info(node->index_expr());
 
   if (AtIdNodeHasClassAt(node, node->id(), object_array_class, 0)) {
-    VisitLoadTwo(node->index_expr(), node->value(), EBX, ECX);
+    // Release CodeGenInfo of index quickly as it may be used in the value,
+    // e.g. a[i] += 3. Fixes issue 1570.
+    bool index_is_smi = false;
+    {
+      CodeGenInfo index_info(node->index_expr());
+      node->index_expr()->Visit(this);
+      index_is_smi = index_info.IsClass(smi_class_);
+    }
+    VisitLoadOne(node->value(), ECX);
     DeoptimizationBlob* deopt_blob =
         AddDeoptimizationBlob(node, EAX, EBX, ECX, kDeoptStoreIndexed);
+    __ popl(EBX);  // index.
     __ popl(EAX);  // array.
     // ECX: value, EBX:index, EAX: array.
     // Check class of array.
-    if (!array_info.IsClass(object_array_class)) {
+    if (known_array_class.raw() != object_array_class.raw()) {
       __ testl(EAX, Immediate(kSmiTagMask));
       __ j(ZERO, deopt_blob->label());  // Array is smi -> deopt.
       __ movl(EDX, FieldAddress(EAX, Object::class_offset()));
       __ CompareObject(EDX, object_array_class);
       __ j(NOT_EQUAL, deopt_blob->label());  // Not ObjectArray -> deopt.
       PropagateBackLocalClass(node->array(), object_array_class);
     }
     // Check class of index.
-    if (!index_info.IsClass(smi_class_)) {
+    if (!index_is_smi) {
       __ testl(EBX, Immediate(kSmiTagMask));
       __ j(NOT_ZERO, deopt_blob->label());  // Index not Smi -> deopt.
       PropagateBackLocalClass(node->index_expr(), smi_class_);
     }
     // Range check.
     __ cmpl(EBX, FieldAddress(EAX, Array::length_offset()));
     __ j(ABOVE_EQUAL, deopt_blob->label());  // Range error -> deopt.
     ASSERT(kSmiTagShift == 1);
     __ StoreIntoObject(EAX,
                        FieldAddress(EAX, EBX, TIMES_2, sizeof(RawArray)),
@@ skipping 10 matching lines @@
   ASSERT(!growable_array_class.IsNull());
   if (AtIdNodeHasClassAt(node, node->id(), growable_array_class, 0)) {
     const String& growable_array_length_field_name =
         String::Handle(String::NewSymbol(kGrowableArrayLengthFieldName));
     const String& growable_array_array_field_name =
         String::Handle(String::NewSymbol(kGrowableArrayArrayFieldName));
     intptr_t length_offset = GetFieldOffset(growable_array_class,
                                             growable_array_length_field_name);
     intptr_t array_offset = GetFieldOffset(growable_array_class,
                                            growable_array_array_field_name);
-    VisitLoadTwo(node->index_expr(), node->value(), EBX, ECX);
+    bool index_is_smi = false;
+    // Release CodeGenInfo of index quickly as it may be used in the value,
+    // e.g. a[i] += 3. Fixes issue 1570.
+    {
+      CodeGenInfo index_info(node->index_expr());
+      node->index_expr()->Visit(this);
+      index_is_smi = index_info.IsClass(smi_class_);
+    }
+    VisitLoadOne(node->value(), ECX);
     DeoptimizationBlob* deopt_blob =
         AddDeoptimizationBlob(node, EAX, EBX, ECX, kDeoptStoreIndexed);
-    __ popl(EAX);  // Array.
+    __ popl(EBX);  // index.
+    __ popl(EAX);  // array.
     // ECX: value, EBX:index, EAX: array, EDX: scratch.
     // Check class of array.
-    if (!array_info.IsClass(growable_array_class)) {
+    if (known_array_class.raw() != growable_array_class.raw()) {
       __ testl(EAX, Immediate(kSmiTagMask));
       __ j(ZERO, deopt_blob->label());  // Array is smi -> deopt.
       __ movl(EDX, FieldAddress(EAX, Object::class_offset()));
       __ CompareObject(EDX, growable_array_class);
       __ j(NOT_EQUAL, deopt_blob->label());  // Not GrowableObjectArray.
       PropagateBackLocalClass(node->array(), growable_array_class);
     }
     // Check class of index.
-    if (!index_info.IsClass(smi_class_)) {
+    if (!index_is_smi) {
       __ testl(EBX, Immediate(kSmiTagMask));
       __ j(NOT_ZERO, deopt_blob->label());  // Index not Smi -> deopt.
       PropagateBackLocalClass(node->index_expr(), smi_class_);
     }
     // Range check: deoptimize if out of bounds.
     __ cmpl(EBX, FieldAddress(EAX, length_offset));
     __ j(ABOVE_EQUAL, deopt_blob->label());
     __ movl(EDX, FieldAddress(EAX, array_offset));  // backingArray.
     // Note that EAX is Smi, i.e, times 2.
     ASSERT(kSmiTagShift == 1);
@@ skipping 547 matching lines @@
     }
   }
   // TODO(srdjan): Implement unary kSUB (negate) Mint.
   CodeGenerator::VisitUnaryOpNode(node);
 }
 
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_IA32