Revision: 5252
Author: [email protected]
Date: Thu Aug 12 06:43:08 2010
Log: Handle overwriting valueOf on String objects correctly when adding

This adds a check to the fast case string add to ensure that the String object still have the default valueOf function. The default valueOf is sitting on a hidden prototype of String.prototype.

Before using the fast case valueOf the object is checked for a local valueOf property. For slow case objects this check always reports true (the dictionary is not probed, so valueOf might be there) and for fast case objects the descriptor array is checked for the valueOf symbol (just liniar scan). After that the prototype is checked for beeing the initial value of String.prototype. If this all pass (that is the default valueOf is still in place) this result is cached on the map making the check fast the next time.

This is only implemented in the optimizing compiler, as the two usages of %_IsStringWrapperSafeForDefaultValueOf is never hit by the full compiler.

I will port to x64 and ARM when this has been reviewed for ia32.

I will remove the performance counters prior to final commit.

BUG=http://code.google.com/p/v8/issues/detail?id=760
TEST=test/mjsunit/regress/regress-760-1.js
TEST=test/mjsunit/regress/regress-760-2.js

Review URL: http://codereview.chromium.org/3117006
http://code.google.com/p/v8/source/detail?r=5252

Added:
 /branches/bleeding_edge/test/mjsunit/regress/regress-760-1.js
 /branches/bleeding_edge/test/mjsunit/regress/regress-760-2.js
Modified:
 /branches/bleeding_edge/src/arm/codegen-arm.cc
 /branches/bleeding_edge/src/arm/codegen-arm.h
 /branches/bleeding_edge/src/arm/full-codegen-arm.cc
 /branches/bleeding_edge/src/arm/macro-assembler-arm.cc
 /branches/bleeding_edge/src/arm/macro-assembler-arm.h
 /branches/bleeding_edge/src/bootstrapper.cc
 /branches/bleeding_edge/src/codegen.h
 /branches/bleeding_edge/src/contexts.h
 /branches/bleeding_edge/src/full-codegen.cc
 /branches/bleeding_edge/src/full-codegen.h
 /branches/bleeding_edge/src/ia32/codegen-ia32.cc
 /branches/bleeding_edge/src/ia32/codegen-ia32.h
 /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
 /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc
 /branches/bleeding_edge/src/objects.h
 /branches/bleeding_edge/src/runtime.js
 /branches/bleeding_edge/src/x64/codegen-x64.cc
 /branches/bleeding_edge/src/x64/codegen-x64.h
 /branches/bleeding_edge/src/x64/full-codegen-x64.cc
 /branches/bleeding_edge/src/x64/macro-assembler-x64.cc
 /branches/bleeding_edge/src/x64/macro-assembler-x64.h

=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-760-1.js Thu Aug 12 06:43:08 2010
@@ -0,0 +1,49 @@
+// 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
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Check that when valueOf for a String object is overwritten it is called and
+// the result used when that object is added with a string.
+
+// See: http://code.google.com/p/v8/issues/detail?id=760
+
+String.prototype.valueOf = function() { return 'y' };
+
+function test() {
+  var o = Object('x');
+  assertEquals('y', o + '');
+  assertEquals('y', '' + o);
+}
+
+for (var i = 0; i < 10; i++) {
+  var o = Object('x');
+  assertEquals('y', o + '');
+  assertEquals('y', '' + o);
+}
+
+for (var i = 0; i < 10; i++) {
+  test()
+}
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-760-2.js Thu Aug 12 06:43:08 2010
@@ -0,0 +1,49 @@
+// 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
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Check that when valueOf for a String object is overwritten it is called and
+// the result used when that object is added with a string.
+
+// See: http://code.google.com/p/v8/issues/detail?id=760
+
+function test() {
+  var o = Object('x');
+  o.valueOf = function() { return 'y' };
+  assertEquals('y', o + '');
+  assertEquals('y', '' + o);
+}
+
+for (var i = 0; i < 10; i++) {
+  var o = Object('x');
+  o.valueOf = function() { return 'y' };
+  assertEquals('y', o + '');
+  assertEquals('y', '' + o);
+}
+
+for (var i = 0; i < 10; i++) {
+  test()
+}
=======================================
--- /branches/bleeding_edge/src/arm/codegen-arm.cc      Wed Aug 11 06:46:10 2010
+++ /branches/bleeding_edge/src/arm/codegen-arm.cc      Thu Aug 12 06:43:08 2010
@@ -4784,6 +4784,149 @@
   __ cmp(value, Operand(FIRST_JS_OBJECT_TYPE));
   cc_reg_ = ge;
 }
