Bug 211600 – Crash under WebKit::XPCServiceMain

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Crash under WebKit::XPCServiceMain:
Thread 0 Crashed ↩:: Dispatch queue: com.apple.main-thread
0   libsystem_platform.dylib      	0x00007fff70c528b5 _platform_strcmp + 181
1   com.apple.WebKit              	0x7fff47d28ca7 WebKit::XPCServiceMain(int, char const**) + 114 (/AppleInternal/BuildRoot/Library/Caches/com.apple.xbs/Sources/WebKit2/WebKit2-7609.1.20.111.8/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:125)
2   libdyld.dylib                 	0x00007fff70a5ccc9 start + 1

[Chris Dumez]

<rdar://problem/62875458>

[Chris Dumez]

Created attachment 398806 [details]
Patch

[Darin Adler]

Comment on attachment 398806 [details]
Patch

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

> Source/WebKit/ChangeLog:12
> +        - Crash under strcmp() could in theory happen if expectedBundleVersion.UTF8String was null, which could
> +          happen if expectedBundleVersion was null. I now use higher level String types for the versions, make
> +          sure they are not null and use String comparison to compare them.

I understand how checks for null are a good idea. Not sure why using WTF::String is helpful.

[Chris Dumez]

(In reply to Darin Adler from comment #3)
> Comment on attachment 398806 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=398806&action=review
> 
> > Source/WebKit/ChangeLog:12
> > +        - Crash under strcmp() could in theory happen if expectedBundleVersion.UTF8String was null, which could
> > +          happen if expectedBundleVersion was null. I now use higher level String types for the versions, make
> > +          sure they are not null and use String comparison to compare them.
> 
> I understand how checks for null are a good idea. Not sure why using
> WTF::String is helpful.

This was my own personal preference, I much prefer dealing C++ types, than C or ObjC types. We are getting one of the version as a const char* and there other as a NSString*, both can silently be converted to String type, so this was convenient too.

[Chris Dumez]

Created attachment 398808 [details]
Patch

[Chris Dumez]

(In reply to Chris Dumez from comment #5)
> Created attachment 398808 [details]
> Patch

Fixes iOS build.

[EWS]

Committed r261360: <https://trac.webkit.org/changeset/261360>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398808 [details].

[Alexey Proskuryakov]

I hope that this fixes all crashes _under_ WebKit::XPCServiceMain ;)

[Darin Adler]

Comment on attachment 398808 [details]
Patch

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

> Source/WebKit/Shared/EntryPointUtilities/Cocoa/XPCService/XPCServiceMain.mm:142
> +        String webKitBundleVersion = xpc_dictionary_get_string(bootstrap.get(), "WebKitBundleVersion");

Just noticed a change in behavior here. The old code treated this as UTF-8. The new code treats it as Latin-1. But I suspect really this is all guaranteed to be ASCII so this change in behavior is not important.