Bug 48485 – Crash in Function.prototype.call.apply

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Charles L]

Created attachment 72114 [details]
Crash test 

The browser crashes when the following function is run:

Function.prototype.call.apply(fn, arguments)

where fn is any function, and the arguments object has at least 3 items.

I have experienced this only in recent nightly builds, using Mac OS X 10.5.8. The crash doesn't happen while the script debugger is on, or when it is run from the location bar, or when fn has been successfully called before.

[Alexey Proskuryakov]

Confirmed with a local debug build of r70400.

#0	0x101ccc9d0 in WTF::RefPtr<JSC::JSGlobalData>::get at RefPtr.h:59
#1	0x101d6cc95 in JSC::JSGlobalObject::globalData at JSGlobalObject.h:279
#2	0x101ce66e9 in JSC::Parser::parse<JSC::FunctionBodyNode> at Parser.h:87
#3	0x101cdee51 in JSC::FunctionExecutable::compileForCallInternal at Executable.cpp:181
#4	0x101c78d00 in JSC::FunctionExecutable::compileForCall at Executable.h:315
#5	0x101cf1bae in JSC::Interpreter::executeCall at Interpreter.cpp:795
#6	0x101ca909f in JSC::call at CallData.cpp:38
#7	0x101ceb580 in JSC::functionProtoFuncCall at FunctionPrototype.cpp:147
#8	0x5711998001aa in ??
#9	0x101cf6a86 in JSC::JITCode::execute at JITCode.h:77
#10	0x101cf2bf1 in JSC::Interpreter::execute at Interpreter.cpp:746
#11	0x101cc26c7 in JSC::evaluate at Completion.cpp:63
#12	0x10310b4a0 in WebCore::JSMainThreadExecState::evaluate at JSMainThreadExecState.h:54

[Geoffrey Garen]

<rdar://problem/8606066>

[Zoltan Herczeg]

Interpreter::executeCall

    CallFrame* newCallFrame = CallFrame::create(oldEnd);
    size_t dst = 0;
    newCallFrame->r(0) = thisValue;
    ArgList::const_iterator end = args.end();
    for (ArgList::const_iterator it = args.begin(); it != end; ++it)
        newCallFrame->r(++dst) = *it;

oldEnd < callFrame, so newCallFrame->r(...) overwrites callframe fields. I will continue the debugging tomorrow morning. Hopefully I could find a fix.

[Zoltan Herczeg]

Hi guys,

actually I am little confused with this bug as I don't exactly remember how JS call system should work, and I need a little help.

This is a simplified example:

function g() {
    Function.prototype.call.apply(g, arguments);
}

g(0, 0, 0)

The byte-code of function g():

[   0] enter
[   1] init_lazy_reg     r1
[   3] init_lazy_reg     r0
[   5] resolve_global    r2, Function(@id0)
[  10] get_by_id         r2, r2, prototype(@id1)
[  18] get_by_id         r2, r2, call(@id2)
[  26] get_by_id         r3, r2, apply(@id3)
[  34] jneq_ptr          r3, r-1220017856, 22(->56)
[  38] mov               r4, r2
[  41] get_global_var    r6, -6
[  44] mov               r7, r1
[  47] load_varargs              r5, r7
[  50] call_varargs      r4, r5, 12
[  54] jmp               17(->71)
[  56] mov               r4, r2
[  59] get_global_var    r5, -6
[  62] create_arguments  r1
[  64] mov               r6, r1
[  67] call              r3, 3, 13
[  71] ret               undefined(@k0)

Before the "enter", the RegisterFile is extended, since the 'g' expects 0 parameter and got 3. Thus op_call_arityCheck() set the registerFile->m_end to 0x..100 (memory returned by mmap is always page aligned, thus the last 3 digit is always the same). As far as I remember, this should be the end of the memory used by the JS functon, shouldn't it?

The emit_op_load_varargs does not call the helper-stub function, it just extend %edi (call frame ptr) to 0x...118, which is 3 registers beyond the end of the registerFile, and should be never written by JIT code, shoudn't it?

call_varargs pass %edi as the call frame, and in Interpreter::executeCall oldEnd still points to 0x..100, and the callFrame to 0x...118. (I think an assert would be useful here).

    for (ArgList::const_iterator it = args.begin(); it != end; ++it)
        newCallFrame->r(++dst) = *it;

The 'for' above overwrites callFrame members (still in Interpreter::executeCall).

As far as I remember, registerFile->m_end points after the last available register, and no writes should happen after it. Thus I would blame emit_op_load_varargs at the moment, but I would be curious about your opinion.

[Oliver Hunt]

I don't see any error in load_varargs, i see the badness in call_varargs, the callframe register is extended to include the registerOffset, but loadvarargs has not bounds checked that extension, only the space for arguments.

[Oliver Hunt]

I have a fix, just building webkit

[Oliver Hunt]

Created attachment 72882 [details]
Patch

[Oliver Hunt]

Committed r71280: <http://trac.webkit.org/changeset/71280>

[Charles L]

Thank you!