+
+
+// Deferred code to check whether the String JavaScript object is safe for using +// default value of. This code is called after the bit caching this information +// in the map has been checked with the map for the object in the map_result_ +// register. On return the register map_result_ contains 1 for true and 0 for
+// false.
+class DeferredIsStringWrapperSafeForDefaultValueOf : public DeferredCode {
+ public:
+  DeferredIsStringWrapperSafeForDefaultValueOf(Register object,
+                                               Register map_result,
+                                               Register scratch1,
+                                               Register scratch2)
+      : object_(object),
+        map_result_(map_result),
+        scratch1_(scratch1),
+        scratch2_(scratch2) { }
+
+  virtual void Generate() {
+    Label false_result;
+
+    // Check that map is loaded as expected.
+    if (FLAG_debug_code) {
+      __ ldr(ip, FieldMemOperand(object_, HeapObject::kMapOffset));
+      __ cmp(map_result_, ip);
+      __ Assert(eq, "Map not in expected register");
+    }
+
+ // Check for fast case object. Generate false result for slow case object. + __ ldr(scratch1_, FieldMemOperand(object_, JSObject::kPropertiesOffset));
+    __ ldr(scratch1_, FieldMemOperand(scratch1_, HeapObject::kMapOffset));
+    __ LoadRoot(ip, Heap::kHashTableMapRootIndex);
+    __ cmp(scratch1_, ip);
+    __ b(eq, &false_result);
+
+ // Look for valueOf symbol in the descriptor array, and indicate false if + // found. The type is not checked, so if it is a transition it is a false
+    // negative.
+    __ ldr(map_result_,
+           FieldMemOperand(map_result_, Map::kInstanceDescriptorsOffset));
+ __ ldr(scratch2_, FieldMemOperand(map_result_, FixedArray::kLengthOffset));
+    // map_result_: descriptor array
+    // scratch2_: length of descriptor array
+    // Calculate the end of the descriptor array.
+    STATIC_ASSERT(kSmiTag == 0);
+    STATIC_ASSERT(kSmiTagSize == 1);
+    STATIC_ASSERT(kPointerSize == 4);
+    __ add(scratch1_,
+           map_result_,
+           Operand(FixedArray::kHeaderSize - kHeapObjectTag));
+    __ add(scratch1_,
+           scratch1_,
+           Operand(scratch2_, LSL, kPointerSizeLog2 - kSmiTagSize));
+
+    // Calculate location of the first key name.
+    __ add(map_result_,
+           map_result_,
+           Operand(FixedArray::kHeaderSize +
+                   DescriptorArray::kFirstIndex * kPointerSize));
+ // Loop through all the keys in the descriptor array. If one of these is the
+    // symbol valueOf the result is false.
+    Label entry, loop;
+    __ jmp(&entry);
+    __ bind(&loop);
+    __ ldr(scratch2_, FieldMemOperand(map_result_, 0));
+    __ cmp(scratch2_, Operand(Factory::value_of_symbol()));
+    __ b(eq, &false_result);
+    __ add(map_result_, map_result_, Operand(kPointerSize));
+    __ bind(&entry);
+    __ cmp(map_result_, Operand(scratch1_));
+    __ b(ne, &loop);
+
+    // Reload map as register map_result_ was used as temporary above.
+    __ ldr(map_result_, FieldMemOperand(object_, HeapObject::kMapOffset));
+
+    // If a valueOf property is not found on the object check that it's
+ // prototype is the un-modified String prototype. If not result is false.
+    __ ldr(scratch1_, FieldMemOperand(map_result_, Map::kPrototypeOffset));
+    __ tst(scratch1_, Operand(kSmiTagMask));
+    __ b(eq, &false_result);
+    __ ldr(scratch1_, FieldMemOperand(scratch1_, HeapObject::kMapOffset));
+    __ ldr(scratch2_,
+           CodeGenerator::ContextOperand(cp, Context::GLOBAL_INDEX));
+    __ ldr(scratch2_,
+           FieldMemOperand(scratch2_, GlobalObject::kGlobalContextOffset));
+    __ ldr(scratch2_,
+           CodeGenerator::ContextOperand(
+               scratch2_, Context::STRING_FUNCTION_PROTOTYPE_MAP_INDEX));
+    __ cmp(scratch1_, scratch2_);
+    __ b(ne, &false_result);
+
+    // Set the bit in the map to indicate that it has been checked safe for
+    // default valueOf and set true result.
+    __ ldr(scratch1_, FieldMemOperand(map_result_, Map::kBitField2Offset));
+    __ orr(scratch1_,
+           scratch1_,
+           Operand(1 << Map::kStringWrapperSafeForDefaultValueOf));
+    __ str(scratch1_, FieldMemOperand(map_result_, Map::kBitField2Offset));
+    __ mov(map_result_, Operand(1));
+    __ jmp(exit_label());
+    __ bind(&false_result);
+    // Set false result.
+    __ mov(map_result_, Operand(0));
+  }
+
+ private:
+  Register object_;
+  Register map_result_;
+  Register scratch1_;
+  Register scratch2_;
+};
+
+
+void CodeGenerator::GenerateIsStringWrapperSafeForDefaultValueOf(
+    ZoneList<Expression*>* args) {
+  ASSERT(args->length() == 1);
+  Load(args->at(0));
+  Register obj = frame_->PopToRegister();  // Pop the string wrapper.
+  if (FLAG_debug_code) {
+    __ AbortIfSmi(obj);
+  }
+
+  // Check whether this map has already been checked to be safe for default
+  // valueOf.
+  Register map_result = VirtualFrame::scratch0();
+  __ ldr(map_result, FieldMemOperand(obj, HeapObject::kMapOffset));
+  __ ldrb(ip, FieldMemOperand(map_result, Map::kBitField2Offset));
+  __ tst(ip, Operand(1 << Map::kStringWrapperSafeForDefaultValueOf));
+  true_target()->Branch(ne);
+
+  // We need an additional two scratch registers for the deferred code.
+  Register scratch1 = VirtualFrame::scratch1();
+  // Use r6 without notifying the virtual frame.
+  Register scratch2 = r6;
+
+  DeferredIsStringWrapperSafeForDefaultValueOf* deferred =
+      new DeferredIsStringWrapperSafeForDefaultValueOf(
+          obj, map_result, scratch1, scratch2);
+  deferred->Branch(eq);
+  deferred->BindExit();
+  __ tst(map_result, Operand(map_result));
+  cc_reg_ = ne;
+}


 void CodeGenerator::GenerateIsFunction(ZoneList<Expression*>* args) {
=======================================
--- /branches/bleeding_edge/src/arm/codegen-arm.h       Fri Aug  6 06:04:27 2010
+++ /branches/bleeding_edge/src/arm/codegen-arm.h       Thu Aug 12 06:43:08 2010
@@ -285,6 +285,10 @@
     ASSERT(inlined_write_barrier_size_ != -1);
     return inlined_write_barrier_size_ + 4;
   }
