Bug 137289 - AX: iOS8: Crash at -[WebAccessibilityObjectWrapper accessibilityElementAtIndex:]
Summary: AX: iOS8: Crash at -[WebAccessibilityObjectWrapper accessibilityElementAtIndex:]
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-09-30 23:07 PDT by chris fleizach
Modified: 2014-10-06 18:10 PDT (History)
10 users (show)

See Also:


Attachments
patch (5.03 KB, patch)
2014-09-30 23:11 PDT, chris fleizach
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2014-09-30 23:07:32 PDT
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>
Comment 1 chris fleizach 2014-09-30 23:11:10 PDT
Created attachment 239006 [details]
patch
Comment 2 WebKit Commit Bot 2014-10-05 16:10:05 PDT
Comment on attachment 239006 [details]
patch

Clearing flags on attachment: 239006

Committed r174330: <http://trac.webkit.org/changeset/174330>
Comment 3 WebKit Commit Bot 2014-10-05 16:10:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Daniel Bates 2014-10-06 16:44:13 PDT
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.
Comment 5 chris fleizach 2014-10-06 16:45:50 PDT
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
Comment 6 Daniel Bates 2014-10-06 17:43:54 PDT
(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).
Comment 7 chris fleizach 2014-10-06 17:48:56 PDT
(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
Comment 8 Daniel Bates 2014-10-06 17:52:02 PDT
(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?
Comment 9 chris fleizach 2014-10-06 17:53:42 PDT
(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
Comment 10 chris fleizach 2014-10-06 18:10:29 PDT
http://trac.webkit.org/changeset/174376