Bug 34939 – Fix the SP at ctiOpThrowNotCaught on Thumb2 (JSVALUE32)

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Gabor Loki]

There is a typo inside ctiOpThrowNotCaught for JSValue32 on Thumb2.
The SP addition should be 0x40 (according to ctiTrampoline).

[Gabor Loki]

Created attachment 48747 [details]
Fix the SP at ctiOpThrowNotCaught on Thumb2 (JSVALUE32)

[Zoltan Herczeg]

Nice catch. Oh wait, this is a fix for not catch :)

[Gabor Loki]

> Nice catch. Oh wait, this is a fix for not catch :)

Hahaha. Thanks Pal! ;)

[Gavin Barraclough]

Comment on attachment 48747 [details]
Fix the SP at ctiOpThrowNotCaught on Thumb2 (JSVALUE32)

r+, but...

Here's one way you could make this awesomer.

The underlying problem here is that the hardcoded constants in the asm make this code really error prone - it's really easy to miss one of these & end up with a typo like this.  So.

The magic value 0x40 represents the amount of stack frame that needs to be dynamically allocated on entry through the cti trampoline stub.  We expect this to be equal to the offset of enabledProfilerReference within JITStackFrame (and assert such in the JITThunks constructor).

You could so something like:

    #define JIT_STACK_FRAME_ALLOCATION 0x40
    #define JIT_STACK_FRAME_ALLOCATION_string STRINGIZE(JIT_STACK_FRAME_ALLOCATION)

then change all the asm to use this string instead of the hardcoded constant, e.g.:

    "add sp, sp," JIT_STACK_FRAME_ALLOCATION_string "\n"

and make the assert in JITThunks::JITThunks be:

    // The fifth argument is the first item already on the stack.
    ASSERT(OBJECT_OFFSETOF(struct JITStackFrame, enabledProfilerReference) == JIT_STACK_FRAME_ALLOCATION);

Less had coded constants should hopefully make this code less fragile in the future.

r+ with or without the asm/ASSERT changes suggested.

[WebKit Commit Bot]

Comment on attachment 48747 [details]
Fix the SP at ctiOpThrowNotCaught on Thumb2 (JSVALUE32)

Clearing flags on attachment: 48747

Committed r54809: <http://trac.webkit.org/changeset/54809>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.