+
+  static MemOperand ContextOperand(Register context, int index) {
+    return MemOperand(context, Context::SlotOffset(index));
+  }

  private:
   // Construction/Destruction
@@ -338,10 +342,6 @@
   void LoadReference(Reference* ref);
   void UnloadReference(Reference* ref);

-  static MemOperand ContextOperand(Register context, int index) {
-    return MemOperand(context, Context::SlotOffset(index));
-  }
-
   MemOperand SlotOperand(Slot* slot, Register tmp);

   MemOperand ContextSlotOperandCheckExtensions(Slot* slot,
@@ -482,6 +482,8 @@
   void GenerateIsSpecObject(ZoneList<Expression*>* args);
   void GenerateIsFunction(ZoneList<Expression*>* args);
   void GenerateIsUndetectableObject(ZoneList<Expression*>* args);
+  void GenerateIsStringWrapperSafeForDefaultValueOf(
+      ZoneList<Expression*>* args);

   // Support for construct call checks.
   void GenerateIsConstructCall(ZoneList<Expression*>* args);
=======================================
--- /branches/bleeding_edge/src/arm/full-codegen-arm.cc Wed Aug 11 06:46:10 2010 +++ /branches/bleeding_edge/src/arm/full-codegen-arm.cc Thu Aug 12 06:43:08 2010
@@ -1955,6 +1955,26 @@

   Apply(context_, if_true, if_false);
 }
+
+
+void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf(
+    ZoneList<Expression*>* args) {
+
+  ASSERT(args->length() == 1);
+
+  VisitForValue(args->at(0), kAccumulator);
+
+  Label materialize_true, materialize_false;
+  Label* if_true = NULL;
+  Label* if_false = NULL;
+  PrepareTest(&materialize_true, &materialize_false, &if_true, &if_false);
+
+ // Just indicate false, as %_IsStringWrapperSafeForDefaultValueOf() is only + // used in a few functions in runtime.js which should not normally be hit by
+  // this compiler.
+  __ jmp(if_false);
+  Apply(context_, if_true, if_false);
+}


 void FullCodeGenerator::EmitIsFunction(ZoneList<Expression*>* args) {
=======================================
--- /branches/bleeding_edge/src/arm/macro-assembler-arm.cc Wed Aug 11 01:12:53 2010 +++ /branches/bleeding_edge/src/arm/macro-assembler-arm.cc Thu Aug 12 06:43:08 2010
@@ -1653,6 +1653,13 @@
   tst(reg2, Operand(kSmiTagMask), ne);
   b(eq, on_either_smi);
 }
