Add missing StoreIntoObject calls in generated code.

runtime/vm/stub_code_ia32.cc

Index: runtime/vm/stub_code_ia32.cc
===================================================================
--- runtime/vm/stub_code_ia32.cc	(revision 41331)
+++ runtime/vm/stub_code_ia32.cc	(working copy)
@@ -437,7 +437,7 @@
 // Input parameters:
 //   EDX: smi-tagged argument count, may be zero.
 //   EBP[kParamEndSlotFromFp + 1]: last argument.
-// Uses EAX, EBX, ECX, EDX.
+// Uses EAX, EBX, ECX, EDX, EDI.
 static void PushArgumentsArray(Assembler* assembler) {
   const Immediate& raw_null =
       Immediate(reinterpret_cast<intptr_t>(Object::null()));
@@ -459,8 +459,9 @@
   Label loop, loop_condition;
   __ jmp(&loop_condition, Assembler::kNearJump);
   __ Bind(&loop);
-  __ movl(EAX, Address(EBX, 0));
-  __ movl(Address(ECX, 0), EAX);
+  __ movl(EDI, Address(EBX, 0));
+  // No generational barrier needed, since array is in new space.
+  __ StoreIntoObjectNoBarrier(EAX, Address(ECX, 0), EDI);
   __ AddImmediate(ECX, Immediate(kWordSize));
   __ AddImmediate(EBX, Immediate(-kWordSize));
   __ Bind(&loop_condition);
@@ -744,7 +745,8 @@
   __ Bind(&init_loop);
   __ cmpl(EDI, EBX);
   __ j(ABOVE_EQUAL, &done, Assembler::kNearJump);
-  __ movl(Address(EDI, 0), raw_null);
+  // No generational barrier needed, since we are storing null.
+  __ StoreIntoObjectNoBarrier(EAX, Address(EDI, 0), Object::null_object());
   __ addl(EDI, Immediate(kWordSize));
   __ jmp(&init_loop, Assembler::kNearJump);
   __ Bind(&done);
@@ -988,12 +990,13 @@
     // EDX: number of context variables as integer value (not object).
     __ movl(FieldAddress(EAX, Context::num_variables_offset()), EDX);
 
-    const Immediate& raw_null =
-        Immediate(reinterpret_cast<intptr_t>(Object::null()));
     // Setup the parent field.
     // EAX: new object.
     // EDX: number of context variables.
-    __ movl(FieldAddress(EAX, Context::parent_offset()), raw_null);
+    // No generational barrier needed, since we are storing null.
+    __ StoreIntoObjectNoBarrier(EAX,

[Ivan Posva, 2014/10/29 07:58:41]

Here and other places: In the long run we will need an
InitializeIntoObjectNoBarrier or similar here which does not expect anything
legal to be in the location being stored. These are the initializing stores.

[koda, 2014/10/29 18:29:21]

Yes, I was aware of that, and that also applies to runtime code
(Object::StorePointer et al.).

I decided to wait with the initial/overwrite triage until a future CL because:

a) This CL is correct under the existing generational barrier, since it only
cares about object alignment

b) This CL will continue to be correct when adding the Verify calls for
StorePointer, since my plan is to Accept the entire area of newly allocated
objects. (Thus we'll verify illegal == illegal.)

The next steps would then be:

1. Start verifying that StoreSmi overwrites a Smi, and use the failures to
correctly distinguish initializing stores from overwrites.

2. Distinguish initializing pointer stores (can do this by looking for the zap
word).

If you insist, I can create the methods now (as simple forwarders), and do an
incomplete triage.

+                                FieldAddress(EAX, Context::parent_offset()),
+                                Object::null_object());
 
     // Initialize the context variables.
     // EAX: new object.
@@ -1005,7 +1008,10 @@
       __ jmp(&entry, Assembler::kNearJump);
       __ Bind(&loop);
       __ decl(EDX);
-      __ movl(Address(EBX, EDX, TIMES_4, 0), raw_null);
+      // No generational barrier needed, since we are storing null.
+      __ StoreIntoObjectNoBarrier(EAX,
+                                  Address(EBX, EDX, TIMES_4, 0),
+                                  Object::null_object());
       __ Bind(&entry);
       __ cmpl(EDX, Immediate(0));
       __ j(NOT_EQUAL, &loop, Assembler::kNearJump);
