AX: iOS8: Crash at -[WebAccessibilityObjectWrapper accessibilityElementAtIndex:]

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Subtype: KERN_INVALID_ADDRESS at 0x00000000bbadbeef
Triggered by Thread:  0

Thread 0 name:  Dispatch queue: com.apple.main-thread
Thread 0 Crashed:
0   JavaScriptCore                	0x00000001855849f4 WTFCrash + 72
1   JavaScriptCore                	0x00000001855849e8 WTFCrash + 60
2   WebCore                       	0x0000000191ab4ba8 WTF::CrashOnOverflow::overflowed() + 8 (CheckedArithmetic.h:77)
3   WebCore                       	0x00000001925a9008 -[WebAccessibilityObjectWrapper accessibilityElementAtIndex:] + 244 (Vector.h:624)
4   WebProcess                    	0x00000001821d58a8 -[WKAccessibilityWebPageObjectAccessibility _accessibilityElementsWithCount:originalElement:next:] + 376


<rdar://problem/18436805>

[chris fleizach]

Created attachment 239006 [details]
patch

[WebKit Commit Bot]

Comment on attachment 239006 [details]
patch

Clearing flags on attachment: 239006

Committed r174330: <http://trac.webkit.org/changeset/174330>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Daniel Bates]

Comment on attachment 239006 [details]
patch

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

> Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:194
> +    for (NSInteger k = location; k < (location+length); ++k)

This broke the iOS build. The comparison here led to the clang warning, "comparison of integers of different signs", because the result of the sub expression location + length is a unsigned data type, but k is a signed data type. And we treat warnings as errors.

[chris fleizach]

Comment on attachment 239006 [details]
patch

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

>> Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:194
>> +    for (NSInteger k = location; k < (location+length); ++k)
> 
> This broke the iOS build. The comparison here led to the clang warning, "comparison of integers of different signs", because the result of the sub expression location + length is a unsigned data type, but k is a signed data type. And we treat warnings as errors.

strange because I tested on iOS simulator to create this unit test. i'll take another look

[Daniel Bates]

(In reply to comment #5)
> (From update of attachment 239006 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=239006&action=review
> 
> >> Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:194
> >> +    for (NSInteger k = location; k < (location+length); ++k)
> > 
> > This broke the iOS build. The comparison here led to the clang warning, "comparison of integers of different signs", because the result of the sub expression location + length is a unsigned data type, but k is a signed data type. And we treat warnings as errors.
> 
> strange because I tested on iOS simulator to create this unit test. i'll take another look

Have you had a chance to take another look? I'm unclear how we are using unsigned, size_t, and NSInteger data types in this code. It seems like we to expect to convert from unsigned to signed to unsigned data types? (why?) Unless you advise otherwise, I plan to roll out <http://trac.webkit.org/changeset/174330> in 10 minutes from now (5:53 PM PST).

[chris fleizach]

(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 239006 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=239006&action=review
> > 
> > >> Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:194
> > >> +    for (NSInteger k = location; k < (location+length); ++k)
> > > 
> > > This broke the iOS build. The comparison here led to the clang warning, "comparison of integers of different signs", because the result of the sub expression location + length is a unsigned data type, but k is a signed data type. And we treat warnings as errors.
> > 
> > strange because I tested on iOS simulator to create this unit test. i'll take another look
> 
> Have you had a chance to take another look? 

Yes, it takes forever to build after an update. I'm still building

> I'm unclear how we are using unsigned, size_t, and NSInteger data types in this code. It seems like we to expect to convert from unsigned to signed to unsigned data types? (why?)

I'm cleaning it up

> Unless you advise otherwise, I plan to roll out <http://trac.webkit.org/changeset/174330> in 10 minutes from now (5:53 PM PST).

I'd urge you to wait for me to resolve this

[Daniel Bates]

(In reply to comment #7)
> [...]
> I'd urge you to wait for me to resolve this

Will wait. When do you envision landing a fix for this issue? In the next 30 minutes? hour?

[chris fleizach]

(In reply to comment #8)
> (In reply to comment #7)
> > [...]
> > I'd urge you to wait for me to resolve this
> 
> Will wait. When do you envision landing a fix for this issue? In the next 30 minutes? hour?

20 mins

[chris fleizach]

http://trac.webkit.org/changeset/174376