Bug 211799 – catch_mach_exception_raise_state() should fail early if the faulting address is not of interest.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Mark Lam]

<rdar://problem/62939204>

[Mark Lam]

Created attachment 399166 [details]
proposed patch.

Let's try this on the EWS.

[Michael Saboff]

Comment on attachment 399166 [details]
proposed patch.

r=me

[Saam Barati]

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

[Saam Barati]

(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

[Mark Lam]

Created attachment 399182 [details]
proposed patch.

[Saam Barati]

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

[Mark Lam]

Created attachment 399189 [details]
proposed patch.

[Mark Lam]

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.

[Mark Lam]

Thanks for the reviews.  Landed in r261598: <http://trac.webkit.org/r261598>.