From cefbb1e7f8daf5d64c6a459208cc541e47fbbd45 Mon Sep 17 00:00:00 2001 From: "lrn@chromium.org" Date: Tue, 18 Oct 2011 12:26:53 +0000 Subject: [PATCH] Make bound functions have poisoned .caller and .arguments. Also makes func.caller return null if the caller is a bound function, matching JSC. Fix bug preventing poisoned setters from triggering. TEST=mjsunit/function-bind, mjsunit/strict-mode Review URL: http://codereview.chromium.org/8333019 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@9681 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/accessors.cc | 7 ++++++- src/bootstrapper.cc | 6 +++--- src/ic.cc | 2 +- src/v8natives.js | 2 ++ test/mjsunit/function-bind.js | 31 +++++++++++++++++++++++++++++++ test/mjsunit/strict-mode.js | 6 ++++++ test/sputnik/sputnik.status | 4 ---- test/test262/test262.status | 13 ------------- 8 files changed, 49 insertions(+), 22 deletions(-) diff --git a/src/accessors.cc b/src/accessors.cc index 951209d964..5c6c7ef00d 100644 --- a/src/accessors.cc +++ b/src/accessors.cc @@ -759,7 +759,12 @@ MaybeObject* Accessors::FunctionGetCaller(Object* object, void*) { caller = potential_caller; potential_caller = it.next(); } - + // If caller is bound, return null. This is compatible with JSC, and + // allows us to make bound functions use the strict function map + // and its associated throwing caller and arguments. + if (caller->shared()->bound()) { + return isolate->heap()->null_value(); + } return CheckNonStrictCallerOrThrow(isolate, caller); } diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 884f28e7ee..56acc86aa7 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -522,7 +522,7 @@ Handle Genesis::ComputeStrictFunctionInstanceDescriptor( ? 4 : 5); PropertyAttributes attributes = static_cast( - DONT_ENUM | DONT_DELETE | READ_ONLY); + DONT_ENUM | DONT_DELETE); { // length Handle foreign = factory()->NewForeign(&Accessors::FunctionLength); @@ -547,8 +547,8 @@ Handle Genesis::ComputeStrictFunctionInstanceDescriptor( // prototype if (prototypeMode != DONT_ADD_PROTOTYPE) { - if (prototypeMode == ADD_WRITEABLE_PROTOTYPE) { - attributes = static_cast(attributes & ~READ_ONLY); + if (prototypeMode != ADD_WRITEABLE_PROTOTYPE) { + attributes = static_cast(attributes | READ_ONLY); } Handle foreign = factory()->NewForeign(&Accessors::FunctionPrototype); diff --git a/src/ic.cc b/src/ic.cc index 947509c24e..107338caee 100644 --- a/src/ic.cc +++ b/src/ic.cc @@ -1413,6 +1413,7 @@ MaybeObject* StoreIC::Store(State state, return TypeError("strict_read_only_property", object, name); } // Ignore other stores where the receiver is not a JSObject. + // TODO(1475): Must check prototype chains of object wrappers. return *value; } @@ -1444,7 +1445,6 @@ MaybeObject* StoreIC::Store(State state, // Lookup the property locally in the receiver. if (FLAG_use_ic && !receiver->IsJSGlobalProxy()) { LookupResult lookup(isolate()); - if (LookupForWrite(*receiver, *name, &lookup)) { // Generate a stub for this store. UpdateCaches(&lookup, state, strict_mode, receiver, name, value); diff --git a/src/v8natives.js b/src/v8natives.js index de80067866..337349cad9 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -1520,6 +1520,8 @@ function FunctionBind(this_arg) { // Length is 1. throw new $TypeError('Bind must be called on a function'); } var boundFunction = function () { + // Poison .arguments and .caller, but is otherwise not detectable. + "use strict"; // This function must not use any object literals (Object, Array, RegExp), // since the literals-array is being used to store the bound data. if (%_IsConstructCall()) { diff --git a/test/mjsunit/function-bind.js b/test/mjsunit/function-bind.js index b6fcac4ad6..4a8f2d2a62 100644 --- a/test/mjsunit/function-bind.js +++ b/test/mjsunit/function-bind.js @@ -263,3 +263,34 @@ assertEquals(["foo", 0, undefined], s()); s = soo.bind(true); assertEquals([true, 0, undefined], s()); + +// Test that .arguments and .caller are poisoned according to the ES5 spec. + +// Check that property descriptors are correct (unconfigurable, unenumerable, +// and both get and set is the ThrowTypeError function). +var cdesc = Object.getOwnPropertyDescriptor(f, "caller"); +var adesc = Object.getOwnPropertyDescriptor(f, "arguments"); + +assertFalse(cdesc.enumerable); +assertFalse(cdesc.configurable); + +assertFalse(adesc.enumerable); +assertFalse(adesc.configurable); + +assertSame(cdesc.get, cdesc.set); +assertSame(cdesc.get, adesc.get); +assertSame(cdesc.get, adesc.set); + +assertTrue(cdesc.get instanceof Function); +assertEquals(0, cdesc.get.length); +assertThrows(cdesc.get, TypeError); + +assertThrows(function() { return f.caller; }, TypeError); +assertThrows(function() { f.caller = 42; }, TypeError); +assertThrows(function() { return f.arguments; }, TypeError); +assertThrows(function() { f.arguments = 42; }, TypeError); + +// Shouldn't throw. Accessing the functions caller must throw if +// the caller is strict and the callee isn't. A bound function is built-in, +// but not considered strict. +(function foo() { return foo.caller; }).bind()(); diff --git a/test/mjsunit/strict-mode.js b/test/mjsunit/strict-mode.js index 30234ba6fa..9c9bdfd52d 100644 --- a/test/mjsunit/strict-mode.js +++ b/test/mjsunit/strict-mode.js @@ -1051,14 +1051,20 @@ function CheckPillDescriptor(func, name) { } assertThrows(function() { strict.caller; }, TypeError); assertThrows(function() { strict.arguments; }, TypeError); + assertThrows(function() { strict.caller = 42; }, TypeError); + assertThrows(function() { strict.arguments = 42; }, TypeError); var another = new Function("'use strict'"); assertThrows(function() { another.caller; }, TypeError); assertThrows(function() { another.arguments; }, TypeError); + assertThrows(function() { another.caller = 42; }, TypeError); + assertThrows(function() { another.arguments = 42; }, TypeError); var third = (function() { "use strict"; return function() {}; })(); assertThrows(function() { third.caller; }, TypeError); assertThrows(function() { third.arguments; }, TypeError); + assertThrows(function() { third.caller = 42; }, TypeError); + assertThrows(function() { third.arguments = 42; }, TypeError); CheckPillDescriptor(strict, "caller"); CheckPillDescriptor(strict, "arguments"); diff --git a/test/sputnik/sputnik.status b/test/sputnik/sputnik.status index ac02d25e6a..ef546312bf 100644 --- a/test/sputnik/sputnik.status +++ b/test/sputnik/sputnik.status @@ -30,10 +30,6 @@ def FAIL_OK = FAIL, OKAY ############################### BUGS ################################### -# A bound function should fail on access to 'caller' and 'arguments'. -S15.3.4.5_A1: FAIL -S15.3.4.5_A2: FAIL - # '__proto__' should be treated as a normal property in JSON. S15.12.2_A1: FAIL diff --git a/test/test262/test262.status b/test/test262/test262.status index 57d83967af..5cbb26747a 100644 --- a/test/test262/test262.status +++ b/test/test262/test262.status @@ -30,10 +30,6 @@ def FAIL_OK = FAIL, OKAY ############################### BUGS ################################### -# A bound function should fail on access to 'caller' and 'arguments'. -S15.3.4.5_A1: FAIL -S15.3.4.5_A2: FAIL - # '__proto__' should be treated as a normal property in JSON. S15.12.2_A1: FAIL @@ -43,9 +39,6 @@ S8.7_A5_T2: FAIL # V8 Bug: http://code.google.com/p/v8/issues/detail?id=1624 S10.4.2.1_A1: FAIL -# V8 Bug. -S13.2.3_A1: FAIL - # V8 Bug: http://code.google.com/p/v8/issues/detail?id=1530 S15.3.3.1_A4: FAIL @@ -128,12 +121,6 @@ S15.3.3.1_A4: FAIL 15.2.3.7-6-a-176: FAIL 15.2.3.7-6-a-177: FAIL -# V8 Bug: http://code.google.com/p/v8/issues/detail?id=893 -15.3.4.5-20-2: FAIL -15.3.4.5-20-3: FAIL -15.3.4.5-21-2: FAIL -15.3.4.5-21-3: FAIL - # Invalid test cases (recent change adding var changes semantics) S8.3_A1_T1: FAIL S15.3_A3_T1: FAIL