+
+
+void MacroAssembler::AbortIfSmi(Register object) {
+  ASSERT_EQ(0, kSmiTag);
+  tst(object, Operand(kSmiTagMask));
+  Assert(ne, "Operand is a smi");
+}


 void MacroAssembler::JumpIfNonSmisNotBothSequentialAsciiStrings(
=======================================
--- /branches/bleeding_edge/src/arm/macro-assembler-arm.h Fri Aug 6 06:04:27 2010 +++ /branches/bleeding_edge/src/arm/macro-assembler-arm.h Thu Aug 12 06:43:08 2010
@@ -618,6 +618,9 @@
   // Jump if either of the registers contain a smi.
   void JumpIfEitherSmi(Register reg1, Register reg2, Label* on_either_smi);

+  // Abort execution if argument is a smi. Used in debug code.
+  void AbortIfSmi(Register object);
+
// ---------------------------------------------------------------------------
   // String utilities

=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Wed Aug 11 07:30:14 2010
+++ /branches/bleeding_edge/src/bootstrapper.cc Thu Aug 12 06:43:08 2010
@@ -1234,6 +1234,14 @@

   InstallNativeFunctions();

+ // Store the map for the string prototype after the natives has been compiled
+  // and the String function has been setup.
+  Handle<JSFunction> string_function(global_context()->string_function());
+  ASSERT(JSObject::cast(
+      string_function->initial_map()->prototype())->HasFastProperties());
+  global_context()->set_string_function_prototype_map(
+ HeapObject::cast(string_function->initial_map()->prototype())->map());
+
   InstallCustomCallGenerators();

   // Install Function.prototype.call and apply.
=======================================
--- /branches/bleeding_edge/src/codegen.h       Mon Aug  9 06:12:02 2010
+++ /branches/bleeding_edge/src/codegen.h       Thu Aug 12 06:43:08 2010
@@ -101,7 +101,8 @@
F(IsObject, 1, 1) \ F(IsFunction, 1, 1) \ F(IsUndetectableObject, 1, 1) \
-  F(IsSpecObject, 1, 1)                                            \
+ F(IsSpecObject, 1, 1) \ + F(IsStringWrapperSafeForDefaultValueOf, 1, 1) \ F(StringAdd, 2, 1) \ F(SubString, 3, 1) \ F(StringCompare, 2, 1) \
=======================================
--- /branches/bleeding_edge/src/contexts.h      Mon Aug  2 08:08:17 2010
+++ /branches/bleeding_edge/src/contexts.h      Thu Aug 12 06:43:08 2010
@@ -56,6 +56,7 @@
   V(BOOLEAN_FUNCTION_INDEX, JSFunction, boolean_function) \
   V(NUMBER_FUNCTION_INDEX, JSFunction, number_function) \
   V(STRING_FUNCTION_INDEX, JSFunction, string_function) \
+ V(STRING_FUNCTION_PROTOTYPE_MAP_INDEX, Map, string_function_prototype_map) \
   V(OBJECT_FUNCTION_INDEX, JSFunction, object_function) \
   V(ARRAY_FUNCTION_INDEX, JSFunction, array_function) \
   V(DATE_FUNCTION_INDEX, JSFunction, date_function) \
@@ -186,6 +187,7 @@
     BOOLEAN_FUNCTION_INDEX,
     NUMBER_FUNCTION_INDEX,
     STRING_FUNCTION_INDEX,
+    STRING_FUNCTION_PROTOTYPE_MAP_INDEX,
     OBJECT_FUNCTION_INDEX,
     ARRAY_FUNCTION_INDEX,
     DATE_FUNCTION_INDEX,
=======================================
--- /branches/bleeding_edge/src/full-codegen.cc Fri Aug  6 06:04:27 2010
+++ /branches/bleeding_edge/src/full-codegen.cc Thu Aug 12 06:43:08 2010
@@ -919,6 +919,9 @@
     EmitGetFromCache(expr->arguments());
   } else if (strcmp("_IsRegExpEquivalent", *name->ToCString()) == 0) {
     EmitIsRegExpEquivalent(expr->arguments());
+  } else if (strcmp("_IsStringWrapperSafeForDefaultValueOf",
+                    *name->ToCString()) == 0) {
+    EmitIsStringWrapperSafeForDefaultValueOf(expr->arguments());
   } else {
     UNREACHABLE();
   }
=======================================
--- /branches/bleeding_edge/src/full-codegen.h  Fri Aug  6 06:04:27 2010
+++ /branches/bleeding_edge/src/full-codegen.h  Thu Aug 12 06:43:08 2010
@@ -408,6 +408,8 @@
   void EmitIsArray(ZoneList<Expression*>* arguments);
   void EmitIsRegExp(ZoneList<Expression*>* arguments);
   void EmitIsConstructCall(ZoneList<Expression*>* arguments);
+  void EmitIsStringWrapperSafeForDefaultValueOf(
+      ZoneList<Expression*>* arguments);
   void EmitObjectEquals(ZoneList<Expression*>* arguments);
   void EmitArguments(ZoneList<Expression*>* arguments);
   void EmitArgumentsLength(ZoneList<Expression*>* arguments);
=======================================
--- /branches/bleeding_edge/src/ia32/codegen-ia32.cc Wed Aug 11 06:46:10 2010 +++ /branches/bleeding_edge/src/ia32/codegen-ia32.cc Thu Aug 12 06:43:08 2010
@@ -6760,7 +6760,7 @@
 }


