From 02c4e8bfcbdf6817e66236fb81c51e3aa9aafef8 Mon Sep 17 00:00:00 2001 From: "lrn@chromium.org" Date: Thu, 26 May 2011 07:35:09 +0000 Subject: [PATCH] Make RegExp objects not callable. Review URL: http://codereview.chromium.org/6930006 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8068 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/execution.cc | 17 ------------ src/ia32/lithium-codegen-ia32.cc | 8 ++---- src/objects.h | 6 ++--- src/runtime.cc | 2 +- test/mjsunit/json.js | 16 +++++------ test/mjsunit/number-string-index-call.js | 5 ++-- test/mjsunit/override-eval-with-non-function.js | 36 ------------------------- test/mjsunit/regexp-call-as-function.js | 2 +- test/mjsunit/regress/regress-603.js | 35 +++++++++++++++++------- test/mjsunit/regress/regress-752.js | 2 +- test/mjsunit/typeof.js | 8 +++--- test/mozilla/mozilla.status | 8 ++++++ test/sputnik/sputnik.status | 10 ------- 13 files changed, 55 insertions(+), 100 deletions(-) delete mode 100644 test/mjsunit/override-eval-with-non-function.js diff --git a/src/execution.cc b/src/execution.cc index e84ab9e..990eca2 100644 --- a/src/execution.cc +++ b/src/execution.cc @@ -210,23 +210,6 @@ Handle Execution::GetFunctionDelegate(Handle object) { // If you return a function from here, it will be called when an // attempt is made to call the given object as a function. - // Regular expressions can be called as functions in both Firefox - // and Safari so we allow it too. - if (object->IsJSRegExp()) { - Handle exec = factory->exec_symbol(); - // TODO(lrn): Bug 617. We should use the default function here, not the - // one on the RegExp object. - Object* exec_function; - { MaybeObject* maybe_exec_function = object->GetProperty(*exec); - // This can lose an exception, but the alternative is to put a failure - // object in a handle, which is not GC safe. - if (!maybe_exec_function->ToObject(&exec_function)) { - return factory->undefined_value(); - } - } - return Handle(exec_function); - } - // Objects created through the API can have an instance-call handler // that should be used when calling the object as a function. if (object->IsHeapObject() && diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 62f4c65..8d2dfd5 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -4281,17 +4281,13 @@ Condition LCodeGen::EmitTypeofIs(Label* true_label, } else if (type_name->Equals(heap()->function_symbol())) { __ JumpIfSmi(input, false_label); - __ CmpObjectType(input, JS_FUNCTION_TYPE, input); - __ j(equal, true_label); - // Regular expressions => 'function' (they are callable). - __ CmpInstanceType(input, JS_REGEXP_TYPE); - final_branch_condition = equal; + __ CmpObjectType(input, FIRST_FUNCTION_CLASS_TYPE, input); + final_branch_condition = above_equal; } else if (type_name->Equals(heap()->object_symbol())) { __ JumpIfSmi(input, false_label); __ cmp(input, factory()->null_value()); __ j(equal, true_label); - // Regular expressions => 'function', not 'object'. __ CmpObjectType(input, FIRST_JS_OBJECT_TYPE, input); __ j(below, false_label); __ CmpInstanceType(input, FIRST_FUNCTION_CLASS_TYPE); diff --git a/src/objects.h b/src/objects.h index 77f25d5..4c0e6ed 100644 --- a/src/objects.h +++ b/src/objects.h @@ -562,9 +562,9 @@ enum InstanceType { JS_GLOBAL_PROXY_TYPE, JS_ARRAY_TYPE, - JS_REGEXP_TYPE, // LAST_JS_OBJECT_TYPE, FIRST_FUNCTION_CLASS_TYPE + JS_REGEXP_TYPE, // LAST_JS_OBJECT_TYPE - JS_FUNCTION_TYPE, + JS_FUNCTION_TYPE, // FIRST_FUNCTION_CLASS_TYPE // Pseudo-types FIRST_TYPE = 0x0, @@ -583,7 +583,7 @@ enum InstanceType { LAST_JS_OBJECT_TYPE = JS_REGEXP_TYPE, // RegExp objects have [[Class]] "function" because they are callable. // All types from this type and above are objects with [[Class]] "function". - FIRST_FUNCTION_CLASS_TYPE = JS_REGEXP_TYPE + FIRST_FUNCTION_CLASS_TYPE = JS_FUNCTION_TYPE }; static const int kExternalArrayTypeCount = LAST_EXTERNAL_ARRAY_TYPE - diff --git a/src/runtime.cc b/src/runtime.cc index 922225f..56b7a90 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -4637,7 +4637,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_Typeof) { } ASSERT(heap_obj->IsUndefined()); return isolate->heap()->undefined_symbol(); - case JS_FUNCTION_TYPE: case JS_REGEXP_TYPE: + case JS_FUNCTION_TYPE: return isolate->heap()->function_symbol(); default: // For any kind of object not handled above, the spec rule for diff --git a/test/mjsunit/json.js b/test/mjsunit/json.js index 812ffeb..8b06a34 100644 --- a/test/mjsunit/json.js +++ b/test/mjsunit/json.js @@ -67,7 +67,7 @@ var d4 = {toJSON: Date.prototype.toJSON, valueOf: "not callable", toString: "not callable either", toISOString: function() { return 42; }}; -assertThrows("d4.toJSON()", TypeError); // ToPrimitive throws. +assertThrows("d4.toJSON()", TypeError); // ToPrimitive throws. var d5 = {toJSON: Date.prototype.toJSON, valueOf: "not callable", @@ -196,9 +196,6 @@ TestInvalid('"Unterminated string'); TestInvalid('"Unterminated string\\"'); TestInvalid('"Unterminated string\\\\\\"'); -// JavaScript RegExp literals not valid in JSON. -TestInvalid('/true/'); - // Test bad JSON that would be good JavaScript (ES5). TestInvalid("{true:42}"); TestInvalid("{false:42}"); @@ -382,7 +379,7 @@ var reJSON = /Is callable/; reJSON.toJSON = function() { return "has toJSON"; }; assertEquals( - '[37,null,1,"foo","37","true",null,"has toJSON",null,"has toJSON"]', + '[37,null,1,"foo","37","true",null,"has toJSON",{},"has toJSON"]', JSON.stringify([num37, numFoo, numTrue, strFoo, str37, strTrue, func, funcJSON, re, reJSON])); @@ -397,6 +394,9 @@ var callCount = 0; var counter = { get toJSON() { getCount++; return function() { callCount++; return 42; }; } }; + +// RegExps are not callable, so they are stringified as objects. +assertEquals('{}', JSON.stringify(/regexp/)); assertEquals('42', JSON.stringify(counter)); assertEquals(1, getCount); assertEquals(1, callCount); @@ -419,9 +419,9 @@ assertEquals('"42"', JSON.stringify(falseNum)); // We don't currently allow plain properties called __proto__ in JSON // objects in JSON.parse. Instead we read them as we would JS object // literals. If we change that, this test should change with it. -// -// Parse a non-object value as __proto__. This must not create a -// __proto__ property different from the original, and should not +// +// Parse a non-object value as __proto__. This must not create a +// __proto__ property different from the original, and should not // change the original. var o = JSON.parse('{"__proto__":5}'); assertEquals(Object.prototype, o.__proto__); // __proto__ isn't changed. diff --git a/test/mjsunit/number-string-index-call.js b/test/mjsunit/number-string-index-call.js index 4391917..85b79d1 100644 --- a/test/mjsunit/number-string-index-call.js +++ b/test/mjsunit/number-string-index-call.js @@ -25,8 +25,7 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -// Flags: --call_regexp -var callbacks = [ function() { return 'foo'; }, "nonobject", /abc/ ]; +var callbacks = [ function() {return 'foo'}, "nonobject", /abc/ ]; assertEquals('foo', callbacks['0']()); assertThrows("callbacks['1']()"); -assertEquals(['abc'], callbacks['2']("abcdefg")); +assertThrows("callbacks['2']('abcdefg')"); diff --git a/test/mjsunit/override-eval-with-non-function.js b/test/mjsunit/override-eval-with-non-function.js deleted file mode 100644 index edbcb19..0000000 --- a/test/mjsunit/override-eval-with-non-function.js +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2011 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. - -// When 'eval' is overridden with a non-function object we should -// check whether the object is callable. - -function test() { - eval = /foo/; - assertEquals(["foo"], eval("foobar")); -} - -test(); diff --git a/test/mjsunit/regexp-call-as-function.js b/test/mjsunit/regexp-call-as-function.js index 4cbe7f9..add81ac 100644 --- a/test/mjsunit/regexp-call-as-function.js +++ b/test/mjsunit/regexp-call-as-function.js @@ -33,4 +33,4 @@ var regexp = /a(b)(c)/; var subject = "xyzabcde"; var expected = 'abc,b,c'; assertEquals(expected, String(regexp.exec(subject))); -assertEquals(expected, String(regexp(subject))); +assertThrows(function(){ regexp(subject); }); diff --git a/test/mjsunit/regress/regress-603.js b/test/mjsunit/regress/regress-603.js index 7d4c322..f9344ee 100644 --- a/test/mjsunit/regress/regress-603.js +++ b/test/mjsunit/regress/regress-603.js @@ -29,21 +29,36 @@ // not mess up the stack. // http://code.google.com/p/v8/issues/detail?id=603 -function test0() { - var re = /b../; +var re = /b../; +assertThrows(function() { return re('abcdefghijklm') + 'z'; -} -assertEquals('bcdz', test0()); +}); var re1 = /c../; re1.call = Function.prototype.call; -var test1 = re1.call(null, 'abcdefghijklm') + 'z'; -assertEquals('cdez', test1); +assertThrows(function() { + re1.call(null, 'abcdefghijklm') + 'z'; +}); var re2 = /d../; -var test2 = Function.prototype.call.call(re2, null, 'abcdefghijklm') + 'z'; -assertEquals('defz', test2); +assertThrows(function() { + Function.prototype.call.call(re2, null, 'abcdefghijklm') + 'z'; +}); var re3 = /e../; -var test3 = Function.prototype.call.apply(re3, [null, 'abcdefghijklm']) + 'z'; -assertEquals('efgz', test3); +assertThrows(function() { + Function.prototype.call.apply( + re3, [null, 'abcdefghijklm']) + 'z'; +}); + +var re4 = /f../; +assertThrows(function() { + Function.prototype.apply.call( + re4, null, ['abcdefghijklm']) + 'z'; +}); + +var re5 = /g../; +assertThrows(function() { + Function.prototype.apply.apply( + re4, [null, ['abcdefghijklm']]) + 'z'; +}); diff --git a/test/mjsunit/regress/regress-752.js b/test/mjsunit/regress/regress-752.js index 1142a1f..d38870e 100644 --- a/test/mjsunit/regress/regress-752.js +++ b/test/mjsunit/regress/regress-752.js @@ -33,4 +33,4 @@ function replacer(key, value) { return value === 42 ? new Boolean(false) : value; } -assertEquals(JSON.stringify([42], replacer), "[false]"); +assertEquals("[false]", JSON.stringify([42], replacer)); diff --git a/test/mjsunit/typeof.js b/test/mjsunit/typeof.js index 39dec72..86f9e0d 100644 --- a/test/mjsunit/typeof.js +++ b/test/mjsunit/typeof.js @@ -29,10 +29,10 @@ // the context of string equality comparisons. var r = new RegExp; -assertEquals('function', typeof r); -assertTrue(typeof r == 'function'); +assertEquals('object', typeof r); +assertTrue(typeof r == 'object'); +assertFalse(typeof r == 'function'); function test(x, y) { return x == y; } -assertFalse(test('object', typeof r)); +assertTrue(test('object', typeof r)); -assertFalse(typeof r == 'object'); diff --git a/test/mozilla/mozilla.status b/test/mozilla/mozilla.status index e281e67..44b5577 100644 --- a/test/mozilla/mozilla.status +++ b/test/mozilla/mozilla.status @@ -432,6 +432,14 @@ js1_2/regexp/RegExp_lastIndex: FAIL_OK js1_2/regexp/string_split: FAIL_OK +# RegExps are not callable. +js1_2/regexp/simple_form: FAIL_OK +js1_2/regexp/regress-6359: FAIL_OK +js1_2/regexp/regress-9141: FAIL_OK +js1_5/Regress/regress-224956: FAIL_OK +js1_5/Regress/regress-325925: FAIL_OK +ecma_2/RegExp/regress-001: FAIL_OK + # We do not check for bad surrogate pairs when quoting strings. js1_5/Regress/regress-315974: FAIL_OK diff --git a/test/sputnik/sputnik.status b/test/sputnik/sputnik.status index f322840..472db6e 100644 --- a/test/sputnik/sputnik.status +++ b/test/sputnik/sputnik.status @@ -50,10 +50,6 @@ S15.8.2.13_A23: PASS || FAIL_OK S15.10.6.2_A1_T16: FAIL_OK S15.10.6.3_A1_T16: FAIL_OK -# We allow regexps to be called as functions for compatibility reasons. -S15.10.7_A1_T1: FAIL_OK -S15.10.7_A1_T2: FAIL_OK - # We are silent in some regexp cases where the spec wants us to give # errors, for compatibility. S15.10.2.11_A1_T2: FAIL @@ -244,12 +240,6 @@ S15.9.5.7_A1_T2: FAIL_OK S15.9.5.8_A1_T2: FAIL_OK S15.9.5.9_A1_T2: FAIL_OK -# Regexps have type "function", not "object". -S11.4.3_A3.6: FAIL_OK -S15.10.7_A3_T2: FAIL_OK -S15.10.7_A3_T1: FAIL_OK - - [ $arch == arm ] # BUG(3251225): Tests that timeout with --nocrankshaft. -- 2.7.4