@@ -1158,7 +1164,7 @@
     __ movl(Address::Absolute(heap->TopAddress(space)), EBX);
     __ UpdateAllocationStats(cls.id(), ECX, space);
 
-    // EAX: new object start.
+    // EAX: new object start (untagged).
     // EBX: next object start.
     // EDX: new object type arguments (if is_cls_parameterized).
     // Set the tags.
@@ -1167,12 +1173,11 @@
     ASSERT(cls.id() != kIllegalCid);
     tags = RawObject::ClassIdTag::update(cls.id(), tags);
     __ movl(Address(EAX, Instance::tags_offset()), Immediate(tags));
+    __ addl(EAX, Immediate(kHeapObjectTag));
 
     // Initialize the remaining words of the object.
-    const Immediate& raw_null =
-        Immediate(reinterpret_cast<intptr_t>(Object::null()));
 
-    // EAX: new object start.
+    // EAX: new object (tagged).
     // EBX: next object start.
     // EDX: new object type arguments (if is_cls_parameterized).
     // First try inlining the initialization without a loop.
@@ -1182,12 +1187,14 @@
       for (intptr_t current_offset = Instance::NextFieldOffset();
            current_offset < instance_size;
            current_offset += kWordSize) {
-        __ movl(Address(EAX, current_offset), raw_null);
+        __ StoreIntoObjectNoBarrier(EAX,

[Ivan Posva, 2014/10/29 07:58:41]

ditto here and other places

+                                    FieldAddress(EAX, current_offset),
+                                    Object::null_object());
       }
     } else {
       __ leal(ECX, Address(EAX, Instance::NextFieldOffset()));
       // Loop until the whole object is initialized.
-      // EAX: new object.
+      // EAX: new object (tagged).
       // EBX: next object start.
       // ECX: next word to be initialized.
       // EDX: new object type arguments (if is_cls_parameterized).
@@ -1196,7 +1203,9 @@
       __ Bind(&init_loop);
       __ cmpl(ECX, EBX);
       __ j(ABOVE_EQUAL, &done, Assembler::kNearJump);
-      __ movl(Address(ECX, 0), raw_null);
+      __ StoreIntoObjectNoBarrier(EAX,
+                                  Address(ECX, 0),
+                                  Object::null_object());
       __ addl(ECX, Immediate(kWordSize));
       __ jmp(&init_loop, Assembler::kNearJump);
       __ Bind(&done);
@@ -1204,11 +1213,11 @@
     if (is_cls_parameterized) {
       // EDX: new object type arguments.
       // Set the type arguments in the new object.
-      __ movl(Address(EAX, cls.type_arguments_field_offset()), EDX);
+      intptr_t offset = cls.type_arguments_field_offset();
+      __ StoreIntoObjectNoBarrier(EAX, FieldAddress(EAX, offset), EDX);
     }
     // Done allocating and initializing the instance.
-    // EAX: new object.
-    __ addl(EAX, Immediate(kHeapObjectTag));
+    // EAX: new object (tagged).
     __ ret();
 
     __ Bind(&slow_case);
@@ -1369,7 +1378,7 @@
   __ addl(ECX, Immediate(Smi::RawValue(1)));
   __ movl(EDI, Immediate(Smi::RawValue(Smi::kMaxValue)));
   __ cmovno(EDI, ECX);
-  __ movl(Address(EBX, count_offset), EDI);
+  __ StoreIntoSmiField(Address(EBX, count_offset), EDI);
 
   __ ret();
 }
@@ -1517,7 +1526,7 @@
   __ addl(EAX, Immediate(Smi::RawValue(1)));
   __ movl(EDI, Immediate(Smi::RawValue(Smi::kMaxValue)));
   __ cmovno(EDI, EAX);
-  __ movl(Address(EBX, count_offset), EDI);
+  __ StoreIntoSmiField(Address(EBX, count_offset), EDI);
 
   __ movl(EAX, Address(EBX, target_offset));
   __ Bind(&call_target_function);