Revision: 7040
Author: [email protected]
Date: Thu Mar  3 03:49:03 2011
Log: Stop using plain Arrays internally in built-in functions.

In built-in code we use arrays for internal computations.
This makes it possible to affect the built-in code by putting getters
or setters on the Array prototype chain.
This adds a new internal Array constructor that creates Arrays with
a very simplistic prototype chain that doesn't include any publicly
visible objects. These Arrays shoudl ofcourse never leak outside the
builtins, since that would expose the prototype object.
The prototype object contains only the array functions that we use:
push, pop and join (and not even a toString, so it doesn't stringify
well).

Also change uses of .call to %_CallFunction.

BUG=1206

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

Modified:
 /branches/bleeding_edge/src/arm/builtins-arm.cc
 /branches/bleeding_edge/src/array.js
 /branches/bleeding_edge/src/bootstrapper.cc
 /branches/bleeding_edge/src/ia32/builtins-ia32.cc
 /branches/bleeding_edge/src/json.js
 /branches/bleeding_edge/src/objects.cc
 /branches/bleeding_edge/src/regexp.js
 /branches/bleeding_edge/src/string.js
 /branches/bleeding_edge/src/v8natives.js
 /branches/bleeding_edge/src/x64/builtins-x64.cc
 /branches/bleeding_edge/src/x64/macro-assembler-x64.cc

=======================================
--- /branches/bleeding_edge/src/arm/builtins-arm.cc     Tue Feb 15 10:57:37 2011
+++ /branches/bleeding_edge/src/arm/builtins-arm.cc     Thu Mar  3 03:49:03 2011
@@ -428,7 +428,7 @@
   GenerateLoadArrayFunction(masm, r1);

   if (FLAG_debug_code) {
-    // Initial map for the builtin Array function shoud be a map.
+    // Initial map for the builtin Array functions should be maps.
__ ldr(r2, FieldMemOperand(r1, JSFunction::kPrototypeOrInitialMapOffset));
     __ tst(r2, Operand(kSmiTagMask));
     __ Assert(ne, "Unexpected initial map for Array function");
=======================================
--- /branches/bleeding_edge/src/array.js        Fri Feb 25 03:30:22 2011
+++ /branches/bleeding_edge/src/array.js        Thu Mar  3 03:49:03 2011
@@ -33,7 +33,7 @@

 // Global list of arrays visited during toString, toLocaleString and
 // join invocations.
-var visited_arrays = new $Array();
+var visited_arrays = new InternalArray();


 // Gets a sorted array of array keys.  Useful for operations on sparse
@@ -73,7 +73,7 @@
   var last_key = -1;
   var keys_length = keys.length;

-  var elements = new $Array(keys_length);
+  var elements = new InternalArray(keys_length);
   var elements_length = 0;

   for (var i = 0; i < keys_length; i++) {
@@ -122,7 +122,7 @@
     }

     // Construct an array for the elements.
-    var elements = new $Array(length);
+    var elements = new InternalArray(length);

     // We pull the empty separator check outside the loop for speed!
     if (separator.length == 0) {
@@ -140,7 +140,7 @@
       return %StringBuilderConcat(elements, elements_length, '');
     }
     // Non-empty separator case.
-    // If the first element is a number then use the heuristic that the
+    // If the first element is a number then use the heuristic that the
     // remaining elements are also likely to be numbers.
     if (!IS_NUMBER(array[0])) {
       for (var i = 0; i < length; i++) {
@@ -148,7 +148,7 @@
         if (!IS_STRING(e)) e = convert(e);
         elements[i] = e;
       }
-    } else {
+    } else {
       for (var i = 0; i < length; i++) {
         var e = array[i];
         if (IS_NUMBER(e)) elements[i] = %_NumberToString(e);
@@ -157,9 +157,9 @@
           elements[i] = e;
         }
       }
-    }
+    }
     var result = %_FastAsciiArrayJoin(elements, separator);
