Make IC patching resilient to flushing of the original target() ic.

src/ic.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 941 matching lines @@
 bool IC::UpdatePolymorphicIC(State state,
                              StrictModeFlag strict_mode,
                              Handle<JSObject> receiver,
                              Handle<String> name,
                              Handle<Code> code) {
   if (code->type() == Code::NORMAL) return false;
   if (target()->ic_state() == MONOMORPHIC &&
       target()->type() == Code::NORMAL) {
     return false;
   }
+
   MapHandleList receiver_maps;
   CodeHandleList handlers;
-  target()->FindAllMaps(&receiver_maps);
-  int number_of_maps = receiver_maps.length();
-  if (number_of_maps == 0 || number_of_maps >= 4) return false;
 
-  target()->FindAllCode(&handlers, receiver_maps.length());
+  {
+    AssertNoAllocation no_gc;
+    target()->FindAllMaps(&receiver_maps);
+    int number_of_maps = receiver_maps.length();
+    if (number_of_maps >= 4) return false;
+
+    // Only allow 0 maps in case target() was reset to UNINITIALIZED by the GC.
+    // In that case, allow the IC to go back monomorphic.
+    if (number_of_maps == 0 && target()->ic_state() != UNINITIALIZED) {
+      return false;
+    }
+    target()->FindAllCode(&handlers, receiver_maps.length());
+  }
 
   if (!AddOneReceiverMapIfMissing(&receiver_maps,
                                   Handle<Map>(receiver->map()))) {
     return false;
   }
 
   handlers.Add(code);
   Handle<Code> ic = isolate()->stub_cache()->ComputePolymorphicIC(
       &receiver_maps, &handlers, name);
   set_target(*ic);
@@ skipping 14 matching lines @@
 void KeyedLoadIC::UpdateMonomorphicIC(Handle<JSObject> receiver,
                                       Handle<Code> handler,
                                       Handle<String> name) {
   if (handler->type() == Code::NORMAL) return set_target(*handler);
   Handle<Code> ic = isolate()->stub_cache()->ComputeKeyedMonomorphicIC(
       receiver, handler, name);
   set_target(*ic);
 }
 
 
+// Since GC may have been invoked, by the time PatchCache is called, |state| is
+// not necessarily equal to target()->state().
 void IC::PatchCache(State state,
                     StrictModeFlag strict_mode,
                     Handle<JSObject> receiver,
                     Handle<String> name,
                     Handle<Code> code) {
   switch (state) {
     case UNINITIALIZED:
     case PREMONOMORPHIC:
     case MONOMORPHIC_PROTOTYPE_FAILURE:
       UpdateMonomorphicIC(receiver, code, name);
       break;
     case MONOMORPHIC:
       // Only move to megamorphic if the target changes.
       if (target() != *code) {
         if (target()->is_load_stub()) {
           if (UpdatePolymorphicIC(state, strict_mode, receiver, name, code)) {
             break;
           }
         }
-        // We are transitioning from monomorphic to megamorphic case.  Place the
-        // stub compiled for the receiver into stub cache.
-        Map* map = target()->FindFirstMap();
-        if (map != NULL) {
-          UpdateMegamorphicCache(map, *name, target());
+        if (target()->type() != Code::NORMAL) {
+          AssertNoAllocation no_gc;
+          // We are transitioning from monomorphic to megamorphic case. Place
+          // the stub compiled for the receiver into stub cache.
+          Map* map;
+          Code* handler;
+          {
+            AssertNoAllocation no_gc;

[Jakob Kummerow, 2013/03/05 17:32:59]

This AssertNoAllocation (and its scope) seem unnecessary, since you have one on
line 1035 too.

[Toon Verwaest, 2013/03/05 17:37:14]

I removed the one above. The idea was to move that one down here, but I forgot
to remove it. UpdateMegamorphicCache could allocate, so the upper scope is
invalid.

On 2013/03/05 17:32:59, Jakob wrote:

+            map = target()->FindFirstMap();
+            if (map != NULL) {
+              if (target()->is_load_stub()) {
+                handler = target()->FindFirstCode();
+              } else {
+                handler = target();
+              }
+            } else {
+              // Avoid compiler warnings.
+              handler = NULL;
+            }
+          }
+          if (handler != NULL) {
+            UpdateMegamorphicCache(map, *name, handler);
+          }
         }
         UpdateMegamorphicCache(receiver->map(), *name, *code);
         set_target((strict_mode == kStrictMode)
                      ? *megamorphic_stub_strict()
                      : *megamorphic_stub());
       }
       break;
     case MEGAMORPHIC:
       // Update the stub cache.
       UpdateMegamorphicCache(receiver->map(), *name, *code);
       break;
     case POLYMORPHIC:
       if (target()->is_load_stub()) {
         if (UpdatePolymorphicIC(state, strict_mode, receiver, name, code)) {
           break;
         }
         MapHandleList receiver_maps;
         CodeHandleList handlers;
-        target()->FindAllMaps(&receiver_maps);
-        target()->FindAllCode(&handlers, receiver_maps.length());
+        {
+          AssertNoAllocation no_gc;
+          target()->FindAllMaps(&receiver_maps);
+          target()->FindAllCode(&handlers, receiver_maps.length());
+        }
         for (int i = 0; i < receiver_maps.length(); i++) {
           UpdateMegamorphicCache(*receiver_maps.at(i), *name, *handlers.at(i));
         }
         UpdateMegamorphicCache(receiver->map(), *name, *code);
         set_target(*megamorphic_stub());
       } else {
         // When trying to patch a polymorphic keyed load/store element stub
         // with anything other than another polymorphic stub, go generic.
         set_target((strict_mode == kStrictMode)
                    ? *generic_stub_strict()
@@ skipping 1585 matching lines @@
 #undef ADDR
 };
 
 
 Address IC::AddressFromUtilityId(IC::UtilityId id) {
   return IC_utilities[id];
 }
 
 
 } }  // namespace v8::internal