481296 - TM: develop proper fix for compiling tree calls with extra args on pending frame.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Graydon Hoare :graydon]

Bug 480244 exposed a mistake in the way we account for the ownership of frame contents: in particular, arguments are considered owned-by the callee frame, not the caller frame, when it comes to depositing information about stack synthesis in fragments. A minimal case exhibiting the behavior is shown in bug 480244 comment 2.

Unfortunately this means that when you have a treecall occurring at the point where the callee-tree has a callee-function receiving extra arguments (beyond those it expects), it becomes impossible to convey what the caller tree and caller function knows (that it's providing extra args) to the callee, since the callee has integrated the stack layout and typemap for a fixed argument set into itself. Passing the extra arguments where unexpected causes random memory corruption during the callee-tree import; omitting the extra arguments causes frame re-synthesis to fail when returning to the caller's bytecode, that expects the extra arguments to be present.

That bug was "solved" by papering-over, to some extent: we detect the condition and abort recording. The correct solution is to overhaul how we divide up ownership of the boundary slots between frames, to make the caller own the arguments.

This is lower-priority though. It's not blocking and the current behavior is safe and correct. It just hurts performance to be aborting in cases that cause it: outer loops fall apart and we lose a nesting opportunity, such as the pretty raytrace example, which spends a lot of time entering and leaving the innermost traces.

Comment 1

[Brendan Eich [:brendan]]

(In reply to comment #0)
> Bug 480244 exposed a mistake in the way we account for the ownership of frame
> contents: in particular, arguments are considered owned-by the callee frame,
> not the caller frame, when it comes to depositing information about stack
> synthesis in fragments.

Nit pick (I hope I have this right): it's only for the entry frame to a trace that callee frame owns arguments (including the callee and this at argv[-2] and argv[-1]). After the entry frame up to calldepth, the caller owns actual args as part of its operand stack, as far as tracing stack analysis goes.

Since the caller does own the arguments for all but the entry frame, the fix this bug proposes really, it seems to me, amounts to making a pseudo-frame or "stub" frame below the entry frame, to hold some (not all) of the operands -- those that are the actual arguments (including callee and this).

/be

Comment 2

[Graydon Hoare :graydon]

(In reply to comment #1)
> (In reply to comment #0)
> > Bug 480244 exposed a mistake in the way we account for the ownership of frame
> > contents: in particular, arguments are considered owned-by the callee frame,
> > not the caller frame, when it comes to depositing information about stack
> > synthesis in fragments.
> 
> Nit pick (I hope I have this right): 

Please! It was my hope (along with simply writing something down so I'd remember what on earth this bug was about) to provoke some correction or suggestion on the correct fix, by spewing my current (semi)understanding of the situation. 

Your suggestion sounds a lot easier than what I was worrying about :)

Comment 3

[David Mandelin [:dmandelin]]

I believe this was fixed by the fix to bug 491620. I confirmed by finding that the test case in bug 484614 doesn't abort any more. Feel free to reopen if I'm wrong.

Comment 4

[Graydon Hoare :graydon]

I think you're right, and we should delete the #if 0'ed code that the argc-keying now covers for.

Comment 5

[David Mandelin [:dmandelin]]

I just checked, and it turns out I did delete the #if 0'd code when I landed the other fix.