-  void CodeGenerator::GenerateIsSpecObject(ZoneList<Expression*>* args) {
+void CodeGenerator::GenerateIsSpecObject(ZoneList<Expression*>* args) {
   // This generates a fast version of:
   // (typeof(arg) === 'object' || %_ClassOf(arg) == 'RegExp' ||
   // typeof(arg) == function).
@@ -6779,6 +6779,143 @@
   value.Unuse();
   destination()->Split(above_equal);
 }
+
+
+// Deferred code to check whether the String JavaScript object is safe for using +// default value of. This code is called after the bit caching this information +// in the map has been checked with the map for the object in the map_result_ +// register. On return the register map_result_ contains 1 for true and 0 for
+// false.
+class DeferredIsStringWrapperSafeForDefaultValueOf : public DeferredCode {
+ public:
+  DeferredIsStringWrapperSafeForDefaultValueOf(Register object,
+                                               Register map_result,
+                                               Register scratch1,
+                                               Register scratch2)
+      : object_(object),
+        map_result_(map_result),
+        scratch1_(scratch1),
+        scratch2_(scratch2) { }
+
+  virtual void Generate() {
+    Label false_result;
+
+    // Check that map is loaded as expected.
+    if (FLAG_debug_code) {
+      __ cmp(map_result_, FieldOperand(object_, HeapObject::kMapOffset));
+      __ Assert(equal, "Map not in expected register");
+    }
+
+ // Check for fast case object. Generate false result for slow case object.
+    __ mov(scratch1_, FieldOperand(object_, JSObject::kPropertiesOffset));
+    __ mov(scratch1_, FieldOperand(scratch1_, HeapObject::kMapOffset));
+    __ cmp(scratch1_, Factory::hash_table_map());
+    __ j(equal, &false_result);
+
+ // Look for valueOf symbol in the descriptor array, and indicate false if + // found. The type is not checked, so if it is a transition it is a false
+    // negative.
+    __ mov(map_result_,
+           FieldOperand(map_result_, Map::kInstanceDescriptorsOffset));
+ __ mov(scratch1_, FieldOperand(map_result_, FixedArray::kLengthOffset));
+    // map_result_: descriptor array
+    // scratch1_: length of descriptor array
+    // Calculate the end of the descriptor array.
+    STATIC_ASSERT(kSmiTag == 0);
+    STATIC_ASSERT(kSmiTagSize == 1);
+    STATIC_ASSERT(kPointerSize == 4);
+    __ lea(scratch1_,
+ Operand(map_result_, scratch1_, times_2, FixedArray::kHeaderSize));
+    // Calculate location of the first key name.
+    __ add(Operand(map_result_),
+           Immediate(FixedArray::kHeaderSize +
+                     DescriptorArray::kFirstIndex * kPointerSize));
+ // Loop through all the keys in the descriptor array. If one of these is the
+    // symbol valueOf the result is false.
+    Label entry, loop;
+    __ jmp(&entry);
+    __ bind(&loop);
+    __ mov(scratch2_, FieldOperand(map_result_, 0));
+    __ cmp(scratch2_, Factory::value_of_symbol());
+    __ j(equal, &false_result);
+    __ add(Operand(map_result_), Immediate(kPointerSize));
+    __ bind(&entry);
+    __ cmp(map_result_, Operand(scratch1_));
+    __ j(not_equal, &loop);
+
+    // Reload map as register map_result_ was used as temporary above.
+    __ mov(map_result_, FieldOperand(object_, HeapObject::kMapOffset));
+
+    // If a valueOf property is not found on the object check that it's
+ // prototype is the un-modified String prototype. If not result is false.
+    __ mov(scratch1_, FieldOperand(map_result_, Map::kPrototypeOffset));
+    __ test(scratch1_, Immediate(kSmiTagMask));
+    __ j(zero, &false_result);
+    __ mov(scratch1_, FieldOperand(scratch1_, HeapObject::kMapOffset));
+ __ mov(scratch2_, Operand(esi, Context::SlotOffset(Context::GLOBAL_INDEX)));
+    __ mov(scratch2_,
+           FieldOperand(scratch2_, GlobalObject::kGlobalContextOffset));
+    __ cmp(scratch1_,
+           CodeGenerator::ContextOperand(
+               scratch2_, Context::STRING_FUNCTION_PROTOTYPE_MAP_INDEX));
+    __ j(not_equal, &false_result);
+    // Set the bit in the map to indicate that it has been checked safe for
+    // default valueOf and set true result.
+    __ or_(FieldOperand(map_result_, Map::kBitField2Offset),
+           Immediate(1 << Map::kStringWrapperSafeForDefaultValueOf));
+    __ Set(map_result_, Immediate(1));
+    __ jmp(exit_label());
+    __ bind(&false_result);
+    // Set false result.
+    __ Set(map_result_, Immediate(0));
+  }
+
+ private:
+  Register object_;
+  Register map_result_;
+  Register scratch1_;
+  Register scratch2_;
+};
+
+
+void CodeGenerator::GenerateIsStringWrapperSafeForDefaultValueOf(
+    ZoneList<Expression*>* args) {
+  ASSERT(args->length() == 1);
+  Load(args->at(0));
+  Result obj = frame_->Pop();  // Pop the string wrapper.
+  obj.ToRegister();
+  ASSERT(obj.is_valid());
+  if (FLAG_debug_code) {
+    __ AbortIfSmi(obj.reg());
+  }
+
+  // Check whether this map has already been checked to be safe for default
+  // valueOf.
+  Result map_result = allocator()->Allocate();
+  ASSERT(map_result.is_valid());
+ __ mov(map_result.reg(), FieldOperand(obj.reg(), HeapObject::kMapOffset));
+  __ test_b(FieldOperand(map_result.reg(), Map::kBitField2Offset),
+            1 << Map::kStringWrapperSafeForDefaultValueOf);
+  destination()->true_target()->Branch(not_zero);
+
+  // We need an additional two scratch registers for the deferred code.
+  Result temp1 = allocator()->Allocate();
+  ASSERT(temp1.is_valid());
+  Result temp2 = allocator()->Allocate();
+  ASSERT(temp2.is_valid());
+
+  DeferredIsStringWrapperSafeForDefaultValueOf* deferred =
+      new DeferredIsStringWrapperSafeForDefaultValueOf(
+          obj.reg(), map_result.reg(), temp1.reg(), temp2.reg());
+  deferred->Branch(zero);
+  deferred->BindExit();
+  __ test(map_result.reg(), Operand(map_result.reg()));
+  obj.Unuse();
+  map_result.Unuse();
+  temp1.Unuse();
+  temp2.Unuse();
+  destination()->Split(not_equal);
+}


 void CodeGenerator::GenerateIsFunction(ZoneList<Expression*>* args) {
=======================================
--- /branches/bleeding_edge/src/ia32/codegen-ia32.h     Mon Aug  9 06:12:02 2010
+++ /branches/bleeding_edge/src/ia32/codegen-ia32.h     Thu Aug 12 06:43:08 2010
@@ -357,6 +357,10 @@
int offset = FixedArray::kHeaderSize + additional_offset * kPointerSize; return FieldOperand(array, index_as_smi, times_half_pointer_size, offset);
   }
+
+  static Operand ContextOperand(Register context, int index) {
+    return Operand(context, Context::SlotOffset(index));
+  }

  private:
   // Construction/Destruction
@@ -430,10 +434,6 @@
   // The following are used by class Reference.
   void LoadReference(Reference* ref);

-  static Operand ContextOperand(Register context, int index) {
-    return Operand(context, Context::SlotOffset(index));
-  }
-
   Operand SlotOperand(Slot* slot, Register tmp);

   Operand ContextSlotOperandCheckExtensions(Slot* slot,
@@ -653,6 +653,8 @@
   void GenerateIsSpecObject(ZoneList<Expression*>* args);
   void GenerateIsFunction(ZoneList<Expression*>* args);
   void GenerateIsUndetectableObject(ZoneList<Expression*>* args);
+  void GenerateIsStringWrapperSafeForDefaultValueOf(
+      ZoneList<Expression*>* args);

   // Support for construct call checks.
   void GenerateIsConstructCall(ZoneList<Expression*>* args);
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Wed Aug 11 06:46:10 2010 +++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Thu Aug 12 06:43:08 2010
@@ -2052,6 +2052,25 @@

   Apply(context_, if_true, if_false);
 }
+
+
+void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf(
+    ZoneList<Expression*>* args) {
+  ASSERT(args->length() == 1);
+
+  VisitForValue(args->at(0), kAccumulator);
+
+  Label materialize_true, materialize_false;
+  Label* if_true = NULL;
+  Label* if_false = NULL;
+  PrepareTest(&materialize_true, &materialize_false, &if_true, &if_false);
+
+ // Just indicate false, as %_IsStringWrapperSafeForDefaultValueOf() is only + // used in a few functions in runtime.js which should not normally be hit by
+  // this compiler.
+  __ jmp(if_false);
+  Apply(context_, if_true, if_false);
+}


 void FullCodeGenerator::EmitIsFunction(ZoneList<Expression*>* args) {
=======================================
--- /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Wed Aug 11 01:12:53 2010 +++ /branches/bleeding_edge/src/ia32/macro-assembler-ia32.cc Thu Aug 12 06:43:08 2010
@@ -373,13 +373,13 @@

 void MacroAssembler::AbortIfNotSmi(Register object) {
   test(object, Immediate(kSmiTagMask));
-  Assert(equal, "Operand not a smi");
+  Assert(equal, "Operand is not a smi");
 }


 void MacroAssembler::AbortIfSmi(Register object) {
   test(object, Immediate(kSmiTagMask));
-  Assert(not_equal, "Operand a smi");
+  Assert(not_equal, "Operand is a smi");
 }


=======================================
--- /branches/bleeding_edge/src/objects.h       Wed Aug 11 07:30:14 2010
+++ /branches/bleeding_edge/src/objects.h       Thu Aug 12 06:43:08 2010
@@ -3207,6 +3207,7 @@
   static const int kIsExtensible = 0;
   static const int kFunctionWithPrototype = 1;
   static const int kHasFastElements = 2;
+  static const int kStringWrapperSafeForDefaultValueOf = 3;

// Layout of the default cache. It holds alternating name and code objects.
   static const int kCodeCacheEntrySize = 2;
=======================================
--- /branches/bleeding_edge/src/runtime.js      Wed Aug 11 06:46:10 2010
+++ /branches/bleeding_edge/src/runtime.js      Thu Aug 12 06:43:08 2010
@@ -175,7 +175,7 @@
 // Left operand (this) is already a string.
 function STRING_ADD_LEFT(y) {
   if (!IS_STRING(y)) {
-    if (IS_STRING_WRAPPER(y)) {
+ if (IS_STRING_WRAPPER(y) && %_IsStringWrapperSafeForDefaultValueOf(y)) {
       y = %_ValueOf(y);
     } else {
       y = IS_NUMBER(y)
@@ -191,7 +191,7 @@
 function STRING_ADD_RIGHT(y) {
   var x = this;
   if (!IS_STRING(x)) {
-    if (IS_STRING_WRAPPER(x)) {
+ if (IS_STRING_WRAPPER(x) && %_IsStringWrapperSafeForDefaultValueOf(x)) {
       x = %_ValueOf(x);
     } else {
       x = IS_NUMBER(x)
=======================================
--- /branches/bleeding_edge/src/x64/codegen-x64.cc      Wed Aug 11 06:46:10 2010
+++ /branches/bleeding_edge/src/x64/codegen-x64.cc      Thu Aug 12 06:43:08 2010
@@ -6038,6 +6038,143 @@
   value.Unuse();
   destination()->Split(above_equal);
 }
+
+
+// Deferred code to check whether the String JavaScript object is safe for using +// default value of. This code is called after the bit caching this information +// in the map has been checked with the map for the object in the map_result_ +// register. On return the register map_result_ contains 1 for true and 0 for
+// false.
+class DeferredIsStringWrapperSafeForDefaultValueOf : public DeferredCode {
+ public:
+  DeferredIsStringWrapperSafeForDefaultValueOf(Register object,
+                                               Register map_result,
+                                               Register scratch1,
+                                               Register scratch2)
+      : object_(object),
+        map_result_(map_result),
+        scratch1_(scratch1),
+        scratch2_(scratch2) { }
+
+  virtual void Generate() {
+    Label false_result;
+
+    // Check that map is loaded as expected.
+    if (FLAG_debug_code) {
+      __ cmpq(map_result_, FieldOperand(object_, HeapObject::kMapOffset));
+      __ Assert(equal, "Map not in expected register");
+    }
+
+ // Check for fast case object. Generate false result for slow case object.
+    __ movq(scratch1_, FieldOperand(object_, JSObject::kPropertiesOffset));
+    __ movq(scratch1_, FieldOperand(scratch1_, HeapObject::kMapOffset));
+    __ CompareRoot(scratch1_, Heap::kHashTableMapRootIndex);
+    __ j(equal, &false_result);
+
+ // Look for valueOf symbol in the descriptor array, and indicate false if + // found. The type is not checked, so if it is a transition it is a false
+    // negative.
+    __ movq(map_result_,
+           FieldOperand(map_result_, Map::kInstanceDescriptorsOffset));
+ __ movq(scratch1_, FieldOperand(map_result_, FixedArray::kLengthOffset));
+    // map_result_: descriptor array
+    // scratch1_: length of descriptor array
+    // Calculate the end of the descriptor array.
+ SmiIndex index = masm_->SmiToIndex(scratch2_, scratch1_, kPointerSizeLog2);
+    __ lea(scratch1_,
+           Operand(
+ map_result_, index.reg, index.scale, FixedArray::kHeaderSize));
+    // Calculate location of the first key name.
+    __ addq(map_result_,
+            Immediate(FixedArray::kHeaderSize +
+                      DescriptorArray::kFirstIndex * kPointerSize));
+ // Loop through all the keys in the descriptor array. If one of these is the
+    // symbol valueOf the result is false.
+    Label entry, loop;
+    __ jmp(&entry);
+    __ bind(&loop);
+    __ movq(scratch2_, FieldOperand(map_result_, 0));
+    __ Cmp(scratch2_, Factory::value_of_symbol());
+    __ j(equal, &false_result);
+    __ addq(map_result_, Immediate(kPointerSize));
+    __ bind(&entry);
+    __ cmpq(map_result_, scratch1_);
+    __ j(not_equal, &loop);
+
+    // Reload map as register map_result_ was used as temporary above.
+    __ movq(map_result_, FieldOperand(object_, HeapObject::kMapOffset));
+
+    // If a valueOf property is not found on the object check that it's
+ // prototype is the un-modified String prototype. If not result is false.
+    __ movq(scratch1_, FieldOperand(map_result_, Map::kPrototypeOffset));
+    __ testq(scratch1_, Immediate(kSmiTagMask));
+    __ j(zero, &false_result);
+    __ movq(scratch1_, FieldOperand(scratch1_, HeapObject::kMapOffset));
+    __ movq(scratch2_,
+            Operand(rsi, Context::SlotOffset(Context::GLOBAL_INDEX)));
+    __ movq(scratch2_,
+            FieldOperand(scratch2_, GlobalObject::kGlobalContextOffset));
+    __ cmpq(scratch1_,
+            CodeGenerator::ContextOperand(
+                scratch2_, Context::STRING_FUNCTION_PROTOTYPE_MAP_INDEX));
+    __ j(not_equal, &false_result);
+    // Set the bit in the map to indicate that it has been checked safe for
+    // default valueOf and set true result.
+    __ or_(FieldOperand(map_result_, Map::kBitField2Offset),
+           Immediate(1 << Map::kStringWrapperSafeForDefaultValueOf));
+    __ Set(map_result_, 1);
+    __ jmp(exit_label());
+    __ bind(&false_result);
+    // Set false result.
+    __ Set(map_result_, 0);
+  }
+
+ private:
+  Register object_;
+  Register map_result_;
+  Register scratch1_;
+  Register scratch2_;
+};
+
+
+void CodeGenerator::GenerateIsStringWrapperSafeForDefaultValueOf(
+    ZoneList<Expression*>* args) {
+  ASSERT(args->length() == 1);
+  Load(args->at(0));
+  Result obj = frame_->Pop();  // Pop the string wrapper.
+  obj.ToRegister();
+  ASSERT(obj.is_valid());
+  if (FLAG_debug_code) {
+    __ AbortIfSmi(obj.reg());
+  }
+
+  // Check whether this map has already been checked to be safe for default
+  // valueOf.
+  Result map_result = allocator()->Allocate();
+  ASSERT(map_result.is_valid());
+ __ movq(map_result.reg(), FieldOperand(obj.reg(), HeapObject::kMapOffset));
+  __ testb(FieldOperand(map_result.reg(), Map::kBitField2Offset),
+           Immediate(1 << Map::kStringWrapperSafeForDefaultValueOf));
+  destination()->true_target()->Branch(not_zero);
+
+  // We need an additional two scratch registers for the deferred code.
+  Result temp1 = allocator()->Allocate();
+  ASSERT(temp1.is_valid());
+  Result temp2 = allocator()->Allocate();
+  ASSERT(temp2.is_valid());
+
+  DeferredIsStringWrapperSafeForDefaultValueOf* deferred =
+      new DeferredIsStringWrapperSafeForDefaultValueOf(
+          obj.reg(), map_result.reg(), temp1.reg(), temp2.reg());
+  deferred->Branch(zero);
+  deferred->BindExit();
+  __ testq(map_result.reg(), map_result.reg());
+  obj.Unuse();
+  map_result.Unuse();
+  temp1.Unuse();
+  temp2.Unuse();
+  destination()->Split(not_equal);
+}


 void CodeGenerator::GenerateIsFunction(ZoneList<Expression*>* args) {
=======================================
--- /branches/bleeding_edge/src/x64/codegen-x64.h       Fri Aug  6 06:04:27 2010
+++ /branches/bleeding_edge/src/x64/codegen-x64.h       Thu Aug 12 06:43:08 2010
@@ -347,6 +347,10 @@
   // expected arguments. Otherwise return -1.
   static int InlineRuntimeCallArgumentsCount(Handle<String> name);

+  static Operand ContextOperand(Register context, int index) {
+    return Operand(context, Context::SlotOffset(index));
+  }
+
  private:
   // Construction/Destruction
   explicit CodeGenerator(MacroAssembler* masm);
@@ -406,10 +410,6 @@
   void LoadReference(Reference* ref);
   void UnloadReference(Reference* ref);

-  static Operand ContextOperand(Register context, int index) {
-    return Operand(context, Context::SlotOffset(index));
-  }
-
   Operand SlotOperand(Slot* slot, Register tmp);

   Operand ContextSlotOperandCheckExtensions(Slot* slot,
@@ -611,6 +611,8 @@
   void GenerateIsSpecObject(ZoneList<Expression*>* args);
   void GenerateIsFunction(ZoneList<Expression*>* args);
   void GenerateIsUndetectableObject(ZoneList<Expression*>* args);
+  void GenerateIsStringWrapperSafeForDefaultValueOf(
+      ZoneList<Expression*>* args);

   // Support for construct call checks.
   void GenerateIsConstructCall(ZoneList<Expression*>* args);
=======================================
--- /branches/bleeding_edge/src/x64/full-codegen-x64.cc Wed Aug 11 06:46:10 2010 +++ /branches/bleeding_edge/src/x64/full-codegen-x64.cc Thu Aug 12 06:43:08 2010
@@ -2057,6 +2057,25 @@

   Apply(context_, if_true, if_false);
 }
+
+
+void FullCodeGenerator::EmitIsStringWrapperSafeForDefaultValueOf(
+    ZoneList<Expression*>* args) {
+  ASSERT(args->length() == 1);
+
+  VisitForValue(args->at(0), kAccumulator);
+
+  Label materialize_true, materialize_false;
+  Label* if_true = NULL;
+  Label* if_false = NULL;
+  PrepareTest(&materialize_true, &materialize_false, &if_true, &if_false);
+
+ // Just indicate false, as %_IsStringWrapperSafeForDefaultValueOf() is only + // used in a few functions in runtime.js which should not normally be hit by
+  // this compiler.
+  __ jmp(if_false);
+  Apply(context_, if_true, if_false);
+}


 void FullCodeGenerator::EmitIsFunction(ZoneList<Expression*>* args) {
=======================================
--- /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Wed Aug 11 07:23:12 2010 +++ /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Thu Aug 12 06:43:08 2010
@@ -1975,12 +1975,19 @@
   Assert(equal, "Operand not a number");
   bind(&ok);
 }
+
+
+void MacroAssembler::AbortIfSmi(Register object) {
+  Label ok;
+  Condition is_smi = CheckSmi(object);
+  Assert(NegateCondition(is_smi), "Operand is a smi");
+}


 void MacroAssembler::AbortIfNotSmi(Register object) {
   Label ok;
   Condition is_smi = CheckSmi(object);
-  Assert(is_smi, "Operand not a smi");
+  Assert(is_smi, "Operand is not a smi");
 }


=======================================
--- /branches/bleeding_edge/src/x64/macro-assembler-x64.h Fri Aug 6 06:04:27 2010 +++ /branches/bleeding_edge/src/x64/macro-assembler-x64.h Thu Aug 12 06:43:08 2010
@@ -582,6 +582,9 @@
   // Abort execution if argument is not a number. Used in debug code.
   void AbortIfNotNumber(Register object);

+  // Abort execution if argument is a smi. Used in debug code.
+  void AbortIfSmi(Register object);
+
   // Abort execution if argument is not a smi. Used in debug code.
   void AbortIfNotSmi(Register object);

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to