Fix SmiCompare on 64 bit to distinguish between comparisons where...

src/arm/codegen-arm.cc

 // Copyright 2010 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 5566 matching lines @@
   frame_->EmitPop(index2);
   frame_->EmitPop(index1);
   frame_->EmitPop(object);
 
   DeferredSwapElements* deferred =
       new DeferredSwapElements(object, index1, index2);
 
   // Fetch the map and check if array is in fast case.
   // Check that object doesn't require security checks and
   // has no indexed interceptor.
-  __ CompareObjectType(object, tmp1, tmp2, FIRST_JS_OBJECT_TYPE);
-  deferred->Branch(lt);
+  __ CompareObjectType(object, tmp1, tmp2, JS_ARRAY_TYPE);
+  deferred->Branch(ne);
   __ ldrb(tmp2, FieldMemOperand(tmp1, Map::kBitFieldOffset));
   __ tst(tmp2, Operand(KeyedLoadIC::kSlowCaseBitFieldMask));
   deferred->Branch(ne);
 
   // Check the object's elements are in fast case and writable.
   __ ldr(tmp1, FieldMemOperand(object, JSObject::kElementsOffset));
   __ ldr(tmp2, FieldMemOperand(tmp1, HeapObject::kMapOffset));
   __ LoadRoot(ip, Heap::kFixedArrayMapRootIndex);
   __ cmp(tmp2, ip);
   deferred->Branch(ne);
@@ skipping 1533 matching lines @@
     Register scratch1 = VirtualFrame::scratch0();
     Register scratch2 = VirtualFrame::scratch1();
     Register scratch3 = r3;
 
     // Counter will be decremented in the deferred code. Placed here to avoid
     // having it in the instruction stream below where patching will occur.
     __ IncrementCounter(&Counters::keyed_store_inline, 1,
                         scratch1, scratch2);
 
 
-
     // Load the value, key and receiver from the stack.
     bool value_is_harmless = frame_->KnownSmiAt(0);
     if (wb_info == NEVER_NEWSPACE) value_is_harmless = true;
     bool key_is_smi = frame_->KnownSmiAt(1);
     Register value = frame_->PopToRegister();
     Register key = frame_->PopToRegister(value);
     VirtualFrame::SpilledScope spilled(frame_);
     Register receiver = r2;
     frame_->EmitPop(receiver);
 
@@ skipping 27 matching lines @@
     }
 
     // Check that the receiver is a heap object.
     __ tst(receiver, Operand(kSmiTagMask));
     deferred->Branch(eq);
 
     // Check that the receiver is a JSArray.
     __ CompareObjectType(receiver, scratch1, scratch1, JS_ARRAY_TYPE);
     deferred->Branch(ne);
 
-    // Check that the key is within bounds. Both the key and the length of
-    // the JSArray are smis. Use unsigned comparison to handle negative keys.
-    __ ldr(scratch1, FieldMemOperand(receiver, JSArray::kLengthOffset));
-    __ cmp(scratch1, key);
-    deferred->Branch(ls);  // Unsigned less equal.
-
     // Get the elements array from the receiver.
     __ ldr(scratch1, FieldMemOperand(receiver, JSObject::kElementsOffset));
     if (!value_is_harmless && wb_info != LIKELY_SMI) {
       Label ok;
       __ and_(scratch2, scratch1, Operand(ExternalReference::new_space_mask()));
       __ cmp(scratch2, Operand(ExternalReference::new_space_start()));
       __ tst(value, Operand(kSmiTagMask), ne);
       deferred->Branch(ne);
 #ifdef DEBUG
       we_remembered_the_write_barrier = true;
 #endif
     }
     // Check that the elements array is not a dictionary.
     __ ldr(scratch2, FieldMemOperand(scratch1, JSObject::kMapOffset));
+
+    // Check that the key is within bounds. Both the key and the length of
+    // the JSArray are smis. Use unsigned comparison to handle negative keys.
+    __ ldr(scratch1, FieldMemOperand(receiver, JSArray::kLengthOffset));
+    __ cmp(scratch1, key);
+    deferred->Branch(ls);  // Unsigned less equal.
+

[Lasse Reichstein, 2011/03/15 09:07:01]

Add to the comment that we know the length is a smi because we tested the array
to have fast elements above.

[Lasse Reichstein, 2011/03/15 09:07:01]

This code now clobbers scratch1 which is assumed to be the elements fixed-array
below.

[Erik Corry, 2011/03/15 10:00:50]

Nice catch.  The tests missed this because this whole file is dead code in the
Cranshaft VM.

[Erik Corry, 2011/03/15 10:00:50]

Done.

     // The following instructions are the part of the inlined store keyed
     // property code which can be patched. Therefore the exact number of
     // instructions generated need to be fixed, so the constant pool is blocked
     // while generating this code.
     { Assembler::BlockConstPoolScope block_const_pool(masm_);
 #ifdef DEBUG
       Label check_inlined_codesize;
       masm_->bind(&check_inlined_codesize);
 #endif
 
@@ skipping 183 matching lines @@
                BinaryOpIC::GetName(runtime_operands_type_));
   return name_;
 }
 
 
 #undef __
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_ARM