Reviewers: Mads Ager, Message: Mads,
Can you take a look this fix? Description: A simple fix of issue http://code.google.com/p/chromium/issues/detail?id=3285 NPN_Construct allows a NPObject to be called as a construct. For example, the test case var s = new app.Packages.java.lang.Integer(5); app.Packages.java.lang.Integer is a NPObject, and it implements NPN_Construct. This fix allows a JSObject created by an API function be called as a construct if it can be called as a function. This is done by generating the same code for var s = new app.Packages.java.lang.Integer(5); as var s = app.Packages.java.lang.Integer(5); and the caller handles both case correctly. A more sophiscated fix is to one extra JSConstructCall frame and allow CallAsConstructor in Builtin::HandleApiCallAsFunction. This change itself shouldn't affect the semantic of normal case such as: var a = {}; var s = new a(); A TypeError exception will be thrown in CALL_NON_FUNCTION (runtime.js). Another part of fix is in the binding code, V8NPObject, which makes NPN_InvokeDefault or NPN_Construct call depending on which function is available. Please review this at http://codereview.chromium.org/60035 SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/builtins-ia32.cc M test/cctest/test-api.cc Index: test/cctest/test-api.cc =================================================================== --- test/cctest/test-api.cc (revision 1658) +++ test/cctest/test-api.cc (working copy) @@ -4557,11 +4557,6 @@ value = Script::Compile(v8_str(call_17))->Run(); CHECK(!try_catch.HasCaught()); CHECK_EQ(17, value->Int32Value()); - - // Try something that will cause an exception: Call the object as a - // constructor. This should be the last test. - value = Script::Compile(v8_str("new obj(42)"))->Run(); - CHECK(try_catch.HasCaught()); } Index: src/builtins-ia32.cc =================================================================== --- src/builtins-ia32.cc (revision 1658) +++ src/builtins-ia32.cc (working copy) @@ -54,9 +54,16 @@ // -- edi: constructor function // ----------------------------------- + Label non_function_call; + // Check that function is not a Smi. + __ test(edi, Immediate(kSmiTagMask)); + __ j(zero, &non_function_call); + // Check that function is a JSFunction + __ CmpObjectType(edi, JS_FUNCTION_TYPE, ecx); + __ j(not_equal, &non_function_call); + // Enter a construct frame. __ EnterConstructFrame(); - // Store a smi-tagged arguments count on the stack. __ shl(eax, kSmiTagSize); __ push(eax); @@ -73,12 +80,6 @@ ExternalReference::debug_step_in_fp_address(); __ cmp(Operand::StaticVariable(debug_step_in_fp), Immediate(0)); __ j(not_equal, &rt_call); - // Check that function is not a Smi. - __ test(edi, Immediate(kSmiTagMask)); - __ j(zero, &rt_call); - // Check that function is a JSFunction - __ CmpObjectType(edi, JS_FUNCTION_TYPE, eax); - __ j(not_equal, &rt_call); // Verified that the constructor is a JSFunction. // Load the initial map and verify that it is in fact a map. @@ -300,6 +301,15 @@ __ lea(esp, Operand(esp, ebx, times_2, 1 * kPointerSize)); // 1 ~ receiver __ push(ecx); __ ret(0); + + // edi: called object + // eax: number of arguments + __ bind(&non_function_call); + + __ xor_(ebx, Operand(ebx)); + __ GetBuiltinEntry(edx, Builtins::CALL_NON_FUNCTION); + __ jmp(Handle<Code>(builtin(ArgumentsAdaptorTrampoline)), + RelocInfo::CODE_TARGET); } --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