-    if (!IS_UNDEFINED(result)) return result;
+    if (!IS_UNDEFINED(result)) return result;

     return %StringBuilderJoin(elements, length, separator);
   } finally {
@@ -171,7 +171,7 @@


 function ConvertToString(x) {
-  // Assumes x is a non-string.
+  // Assumes x is a non-string.
   if (IS_NUMBER(x)) return %_NumberToString(x);
   if (IS_BOOLEAN(x)) return x ? 'true' : 'false';
   return (IS_NULL_OR_UNDEFINED(x)) ? '' : %ToString(%DefaultString(x));
@@ -241,7 +241,7 @@
 // special array operations to handle sparse arrays in a sensible fashion.
 function SmartMove(array, start_i, del_count, len, num_additional_args) {
   // Move data to new array.
-  var new_array = new $Array(len - del_count + num_additional_args);
+  var new_array = new InternalArray(len - del_count + num_additional_args);
   var intervals = %GetArrayKeys(array, len);
   var length = intervals.length;
   for (var k = 0; k < length; k++) {
@@ -419,7 +419,7 @@

 function ArrayConcat(arg1) {  // length == 1
   var arg_count = %_ArgumentsLength();
-  var arrays = new $Array(1 + arg_count);
+  var arrays = new InternalArray(1 + arg_count);
   arrays[0] = this;
   for (var i = 0; i < arg_count; i++) {
     arrays[i + 1] = %_Arguments(i);
@@ -925,7 +925,9 @@
   for (var i = 0; i < length; i++) {
     var current = this[i];
     if (!IS_UNDEFINED(current) || i in this) {
- if (f.call(receiver, current, i, this)) result[result_length++] = current;
+      if (%_CallFunction(receiver, current, i, this, f)) {
+        result[result_length++] = current;
+      }
     }
   }
   return result;
@@ -942,7 +944,7 @@
   for (var i = 0; i < length; i++) {
     var current = this[i];
     if (!IS_UNDEFINED(current) || i in this) {
-      f.call(receiver, current, i, this);
+      %_CallFunction(receiver, current, i, this, f);
     }
   }
 }
@@ -960,7 +962,7 @@
   for (var i = 0; i < length; i++) {
     var current = this[i];
     if (!IS_UNDEFINED(current) || i in this) {
-      if (f.call(receiver, current, i, this)) return true;
+      if (%_CallFunction(receiver, current, i, this, f)) return true;
     }
   }
   return false;
@@ -977,7 +979,7 @@
   for (var i = 0; i < length; i++) {
     var current = this[i];
     if (!IS_UNDEFINED(current) || i in this) {
-      if (!f.call(receiver, current, i, this)) return false;
+      if (!%_CallFunction(receiver, current, i, this, f)) return false;
     }
   }
   return true;
@@ -990,13 +992,15 @@
   // Pull out the length so that modifications to the length in the
   // loop will not affect the looping.
   var length = TO_UINT32(this.length);
-  var result = new $Array(length);
+  var result = new $Array();
+  var accumulator = new InternalArray(length);
   for (var i = 0; i < length; i++) {
     var current = this[i];
     if (!IS_UNDEFINED(current) || i in this) {
-      result[i] = f.call(receiver, current, i, this);
+      accumulator[i] = %_CallFunction(receiver, current, i, this, f);
     }
   }
+  %MoveArrayContents(accumulator, result);
   return result;
 }

@@ -1134,7 +1138,7 @@
   for (; i < length; i++) {
     var element = this[i];
     if (!IS_UNDEFINED(element) || i in this) {
-      current = callback.call(null, current, element, i, this);
+      current = %_CallFunction(null, current, element, i, this, callback);
     }
   }
   return current;
@@ -1160,7 +1164,7 @@
   for (; i >= 0; i--) {
     var element = this[i];
     if (!IS_UNDEFINED(element) || i in this) {
-      current = callback.call(null, current, element, i, this);
+      current = %_CallFunction(null, current, element, i, this, callback);
     }
   }
   return current;
@@ -1225,6 +1229,20 @@
   ));

   %FinishArrayPrototypeSetup($Array.prototype);
+
+ // The internal Array prototype doesn't need to be fancy, since it's never
+  // exposed to user code, so no hidden prototypes or DONT_ENUM attributes
+  // are necessary.
+  // The null __proto__ ensures that we never inherit any user created
+  // getters or setters from, e.g., Object.prototype.
+  InternalArray.prototype.__proto__ = null;
+  // Adding only the functions that are actually used, and a toString.
+  InternalArray.prototype.join = getFunction("join", ArrayJoin);
+  InternalArray.prototype.pop = getFunction("pop", ArrayPop);
+  InternalArray.prototype.push = getFunction("push", ArrayPush);
+  InternalArray.prototype.toString = function() {
+    return "Internal Array, length " + this.length;
+  };
 }


=======================================
--- /branches/bleeding_edge/src/bootstrapper.cc Thu Feb 10 06:41:16 2011
+++ /branches/bleeding_edge/src/bootstrapper.cc Thu Mar  3 03:49:03 2011
@@ -1239,6 +1239,43 @@
     SetPrototype(opaque_reference_fun, prototype);
     global_context()->set_opaque_reference_function(*opaque_reference_fun);
   }
