Bug 212104 - Checkpoint inlined call return handler needs an exception check when dispatching
Summary: Checkpoint inlined call return handler needs an exception check when dispatching
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-19 13:24 PDT by Keith Miller
Modified: 2020-05-22 11:01 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.02 KB, patch)
2020-05-19 13:25 PDT, Keith Miller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2020-05-19 13:24:33 PDT
Checkpoint inlined call return handler needs an exception check when dispatching
Comment 1 Keith Miller 2020-05-19 13:25:05 PDT
Created attachment 399765 [details]
Patch
Comment 2 Yusuke Suzuki 2020-05-19 13:29:46 PDT
Comment on attachment 399765 [details]
Patch

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

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:2093
> +    if (scope.exception())
> +        return encodeResult(returnToThrow(scope.vm()), nullptr);
> +

Can we insert exception check just after the operation which throws an exception? Maybe, handleIteratorNextCheckpoint?
Comment 3 Keith Miller 2020-05-19 13:31:06 PDT
(In reply to Yusuke Suzuki from comment #2)
> Comment on attachment 399765 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=399765&action=review
> 
> > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:2093
> > +    if (scope.exception())
> > +        return encodeResult(returnToThrow(scope.vm()), nullptr);
> > +
> 
> Can we insert exception check just after the operation which throws an
> exception? Maybe, handleIteratorNextCheckpoint?

No, because the LLInt code we're returning to dispatches to whatever pc we return. It's easier to handle it all here. This also means that other new bytecodes "just work".
Comment 4 Keith Miller 2020-05-19 13:31:15 PDT
rdar://problem/63268700
Comment 5 Yusuke Suzuki 2020-05-19 13:34:35 PDT
Comment on attachment 399765 [details]
Patch

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

>>> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:2093
>>> +
>> 
>> Can we insert exception check just after the operation which throws an exception? Maybe, handleIteratorNextCheckpoint?
> 
> No, because the LLInt code we're returning to dispatches to whatever pc we return. It's easier to handle it all here. This also means that other new bytecodes "just work".

OK, make sense.
Comment 6 EWS 2020-05-22 11:01:25 PDT
Committed r262064: <https://trac.webkit.org/changeset/262064>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399765 [details].