From 4707d7ac0821dadcddd178a0f526406b7bdb46e7 Mon Sep 17 00:00:00 2001 From: "lrn@chromium.org" Date: Thu, 3 Mar 2011 11:49:03 +0000 Subject: [PATCH] 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 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7040 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/builtins-arm.cc | 2 +- src/array.js | 54 ++++++++++++++++++++++++++++-------------- src/bootstrapper.cc | 38 +++++++++++++++++++++++++++++ src/ia32/builtins-ia32.cc | 8 +++---- src/json.js | 20 ++++++++-------- src/objects.cc | 4 ++++ src/regexp.js | 6 ++--- src/string.js | 14 +++++------ src/v8natives.js | 10 ++++---- src/x64/builtins-x64.cc | 2 +- src/x64/macro-assembler-x64.cc | 2 +- 11 files changed, 109 insertions(+), 51 deletions(-) diff --git a/src/arm/builtins-arm.cc b/src/arm/builtins-arm.cc index 6e8fe28..169bc6b 100644 --- a/src/arm/builtins-arm.cc +++ b/src/arm/builtins-arm.cc @@ -428,7 +428,7 @@ void Builtins::Generate_ArrayCode(MacroAssembler* masm) { 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"); diff --git a/src/array.js b/src/array.js index 0753f1e..e8767b2 100644 --- a/src/array.js +++ b/src/array.js @@ -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 @@ function SparseJoin(array, len, convert) { 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 @@ function Join(array, length, separator, convert) { } // 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 @@ function Join(array, length, separator, convert) { 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 @@ function Join(array, length, separator, convert) { 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 @@ function Join(array, length, separator, convert) { 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 Join(array, length, separator, convert) { 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 @@ function SmartSlice(array, start_i, del_count, len, deleted_elements) { // 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 ArrayPush() { 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 @@ function ArrayFilter(f, receiver) { 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 @@ function ArrayForEach(f, receiver) { 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 @@ function ArraySome(f, receiver) { 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 @@ function ArrayEvery(f, receiver) { 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 @@ function ArrayMap(f, receiver) { // 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 @@ function ArrayReduce(callback, current) { 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 @@ function ArrayReduceRight(callback, current) { 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 @@ function SetupArray() { )); %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; + }; } diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 415d2dd..8cd29b2 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -1240,6 +1240,43 @@ bool Genesis::InstallNatives() { 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 array_function = + InstallFunction(builtins, + "InternalArray", + JS_ARRAY_TYPE, + JSArray::kSize, + Top::initial_object_prototype(), + Builtins::ArrayCode, + true); + Handle 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 array_descriptors = + Factory::CopyAppendProxyDescriptor( + Factory::empty_descriptor_array(), + Factory::length_symbol(), + Factory::NewProxy(&Accessors::ArrayLength), + static_cast(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"); return true; @@ -1358,6 +1395,7 @@ bool Genesis::InstallNatives() { global_context()->set_regexp_result_map(*initial_map); } + #ifdef DEBUG builtins->Verify(); #endif diff --git a/src/ia32/builtins-ia32.cc b/src/ia32/builtins-ia32.cc index f15fd1c..c7e5527 100644 --- a/src/ia32/builtins-ia32.cc +++ b/src/ia32/builtins-ia32.cc @@ -1248,11 +1248,9 @@ void Builtins::Generate_ArrayConstructCode(MacroAssembler* masm) { 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. diff --git a/src/json.js b/src/json.js index e6ada51..7a6189c 100644 --- a/src/json.js +++ b/src/json.js @@ -49,7 +49,7 @@ function Revive(holder, name, reviver) { } } } - return reviver.call(holder, name, val); + return %_CallFunction(holder, name, val, reviver); } function JSONParse(text, reviver) { @@ -63,11 +63,11 @@ function JSONParse(text, reviver) { 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 SerializeArray(value, replacer, stack, indent, gap) { 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 @@ function BasicSerializeArray(value, stack, builder) { 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 BasicSerializeArray(value, stack, builder) { 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 BasicJSONSerialize(key, value, stack, builder) { 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 @@ function JSONStringify(value, replacer, space) { } else { gap = ""; } - return JSONSerialize('', {'': value}, replacer, [], "", gap); + return JSONSerialize('', {'': value}, replacer, new InternalArray(), "", gap); } function SetupJSON() { diff --git a/src/objects.cc b/src/objects.cc index 0975a03..aa485df 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -5552,6 +5552,10 @@ MaybeObject* JSFunction::SetPrototype(Object* value) { 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()); diff --git a/src/regexp.js b/src/regexp.js index 5b7e3a9..f68dee6 100644 --- a/src/regexp.js +++ b/src/regexp.js @@ -384,13 +384,13 @@ function RegExpMakeCaptureGetter(n) { // 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. diff --git a/src/string.js b/src/string.js index 2b73e0f..d8d402c 100644 --- a/src/string.js +++ b/src/string.js @@ -87,7 +87,7 @@ function StringConcat() { 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 @@ function addCaptureString(builder, matchInfo, index) { // 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 @@ function StringReplaceGlobalRegExpWithFunction(subject, regexp, replace) { // 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 @@ function StringReplaceGlobalRegExpWithFunction(subject, regexp, replace) { 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 @@ function StringReplaceNonGlobalRegExpWithFunction(subject, regexp, replace) { 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 @@ function StringTrimRight() { 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 @@ function ReplaceResultBuilder(str) { if (%_ArgumentsLength() > 1) { this.elements = %_Arguments(1); } else { - this.elements = new $Array(); + this.elements = new InternalArray(); } this.special_string = str; } diff --git a/src/v8natives.js b/src/v8natives.js index 91e19c1..563de73 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -143,7 +143,7 @@ function GlobalEval(x) { var f = %CompileString(x); if (!IS_FUNCTION(f)) return f; - return f.call(this); + return %_CallFunction(this, f); } @@ -152,7 +152,7 @@ function GlobalExecScript(expr, lang) { // 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 @@ function FunctionBind(this_arg) { // Length is 1. 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 @@ function FunctionBind(this_arg) { // Length is 1. // 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 @@ function NewFunction(arg1) { // length == 1 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 diff --git a/src/x64/builtins-x64.cc b/src/x64/builtins-x64.cc index a2dd6cd..4191792 100644 --- a/src/x64/builtins-x64.cc +++ b/src/x64/builtins-x64.cc @@ -1248,7 +1248,7 @@ void Builtins::Generate_ArrayCode(MacroAssembler* masm) { __ 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); diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc index 8845bbb..cdd96e2 100644 --- a/src/x64/macro-assembler-x64.cc +++ b/src/x64/macro-assembler-x64.cc @@ -908,7 +908,7 @@ Condition MacroAssembler::CheckSmi(const Operand& src) { 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)); -- 2.7.4