Allocation type info advice consumed in bailout path leads to assert failure.

src/builtins.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 191 matching lines @@
   bool no_caller_args = args.length() == 2;
   ASSERT(no_caller_args || args.length() == 3);
   int parameters_start = no_caller_args ? 0 : 1;
   Arguments* caller_args = no_caller_args
       ? &empty_args
       : reinterpret_cast<Arguments*>(args[0]);
   Handle<JSFunction> constructor = args.at<JSFunction>(parameters_start);
   Handle<Object> type_info = args.at<Object>(parameters_start + 1);
 
   bool holey = false;
-  if (caller_args->length() == 1 && (*caller_args)[0]->IsSmi()) {
-    int value = Smi::cast((*caller_args)[0])->value();
-    holey = (value > 0 && value < JSObject::kInitialMaxFastElementArray);
+  bool ignore_advice = false;

[danno, 2013/06/04 16:29:47]

ignore_type_feedback is probably more accurate.

[mvstanton, 2013/06/05 08:47:07]

Done.

+  if (caller_args->length() == 1) {
+    Object* argument_one = (*caller_args)[0];
+    if (argument_one->IsSmi()) {
+      int value = Smi::cast(argument_one)->value();
+      holey = (value > 0 && value < JSObject::kInitialMaxFastElementArray);

[danno, 2013/06/04 16:29:47]

Don't you mean >= 0? Or is the distinction important?

[mvstanton, 2013/06/05 08:47:07]

I think it's important because if someone calls new Array(0), they should get a
packed array instead of a holey one.

But your comment brought to light a bug in this new code. I would choose to
ignore type feedback if the value == 0, which is wrong. I've prepped a more
careful initialization of ignore_type_feedback.

+    }
+
+    // If we have a single argument, and we failed to set holey above,
+    // then we'll be forced down the dictionary path. In this case it's
+    // useless to consume allocation site info advice, even if we have it.
+    ignore_advice = !holey;
   }
 
   JSArray* array;
   MaybeObject* maybe_array;
   if (*type_info != isolate->heap()->undefined_value() &&
-      JSGlobalPropertyCell::cast(*type_info)->value()->IsSmi()) {
+      JSGlobalPropertyCell::cast(*type_info)->value()->IsSmi() &&
+      !ignore_advice) {
     JSGlobalPropertyCell* cell = JSGlobalPropertyCell::cast(*type_info);
     Smi* smi = Smi::cast(cell->value());
     ElementsKind to_kind = static_cast<ElementsKind>(smi->value());
     if (holey && !IsFastHoleyElementsKind(to_kind)) {
       to_kind = GetHoleyElementsKind(to_kind);
       // Update the allocation site info to reflect the advice alteration.
       cell->set_value(Smi::FromInt(to_kind));
     }
 
     maybe_array = isolate->heap()->AllocateJSObjectWithAllocationSite(
@@ skipping 1652 matching lines @@
   return Handle<Code>(code_address);                        \
 }
 BUILTIN_LIST_C(DEFINE_BUILTIN_ACCESSOR_C)
 BUILTIN_LIST_A(DEFINE_BUILTIN_ACCESSOR_A)
 BUILTIN_LIST_DEBUG_A(DEFINE_BUILTIN_ACCESSOR_A)
 #undef DEFINE_BUILTIN_ACCESSOR_C
 #undef DEFINE_BUILTIN_ACCESSOR_A
 
 
 } }  // namespace v8::internal