Bug 206726 – [JSC] DFG OSR exit is not marking CodeBlock::m_hasLinkedOSRExit when the exit target is checkpoint

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] DFG OSR exit is not marking CodeBlock::m_hasLinkedOSRExit when the exit target is checkpoint

[Yusuke Suzuki]

Created attachment 388649 [details]
Patch

[Yusuke Suzuki]

<rdar://problem/58827849>

[Yusuke Suzuki]

Created attachment 388650 [details]
Patch

[Mark Lam]

Comment on attachment 388650 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:16
> +            1. The caller CodeBlock "A" has Baseline code.
> +            2. Compile DFG code exiting to the checkpoint of "A". We are not marking "A"'s CodeBlock::m_hasLinkedOSRExit.
> +            3. GC happens and we decide dropping Baseline code for "A" since it is not marked. Switching it to LLInt.
> +            4. However, DFG OSR exit code is compiled by assuming that "A" is Baseline. So LLInt registers are not recovered correctly.
> +            5. Then, exiting to LLInt of "A", LLInt sees that LLInt registers have garbage.

Can you clarify the following:
1. GC cannot drop baseline code while the OSR exit compiler is running.  Is that true?
2. the same compiled OSR exit ramp can be used multiple times after it is compiled, and the GC can drop the baseline code between invocations of the OSR exit ramp.  Is that true?

If the OSR exit ramp is compiled to jump into the baseline code, and the baseline code has been deleted, how does the ramp adapt and exit into the LLInt instead?  I see the LLInt VM state and register being set up in the exit ramp, but I don't see how the ramp switches between jumping into the baseline code address vs the LLInt.  Am I missing something?  Thanks.

[Saam Barati]

Comment on attachment 388650 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:18
> +        While marking CodeBlock::m_hasLinkedOSRExit can fix the problem, this is a tricky fix since it relies on the heuristics we introduced about throwing

I don’t see why this isn’t the entire fix. That bit isn’t a heuristic. It’s required for the correctness of throwing away baseline code. Always recovering the registers isn’t needed. I feel like we should do only what’s required.

[Saam Barati]

(In reply to Mark Lam from comment #4)
> Comment on attachment 388650 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388650&action=review
> 
> > Source/JavaScriptCore/ChangeLog:16
> > +            1. The caller CodeBlock "A" has Baseline code.
> > +            2. Compile DFG code exiting to the checkpoint of "A". We are not marking "A"'s CodeBlock::m_hasLinkedOSRExit.
> > +            3. GC happens and we decide dropping Baseline code for "A" since it is not marked. Switching it to LLInt.
> > +            4. However, DFG OSR exit code is compiled by assuming that "A" is Baseline. So LLInt registers are not recovered correctly.
> > +            5. Then, exiting to LLInt of "A", LLInt sees that LLInt registers have garbage.
> 
> Can you clarify the following:
> 1. GC cannot drop baseline code while the OSR exit compiler is running.  Is
> that true?
> 2. the same compiled OSR exit ramp can be used multiple times after it is
> compiled, and the GC can drop the baseline code between invocations of the
> OSR exit ramp.  Is that true?
> 
> If the OSR exit ramp is compiled to jump into the baseline code, and the
> baseline code has been deleted, how does the ramp adapt and exit into the
> LLInt instead?  I see the LLInt VM state and register being set up in the
> exit ramp, but I don't see how the ramp switches between jumping into the
> baseline code address vs the LLInt.  Am I missing something?  Thanks.
 We don’t throw away baseline code once we set the “I have an OSR exit jumping to me” bit.

[Saam Barati]

Comment on attachment 388650 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:151
> +            baselineCodeBlockForCaller->m_hasLinkedOSRExit = true;

This is all that’s needed, right?

[Yusuke Suzuki]

Created attachment 388654 [details]
Patch

[Yusuke Suzuki]

Committed r255055: <https://trac.webkit.org/changeset/255055>