| Summary: | catch_mach_exception_raise_state() should fail early if the faulting address is not of interest. | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||||
| Component: | Web Template Framework | Assignee: | Mark Lam <mark.lam> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, ews-watchlist, msaboff, saam, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Mark Lam
2020-05-12 13:07:42 PDT
Created attachment 399166 [details]
proposed patch.
Let's try this on the EWS.
Comment on attachment 399166 [details]
proposed patch.
r=me
Comment on attachment 399166 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=399166&action=review > Source/WTF/wtf/threads/Signals.cpp:178 > +#if CPU(ADDRESS64) && (CPU(ARM64) || CPU(X86_64)) We already specify valid pointer width in some WTF header. You should use that (In reply to Saam Barati from comment #3) > Comment on attachment 399166 [details] > proposed patch. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=399166&action=review > > > Source/WTF/wtf/threads/Signals.cpp:178 > > +#if CPU(ADDRESS64) && (CPU(ARM64) || CPU(X86_64)) > > We already specify valid pointer width in some WTF header. You should use > that WTF_OS_CONSTANT_EFFECTIVE_ADDRESS_WIDTH Created attachment 399182 [details]
proposed patch.
Comment on attachment 399182 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=399182&action=review r=me > Source/WTF/wtf/threads/Signals.cpp:181 > + if ((exceptionType == EXC_BAD_ACCESS) && (exceptionData[1] & invalidAddressMask)) can this code be moved below so we can use "faultingAddress" below? > Source/WTF/wtf/threads/Signals.cpp:183 > + compilerFence(); this doesn't seem necessary Created attachment 399189 [details]
proposed patch.
Comment on attachment 399189 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=399189&action=review > Source/WTF/wtf/threads/Signals.cpp:201 > memcpy(outState, inState, inStateCount * sizeof(inState[0])); > *outStateCount = inStateCount; Talked with Saam offline: we can't find any documentation on whether this memcpy is required even if we return KERN_FAILURE. So, I'll move the above if statement back to its original position after the memcpy to be conservative. Thanks for the reviews. Landed in r261598: <http://trac.webkit.org/r261598>. |