+
+  {  // --- I n t e r n a l   A r r a y ---
+    // An array constructor on the builtins object that works like
+    // the public Array constructor, except that its prototype
+    // doesn't inherit from Object.prototype.
+    // To be used only for internal work by builtins. Instances
+    // must not be leaked to user code.
+    // Only works correctly when called as a constructor. The normal
+    // Array code uses Array.prototype as prototype when called as
+    // a function.
+    Handle<JSFunction> array_function =
+        InstallFunction(builtins,
+                        "InternalArray",
+                        JS_ARRAY_TYPE,
+                        JSArray::kSize,
+                        Top::initial_object_prototype(),
+                        Builtins::ArrayCode,
+                        true);
+    Handle<JSObject> prototype =
+        Factory::NewJSObject(Top::object_function(), TENURED);
+    SetPrototype(array_function, prototype);
+
+    array_function->shared()->set_construct_stub(
+        Builtins::builtin(Builtins::ArrayConstructCode));
+    array_function->shared()->DontAdaptArguments();
+
+    // Make "length" magic on instances.
+    Handle<DescriptorArray> array_descriptors =
+        Factory::CopyAppendProxyDescriptor(
+            Factory::empty_descriptor_array(),
+            Factory::length_symbol(),
+            Factory::NewProxy(&Accessors::ArrayLength),
+            static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE));
+
+    array_function->initial_map()->set_instance_descriptors(
+        *array_descriptors);
+  }

   if (FLAG_disable_native_files) {
     PrintF("Warning: Running without installed natives!\n");
@@ -1357,6 +1394,7 @@

     global_context()->set_regexp_result_map(*initial_map);
   }
+

 #ifdef DEBUG
   builtins->Verify();
=======================================
--- /branches/bleeding_edge/src/ia32/builtins-ia32.cc Tue Feb 15 10:57:37 2011 +++ /branches/bleeding_edge/src/ia32/builtins-ia32.cc Thu Mar 3 03:49:03 2011
@@ -1248,11 +1248,9 @@
   Label generic_constructor;

   if (FLAG_debug_code) {
- // The array construct code is only set for the builtin Array function which
-    // does always have a map.
-    __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, ebx);
-    __ cmp(edi, Operand(ebx));
-    __ Assert(equal, "Unexpected Array function");
+    // The array construct code is only set for the global and natives
+    // builtin Array functions which always have maps.
+
     // Initial map for the builtin Array function should be a map.
__ mov(ebx, FieldOperand(edi, JSFunction::kPrototypeOrInitialMapOffset));
     // Will both indicate a NULL and a Smi.
=======================================
--- /branches/bleeding_edge/src/json.js Fri Jan 28 02:33:10 2011
+++ /branches/bleeding_edge/src/json.js Thu Mar  3 03:49:03 2011
@@ -49,7 +49,7 @@
       }
     }
   }
