Bug 212104

Summary: Checkpoint inlined call return handler needs an exception check when dispatching
Product: WebKit Reporter: Keith Miller <keith_miller>
Component: New BugsAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Keith Miller
Reported 2020-05-19 13:24:33 PDT
Checkpoint inlined call return handler needs an exception check when dispatching
Attachments
Patch (5.02 KB, patch)
2020-05-19 13:25 PDT, Keith Miller 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Keith Miller
Comment 1 2020-05-19 13:25:05 PDT
Created attachment 399765 [details] Patch
Yusuke Suzuki
Comment 2 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?
Keith Miller
Comment 3 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".
Keith Miller
Comment 4 2020-05-19 13:31:15 PDT
rdar://problem/63268700
Yusuke Suzuki
Comment 5 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.
EWS
Comment 6 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].
Note You need to log in before you can comment on or make changes to this bug.