Bug 180653 – LLInt: reserve 16 bytes of stack on MIPS for native calls

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

[Guillaume Emont]

The calling convention on MIPS is that the caller reserves 16 bytes (4 words) of stack space for all calls (regardless of the number of arguments). The callee can use that space. in LLInt's nativeCallTrampoline (which is only used if JIT is disabled), we effectively only save 8 bytes when adjusting the alignment. This crashes many date-related tests when running in llint-only mode when compiling in -Os.

[Guillaume Emont]

Note that it does not crash when the JIT is enabled, because it seems we use the thunk generated by JSC::nativeForGenerator() even when calling from llint. This thunk correctly allocates the 16 bytes of stack.

[Guillaume Emont]

Created attachment 328994 [details]
Patch

Patch fixing the issue.

[Adrian Perez]

Comment on attachment 328994 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=328994&action=review

Informal review: r+, with a comment. Please check it before landing.

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:2086
> +        # calling convention says to save stack space for 4 first registers in

Wow, good find. A similar scheme is used in the
“internalFunctionCallTrampoline()” macro but this patch does NOT
modify it accordingly. Is that intended?

[Guillaume Emont]

> 
> Wow, good find. A similar scheme is used in the
> “internalFunctionCallTrampoline()” macro but this patch does NOT
> modify it accordingly. Is that intended?

My understanding is that nativeCallTrampoline() is used to call C++ (native) functions, and internalFunctionCallTrampoline() is used to call js (internal) functions. Neither LLInt nor the JIT use that extra space, so we can save some memory by not allocating it.

[Adrian Perez]

(In reply to Guillaume Emont from comment #4)
> > 
> > Wow, good find. A similar scheme is used in the
> > “internalFunctionCallTrampoline()” macro but this patch does NOT
> > modify it accordingly. Is that intended?
> 
> My understanding is that nativeCallTrampoline() is used to call C++ (native)
> functions, and internalFunctionCallTrampoline() is used to call js
> (internal) functions. Neither LLInt nor the JIT use that extra space, so we
> can save some memory by not allocating it.

That was also my suspicion, okay then!

[Carlos Alberto Lopez Perez]

Comment on attachment 328994 [details]
Patch

r=me (trusting Adrian review)

[WebKit Commit Bot]

Comment on attachment 328994 [details]
Patch

Clearing flags on attachment: 328994

Committed r225788: <https://trac.webkit.org/changeset/225788>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35998684>