-  return reviver.call(holder, name, val);
+  return %_CallFunction(holder, name, val, reviver);
 }

 function JSONParse(text, reviver) {
@@ -63,11 +63,11 @@

 function SerializeArray(value, replacer, stack, indent, gap) {
   if (!%PushIfAbsent(stack, value)) {
-    throw MakeTypeError('circular_structure', []);
+    throw MakeTypeError('circular_structure', $Array());
   }
   var stepback = indent;
   indent += gap;
-  var partial = [];
+  var partial = new InternalArray();
   var len = value.length;
   for (var i = 0; i < len; i++) {
     var strP = JSONSerialize($String(i), value, replacer, stack,
@@ -93,11 +93,11 @@

 function SerializeObject(value, replacer, stack, indent, gap) {
   if (!%PushIfAbsent(stack, value)) {
-    throw MakeTypeError('circular_structure', []);
+    throw MakeTypeError('circular_structure', $Array());
   }
   var stepback = indent;
   indent += gap;
-  var partial = [];
+  var partial = new InternalArray();
   if (IS_ARRAY(replacer)) {
     var length = replacer.length;
     for (var i = 0; i < length; i++) {
@@ -185,7 +185,7 @@
     return;
   }
   if (!%PushIfAbsent(stack, value)) {
-    throw MakeTypeError('circular_structure', []);
+    throw MakeTypeError('circular_structure', $Array());
   }
   builder.push("[");
   var val = value[0];
@@ -238,7 +238,7 @@

 function BasicSerializeObject(value, stack, builder) {
   if (!%PushIfAbsent(stack, value)) {
-    throw MakeTypeError('circular_structure', []);
+    throw MakeTypeError('circular_structure', $Array());
   }
   builder.push("{");
   var first = true;
@@ -301,8 +301,8 @@

 function JSONStringify(value, replacer, space) {
   if (%_ArgumentsLength() == 1) {
-    var builder = [];
-    BasicJSONSerialize('', value, [], builder);
+    var builder = new InternalArray();
+    BasicJSONSerialize('', value, new InternalArray(), builder);
     if (builder.length == 0) return;
     var result = %_FastAsciiArrayJoin(builder, "");
     if (!IS_UNDEFINED(result)) return result;
@@ -329,7 +329,7 @@
   } else {
     gap = "";
   }
-  return JSONSerialize('', {'': value}, replacer, [], "", gap);
+ return JSONSerialize('', {'': value}, replacer, new InternalArray(), "", gap);
 }

 function SetupJSON() {
=======================================
--- /branches/bleeding_edge/src/objects.cc      Thu Mar  3 02:16:22 2011
+++ /branches/bleeding_edge/src/objects.cc      Thu Mar  3 03:49:03 2011
@@ -5552,6 +5552,10 @@


 Object* JSFunction::RemovePrototype() {
+ if (map() == context()->global_context()->function_without_prototype_map()) {
+    // Be idempotent.
+    return this;
+  }
   ASSERT(map() == context()->global_context()->function_map());
   set_map(context()->global_context()->function_without_prototype_map());
   set_prototype_or_initial_map(Heap::the_hole_value());
=======================================
--- /branches/bleeding_edge/src/regexp.js       Fri Jan 28 02:33:10 2011
+++ /branches/bleeding_edge/src/regexp.js       Thu Mar  3 03:49:03 2011
@@ -384,13 +384,13 @@
 // pairs for the match and all the captured substrings), the invariant is
 // that there are at least two capture indeces.  The array also contains
 // the subject string for the last successful match.
-var lastMatchInfo = [
+var lastMatchInfo = new InternalArray(
     2,                 // REGEXP_NUMBER_OF_CAPTURES
     "",                // Last subject.
     void 0,            // Last input - settable with RegExpSetInput.
     0,                 // REGEXP_FIRST_CAPTURE + 0
-    0,                 // REGEXP_FIRST_CAPTURE + 1
-];
+    0                  // REGEXP_FIRST_CAPTURE + 1
+);

 // Override last match info with an array of actual substrings.
 // Used internally by replace regexp with function.
=======================================
--- /branches/bleeding_edge/src/string.js       Fri Feb 11 05:30:37 2011
+++ /branches/bleeding_edge/src/string.js       Thu Mar  3 03:49:03 2011
@@ -87,7 +87,7 @@
   if (len === 1) {
     return this_as_string + %_Arguments(0);
   }
-  var parts = new $Array(len + 1);
+  var parts = new InternalArray(len + 1);
   parts[0] = this_as_string;
   for (var i = 0; i < len; i++) {
     var part = %_Arguments(i);
@@ -357,7 +357,7 @@
 // TODO(lrn): This array will survive indefinitely if replace is never
 // called again. However, it will be empty, since the contents are cleared
 // in the finally block.
-var reusableReplaceArray = $Array(16);
+var reusableReplaceArray = new InternalArray(16);

 // Helper function for replacing regular expressions with the result of a
 // function application in String.prototype.replace.
@@ -370,7 +370,7 @@
     // of another replace) or we have failed to set the reusable array
     // back due to an exception in a replacement function. Create a new
     // array to use in the future, or until the original is written back.
-    resultArray = $Array(16);
+    resultArray = new InternalArray(16);
   }
   var res = %RegExpExecMultiple(regexp,
                                 subject,
@@ -386,7 +386,7 @@
   var i = 0;
   if (NUMBER_OF_CAPTURES(lastMatchInfo) == 2) {
     var match_start = 0;
-    var override = [null, 0, subject];
+    var override = new InternalArray(null, 0, subject);
     var receiver = %GetGlobalReceiver();
     while (i < len) {
       var elem = res[i];
@@ -447,7 +447,7 @@
     replacement =
         %_CallFunction(%GetGlobalReceiver(), s, index, subject, replace);
   } else {
-    var parameters = $Array(m + 2);
+    var parameters = new InternalArray(m + 2);
     for (var j = 0; j < m; j++) {
       parameters[j] = CaptureString(subject, matchInfo, j);
     }
@@ -720,7 +720,7 @@
   return %StringTrim(TO_STRING_INLINE(this), false, true);
 }

-var static_charcode_array = new $Array(4);
+var static_charcode_array = new InternalArray(4);

 // ECMA-262, section 15.5.3.2
 function StringFromCharCode(code) {
@@ -825,7 +825,7 @@
   if (%_ArgumentsLength() > 1) {
     this.elements = %_Arguments(1);
   } else {
-    this.elements = new $Array();
+    this.elements = new InternalArray();
   }
   this.special_string = str;
 }
=======================================
--- /branches/bleeding_edge/src/v8natives.js    Tue Feb 22 03:21:15 2011
+++ /branches/bleeding_edge/src/v8natives.js    Thu Mar  3 03:49:03 2011
@@ -143,7 +143,7 @@
   var f = %CompileString(x);
   if (!IS_FUNCTION(f)) return f;

-  return f.call(this);
+  return %_CallFunction(this, f);
 }


@@ -152,7 +152,7 @@
   // NOTE: We don't care about the character casing.
   if (!lang || /javascript/i.test(lang)) {
     var f = %CompileString(ToString(expr));
-    f.call(%GlobalReceiver(global));
+    %_CallFunction(%GlobalReceiver(global), f);
   }
   return null;
 }
@@ -1170,7 +1170,7 @@
       return fn.apply(this_arg, arguments);
     };
   } else {
-    var bound_args = new $Array(argc_bound);
+    var bound_args = new InternalArray(argc_bound);
     for(var i = 0; i < argc_bound; i++) {
       bound_args[i] = %_Arguments(i+1);
     }
@@ -1188,7 +1188,7 @@
       // Combine the args we got from the bind call with the args
       // given as argument to the invocation.
       var argc = %_ArgumentsLength();
-      var args = new $Array(argc + argc_bound);
+      var args = new InternalArray(argc + argc_bound);
       // Add bound arguments.
       for (var i = 0; i < argc_bound; i++) {
         args[i] = bound_args[i];
@@ -1220,7 +1220,7 @@
   var n = %_ArgumentsLength();
   var p = '';
   if (n > 1) {
-    p = new $Array(n - 1);
+    p = new InternalArray(n - 1);
     for (var i = 0; i < n - 1; i++) p[i] = %_Arguments(i);
     p = Join(p, n - 1, ',', NonStringToString);
     // If the formal parameters string include ) - an illegal
=======================================
--- /branches/bleeding_edge/src/x64/builtins-x64.cc     Fri Feb 25 05:22:38 2011
+++ /branches/bleeding_edge/src/x64/builtins-x64.cc     Thu Mar  3 03:49:03 2011
@@ -1248,7 +1248,7 @@
   __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, rdi);

   if (FLAG_debug_code) {
-    // Initial map for the builtin Array function shoud be a map.
+    // Initial map for the builtin Array functions should be maps.
__ movq(rbx, FieldOperand(rdi, JSFunction::kPrototypeOrInitialMapOffset));
     // Will both indicate a NULL and a Smi.
     ASSERT(kSmiTag == 0);
=======================================
--- /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Fri Feb 25 05:22:38 2011 +++ /branches/bleeding_edge/src/x64/macro-assembler-x64.cc Thu Mar 3 03:49:03 2011
@@ -908,7 +908,7 @@

 Condition MacroAssembler::CheckNonNegativeSmi(Register src) {
   ASSERT_EQ(0, kSmiTag);
-  // Make mask 0x8000000000000001 and test that both bits are zero.
+  // Test that both bits of the mask 0x8000000000000001 are zero.
   movq(kScratchRegister, src);
   rol(kScratchRegister, Immediate(1));
   testb(kScratchRegister, Immediate(3));

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

Reply via email to