Do not try to patch type objects that are already canonical.

runtime/vm/snapshot.cc

Index: runtime/vm/snapshot.cc
diff --git a/runtime/vm/snapshot.cc b/runtime/vm/snapshot.cc
index b08e704d7b220ad08c1691e7c49f3b2a7c169cbb..0219aa8c257d09757bab1d34799e5234b0292136 100644
--- a/runtime/vm/snapshot.cc
+++ b/runtime/vm/snapshot.cc
@@ -1164,41 +1164,37 @@ void SnapshotReader::ProcessDeferredCanonicalizations() {
     BackRefNode& backref = (*backward_references_)[i];
     if (backref.defer_canonicalization()) {
       Object* objref = backref.reference();
-      bool needs_patching = false;
       // Object should either be an abstract type or a type argument.
       if (objref->IsType()) {
         typeobj ^= objref->raw();
         newobj = typeobj.Canonicalize();
-        if ((newobj.raw() != typeobj.raw()) && !typeobj.IsRecursive()) {
-          needs_patching = true;
-        } else {
-          // Set Canonical bit.
-          objref->SetCanonical();
-        }
       } else {
         ASSERT(objref->IsTypeArguments());
         typeargs ^= objref->raw();
         newobj = typeargs.Canonicalize();
-        if ((newobj.raw() != typeargs.raw()) && !typeargs.IsRecursive()) {
-          needs_patching = true;
-        } else {
-          // Set Canonical bit.
-          objref->SetCanonical();
-        }
       }
-      if (needs_patching) {
+      if (newobj.raw() != objref->raw()) {
         ZoneGrowableArray<intptr_t>* patches = backref.patch_records();
         ASSERT(newobj.IsCanonical());
         ASSERT(patches != NULL);
+        // First we replace the back ref table with the canonical object.
+        *objref = newobj.raw();
+        // Now we go over all the patch records and patch the canonical object.
         for (intptr_t j = 0; j < patches->length(); j+=2) {
           NoSafepointScope no_safepoint;
           intptr_t patch_object_id = (*patches)[j];
           intptr_t patch_offset = (*patches)[j + 1];
           Object* target = GetBackRef(patch_object_id);
-          RawObject** rawptr =
-              reinterpret_cast<RawObject**>(target->raw()->ptr());
-          target->StorePointer((rawptr + patch_offset), newobj.raw());
+          // We should not backpatch an object that is canonical.
+          if (!target->IsCanonical()) {
+            RawObject** rawptr =
+                reinterpret_cast<RawObject**>(target->raw()->ptr());
+            target->StorePointer((rawptr + patch_offset), newobj.raw());
+          }
         }
+      } else {
+        // Set Canonical bit.
+        objref->SetCanonical();

[regis, 2015/08/07 22:41:42]

Is this necessary? It should get set by the Canonicalize() call above.

[siva, 2015/08/07 23:30:07]

Changed it to an ASSERT.

       }
     }
   }