From 582cf097e90da5835bbb3456bef030e0c23defb9 Mon Sep 17 00:00:00 2001 From: "mmaly@chromium.org" Date: Tue, 15 Feb 2011 18:57:37 +0000 Subject: [PATCH] Strict mode "this" transformation in Function.call/Function.apply. In strict mode the transformation of "this" is skipped. Code review feedback. Testing memory operand against 8 bit IMM on ia32 and x64. Review URL: http://codereview.chromium.org/6524006 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6799 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/builtins-arm.cc | 18 +++++++++++ src/ia32/builtins-ia32.cc | 15 +++++++++ src/objects.h | 30 +++++++++++++++++- src/x64/builtins-x64.cc | 15 +++++++++ test/es5conform/es5conform.status | 51 +------------------------------ test/mjsunit/strict-mode.js | 42 +++++++++++++++++++++++++ 6 files changed, 120 insertions(+), 51 deletions(-) diff --git a/src/arm/builtins-arm.cc b/src/arm/builtins-arm.cc index f14d77af0a..6e8fe28a2b 100644 --- a/src/arm/builtins-arm.cc +++ b/src/arm/builtins-arm.cc @@ -1231,6 +1231,14 @@ void Builtins::Generate_FunctionCall(MacroAssembler* masm) { // Change context eagerly in case we need the global receiver. __ ldr(cp, FieldMemOperand(r1, JSFunction::kContextOffset)); + // Do not transform the receiver for strict mode functions. + __ ldr(r2, FieldMemOperand(r1, JSFunction::kSharedFunctionInfoOffset)); + __ ldr(r2, FieldMemOperand(r2, SharedFunctionInfo::kCompilerHintsOffset)); + __ tst(r2, Operand(1 << (SharedFunctionInfo::kStrictModeFunction + + kSmiTagSize))); + __ b(ne, &shift_arguments); + + // Compute the receiver in non-strict mode. __ add(r2, sp, Operand(r0, LSL, kPointerSizeLog2)); __ ldr(r2, MemOperand(r2, -kPointerSize)); // r0: actual number of arguments @@ -1394,10 +1402,20 @@ void Builtins::Generate_FunctionApply(MacroAssembler* masm) { // Change context eagerly to get the right global object if necessary. __ ldr(r0, MemOperand(fp, kFunctionOffset)); __ ldr(cp, FieldMemOperand(r0, JSFunction::kContextOffset)); + // Load the shared function info while the function is still in r0. + __ ldr(r1, FieldMemOperand(r0, JSFunction::kSharedFunctionInfoOffset)); // Compute the receiver. Label call_to_object, use_global_receiver, push_receiver; __ ldr(r0, MemOperand(fp, kRecvOffset)); + + // Do not transform the receiver for strict mode functions. + __ ldr(r1, FieldMemOperand(r1, SharedFunctionInfo::kCompilerHintsOffset)); + __ tst(r1, Operand(1 << (SharedFunctionInfo::kStrictModeFunction + + kSmiTagSize))); + __ b(ne, &push_receiver); + + // Compute the receiver in non-strict mode. __ tst(r0, Operand(kSmiTagMask)); __ b(eq, &call_to_object); __ LoadRoot(r1, Heap::kNullValueRootIndex); diff --git a/src/ia32/builtins-ia32.cc b/src/ia32/builtins-ia32.cc index 0a3e093056..f15fd1cd84 100644 --- a/src/ia32/builtins-ia32.cc +++ b/src/ia32/builtins-ia32.cc @@ -589,6 +589,13 @@ void Builtins::Generate_FunctionCall(MacroAssembler* masm) { // Change context eagerly in case we need the global receiver. __ mov(esi, FieldOperand(edi, JSFunction::kContextOffset)); + // Do not transform the receiver for strict mode functions. + __ mov(ebx, FieldOperand(edi, JSFunction::kSharedFunctionInfoOffset)); + __ test_b(FieldOperand(ebx, SharedFunctionInfo::kStrictModeByteOffset), + 1 << SharedFunctionInfo::kStrictModeBitWithinByte); + __ j(not_equal, &shift_arguments); + + // Compute the receiver in non-strict mode. __ mov(ebx, Operand(esp, eax, times_4, 0)); // First argument. __ test(ebx, Immediate(kSmiTagMask)); __ j(zero, &convert_to_object); @@ -736,6 +743,14 @@ void Builtins::Generate_FunctionApply(MacroAssembler* masm) { // Compute the receiver. Label call_to_object, use_global_receiver, push_receiver; __ mov(ebx, Operand(ebp, 3 * kPointerSize)); + + // Do not transform the receiver for strict mode functions. + __ mov(ecx, FieldOperand(edi, JSFunction::kSharedFunctionInfoOffset)); + __ test_b(FieldOperand(ecx, SharedFunctionInfo::kStrictModeByteOffset), + 1 << SharedFunctionInfo::kStrictModeBitWithinByte); + __ j(not_equal, &push_receiver); + + // Compute the receiver in non-strict mode. __ test(ebx, Immediate(kSmiTagMask)); __ j(zero, &call_to_object); __ cmp(ebx, Factory::null_value()); diff --git a/src/objects.h b/src/objects.h index 559ada63f4..d7acded2a2 100644 --- a/src/objects.h +++ b/src/objects.h @@ -4370,7 +4370,6 @@ class SharedFunctionInfo: public HeapObject { kThisPropertyAssignmentsOffset + kPointerSize, kSize> BodyDescriptor; - private: // Bit positions in start_position_and_type. // The source code start position is in the 30 most significant bits of // the start_position_and_type field. @@ -4389,6 +4388,35 @@ class SharedFunctionInfo: public HeapObject { static const int kOptimizationDisabled = 7; static const int kStrictModeFunction = 8; +private: +#if V8_HOST_ARCH_32_BIT + // On 32 bit platforms, compiler hints is a smi. + static const int kCompilerHintsSmiTagSize = kSmiTagSize; + static const int kCompilerHintsSize = kPointerSize; +#else + // On 64 bit platforms, compiler hints is not a smi, see comment above. + static const int kCompilerHintsSmiTagSize = 0; + static const int kCompilerHintsSize = kIntSize; +#endif + +public: + // Constants for optimizing codegen for strict mode function tests. + // Allows to use byte-widgh instructions. + static const int kStrictModeBitWithinByte = + (kStrictModeFunction + kCompilerHintsSmiTagSize) % kBitsPerByte; + +#if __BYTE_ORDER == __LITTLE_ENDIAN + static const int kStrictModeByteOffset = kCompilerHintsOffset + + (kStrictModeFunction + kCompilerHintsSmiTagSize) / kBitsPerByte; +#elif __BYTE_ORDER == __BIG_ENDIAN + static const int kStrictModeByteOffset = kCompilerHintsOffset + + (kCompilerHintsSize - 1) - + ((kStrictModeFunction + kCompilerHintsSmiTagSize) / kBitsPerByte); +#else +#error Unknown byte ordering +#endif + +private: DISALLOW_IMPLICIT_CONSTRUCTORS(SharedFunctionInfo); }; diff --git a/src/x64/builtins-x64.cc b/src/x64/builtins-x64.cc index d75091375d..c362f7b79f 100644 --- a/src/x64/builtins-x64.cc +++ b/src/x64/builtins-x64.cc @@ -651,6 +651,13 @@ void Builtins::Generate_FunctionCall(MacroAssembler* masm) { // Change context eagerly in case we need the global receiver. __ movq(rsi, FieldOperand(rdi, JSFunction::kContextOffset)); + // Do not transform the receiver for strict mode functions. + __ movq(rbx, FieldOperand(rdi, JSFunction::kSharedFunctionInfoOffset)); + __ testb(FieldOperand(rbx, SharedFunctionInfo::kStrictModeByteOffset), + Immediate(1 << SharedFunctionInfo::kStrictModeBitWithinByte)); + __ j(not_equal, &shift_arguments); + + // Compute the receiver in non-strict mode. __ movq(rbx, Operand(rsp, rax, times_pointer_size, 0)); __ JumpIfSmi(rbx, &convert_to_object); @@ -807,6 +814,14 @@ void Builtins::Generate_FunctionApply(MacroAssembler* masm) { // Compute the receiver. Label call_to_object, use_global_receiver, push_receiver; __ movq(rbx, Operand(rbp, kReceiverOffset)); + + // Do not transform the receiver for strict mode functions. + __ movq(rdx, FieldOperand(rdi, JSFunction::kSharedFunctionInfoOffset)); + __ testb(FieldOperand(rdx, SharedFunctionInfo::kStrictModeByteOffset), + Immediate(1 << SharedFunctionInfo::kStrictModeBitWithinByte)); + __ j(not_equal, &push_receiver); + + // Compute the receiver in non-strict mode. __ JumpIfSmi(rbx, &call_to_object); __ CompareRoot(rbx, Heap::kNullValueRootIndex); __ j(equal, &use_global_receiver); diff --git a/test/es5conform/es5conform.status b/test/es5conform/es5conform.status index d7b10a88cf..e021fc54dc 100644 --- a/test/es5conform/es5conform.status +++ b/test/es5conform/es5conform.status @@ -239,15 +239,6 @@ chapter15/15.10/15.10.7/15.10.7.5/15.10.7.5-2: FAIL_OK # Incorrect test - need double escape in eval. chapter07/7.8/7.8.4/7.8.4-1-s: FAIL -# this is not coerced to an object in strict mode (Number) -chapter10/10.4/10.4.3/10.4.3-1-1-s: FAIL -# this is not coerced to an object in strict mode (string) -chapter10/10.4/10.4.3/10.4.3-1-2-s: FAIL -# this is not coerced to an object in strict mode (undefined) -chapter10/10.4/10.4.3/10.4.3-1-3-s: FAIL -# this is not coerced to an object in strict mode (boolean) -chapter10/10.4/10.4.3/10.4.3-1-4-s: FAIL - # arguments[i] remains same after changing actual parameters in strict mode chapter10/10.6/10.6-10-c-ii-1-s: FAIL # arguments[i] doesn't map to actual parameters in strict mode @@ -436,53 +427,13 @@ chapter13/13.1/13.1-3-11-s: FAIL # Test fails to return true on success (invalid test case). chapter13/13.1/13.1-3-12-s: FAIL -# 'use strict' directive - correct usage -# depends on "this is not coerced to an object in strict mode (undefined)" -chapter14/14.1/14.1-1-s: FAIL -# "use strict" directive - correct usage double quotes -# depends on "this is not coerced to an object in strict mode (undefined)" -chapter14/14.1/14.1-2-s: FAIL -# 'use strict' directive - may follow other directives -# depends on "this is not coerced to an object in strict mode (undefined)" -chapter14/14.1/14.1-8-s: FAIL -# 'use strict' directive - may occur multiple times -# depends on "this is not coerced to an object in strict mode (undefined)" -chapter14/14.1/14.1-9-s: FAIL -# other directives - may follow 'use strict' directive -# depends on "this is not coerced to an object in strict mode (undefined)" -chapter14/14.1/14.1-10-s: FAIL -# comments may preceed 'use strict' directive -# depends on "this is not coerced to an object in strict mode (undefined)" -chapter14/14.1/14.1-11-s: FAIL -# comments may follow 'use strict' directive -# depends on "this is not coerced to an object in strict mode (undefined)" -chapter14/14.1/14.1-12-s: FAIL -# semicolon insertion works for'use strict' directive -# depends on "this is not coerced to an object in strict mode (undefined)" -chapter14/14.1/14.1-13-s: FAIL -# semicolon insertion may come before 'use strict' directive -# depends on "this is not coerced to an object in strict mode (undefined)" -chapter14/14.1/14.1-14-s: FAIL -# blank lines may come before 'use strict' directive -# depends on "this is not coerced to an object in strict mode (undefined)" -chapter14/14.1/14.1-15-s: FAIL - # Duplicate combined parameter name allowed in Function constructor called # in strict mode if body not strict # Test fails to return true on success (invalid test case). chapter15/15.3/15.3.2/15.3.2.1/15.3.2.1-11-6-s: FAIL -# Array.prototype.every - thisArg not passed to strict callbackfn -chapter15/15.4/15.4.4/15.4.4.16/15.4.4.16-5-1-s: FAIL -# Array.prototype.some - thisArg not passed to strict callbackfn -chapter15/15.4/15.4.4/15.4.4.17/15.4.4.17-5-1-s: FAIL -# Array.prototype.forEach - thisArg not passed to strict callbackfn -chapter15/15.4/15.4.4/15.4.4.18/15.4.4.18-5-1-s: FAIL -# Array.prototype.map - thisArg not passed to strict callbackfn -chapter15/15.4/15.4.4/15.4.4.19/15.4.4.19-5-1-s: FAIL -# Array.prototype.filter - thisArg not passed to strict callbackfn -chapter15/15.4/15.4.4/15.4.4.20/15.4.4.20-5-1-s: FAIL # Array.prototype.reduce - null passed as thisValue to strict callbackfn +# Invalid test case: http://es5conform.codeplex.com/workitem/29085 chapter15/15.4/15.4.4/15.4.4.21/15.4.4.21-9-c-ii-4-s: FAIL [ $arch == mips ] diff --git a/test/mjsunit/strict-mode.js b/test/mjsunit/strict-mode.js index fead64ad25..fbba64ed66 100644 --- a/test/mjsunit/strict-mode.js +++ b/test/mjsunit/strict-mode.js @@ -436,3 +436,45 @@ repeat(10, function() { testAssignToUndefined(false); }); assertThrows(function() { delete_element(object, 3.14); }, TypeError); assertEquals(object[3.14], "pi"); })(); + +// Not transforming this in Function.call and Function.apply. +(function testThisTransform() { + function non_strict() { + return this; + } + function strict() { + "use strict"; + return this; + } + + var global_object = (function() { return this; })(); + var object = {}; + + // Non-strict call. + assertTrue(non_strict.call(null) === global_object); + assertTrue(non_strict.call(undefined) === global_object); + assertEquals(typeof non_strict.call(7), "object"); + assertEquals(typeof non_strict.call("Hello"), "object"); + assertTrue(non_strict.call(object) === object); + + // Non-strict apply. + assertTrue(non_strict.apply(null) === global_object); + assertTrue(non_strict.apply(undefined) === global_object); + assertEquals(typeof non_strict.apply(7), "object"); + assertEquals(typeof non_strict.apply("Hello"), "object"); + assertTrue(non_strict.apply(object) === object); + + // Strict call. + assertTrue(strict.call(null) === null); + assertTrue(strict.call(undefined) === undefined); + assertEquals(typeof strict.call(7), "number"); + assertEquals(typeof strict.call("Hello"), "string"); + assertTrue(strict.call(object) === object); + + // Strict apply. + assertTrue(strict.apply(null) === null); + assertTrue(strict.apply(undefined) === undefined); + assertEquals(typeof strict.apply(7), "number"); + assertEquals(typeof strict.apply("Hello"), "string"); + assertTrue(strict.apply(object) === object); +})();