Bug 147639 - jsc-tailcall: Exceptions should properly restore callee save register contents during unwinding
Summary: jsc-tailcall: Exceptions should properly restore callee save register content...
Status: RESOLVED DUPLICATE of bug 148666
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on: 147640
Blocks: 146845 147747
  Show dependency treegraph
 
Reported: 2015-08-04 11:45 PDT by Michael Saboff
Modified: 2015-09-14 10:58 PDT (History)
1 user (show)

See Also:


Attachments
Patch (37.03 KB, patch)
2015-08-06 14:35 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2015-08-04 11:45:57 PDT
When we unwind to a catch block or uncaught exception code, callee save registers should be restored to their proper values.
Comment 1 Michael Saboff 2015-08-06 14:35:15 PDT
Created attachment 258398 [details]
Patch
Comment 2 Basile Clement 2015-08-06 15:22:50 PDT
Comment on attachment 258398 [details]
Patch

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

LGTM.

> Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp:260
> +    m_jit.emitMaterializeTagCheckRegisters();
> +

I'd add a comment to state that we need the tag check registers to be materialized for the recoveries below.

> Source/JavaScriptCore/jit/AssemblyHelpers.h:228
> +        move(TrustedImmPtr(m_vm), temp1);
> +        addPtr(TrustedImm32(VM::calleeSaveRegistersBufferOffset()), temp1);

Why not just move(TrustedImmPtr(m_vm->calleeSaveRegistersBuffer), temp1);?

> Source/JavaScriptCore/jit/AssemblyHelpers.h:243
> +        move(TrustedImmPtr(m_vm), temp1);
> +        addPtr(TrustedImm32(VM::calleeSaveRegistersBufferOffset()), temp1);

Ditto.

> Source/JavaScriptCore/jit/AssemblyHelpers.h:263
> +        move(TrustedImmPtr(m_vm), temp1);
> +        addPtr(TrustedImm32(VM::calleeSaveRegistersBufferOffset()), temp1);

Ditto.
Comment 3 Michael Saboff 2015-08-06 15:41:47 PDT
(In reply to comment #2)
> Comment on attachment 258398 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258398&action=review
> 
> LGTM.
> 
> > Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp:260
> > +    m_jit.emitMaterializeTagCheckRegisters();
> > +
> 
> I'd add a comment to state that we need the tag check registers to be
> materialized for the recoveries below.
> 
> > Source/JavaScriptCore/jit/AssemblyHelpers.h:228
> > +        move(TrustedImmPtr(m_vm), temp1);
> > +        addPtr(TrustedImm32(VM::calleeSaveRegistersBufferOffset()), temp1);
> 
> Why not just move(TrustedImmPtr(m_vm->calleeSaveRegistersBuffer), temp1);?
> 
> > Source/JavaScriptCore/jit/AssemblyHelpers.h:243
> > +        move(TrustedImmPtr(m_vm), temp1);
> > +        addPtr(TrustedImm32(VM::calleeSaveRegistersBufferOffset()), temp1);
> 
> Ditto.
> 
> > Source/JavaScriptCore/jit/AssemblyHelpers.h:263
> > +        move(TrustedImmPtr(m_vm), temp1);
> > +        addPtr(TrustedImm32(VM::calleeSaveRegistersBufferOffset()), temp1);
> 
> Ditto.

I made the suggested changes locally.
Comment 4 Michael Saboff 2015-08-06 15:42:02 PDT
Committed r188071: <http://trac.webkit.org/changeset/188071>
Comment 5 Basile Clement 2015-08-31 18:08:17 PDT

*** This bug has been marked as a duplicate of bug 148666 ***
Comment 6 Csaba Osztrogonác 2015-09-14 10:58:49 PDT
Comment on attachment 258398 [details]
Patch

Cleared review? from attachment 258398 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).