Bug 98433 - Crash in Safari at com.apple.JavaScriptCore: WTF::StringImpl::is8Bit const + 12
Summary: Crash in Safari at com.apple.JavaScriptCore: WTF::StringImpl::is8Bit const + 12
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks: 98300
  Show dependency tree / graph
 
Reported: 2012-10-04 11:34 PDT by Michael Saboff
Modified: 2012-10-10 10:16 PDT (History)
0 users

See Also:

Attachments
Patch (1.63 KB, patch)
2012-10-04 11:48 PDT, Michael Saboff 		jberlin: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2012-10-04 11:34:33 PDT 
Crash running safari extension tests.

Looks like is8Bit() is called on a String without a StringImpl.

<rdar://problem/12433701>
Comment 1 Michael Saboff 2012-10-04 11:48:15 PDT 
Created attachment 167152 [details]
Patch
Comment 2 Jessie Berlin 2012-10-04 11:51:15 PDT 
Comment on attachment 167152 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:11
> +        (JSValueMakeFromJSONString):

You should also mention that characters16() does the is8Bit() check, which is why you changed it to characters().
Comment 3 Michael Saboff 2012-10-04 13:27:35 PDT 
Committed r130424: <http://trac.webkit.org/changeset/130424>
Comment 4 Michael Saboff 2012-10-04 17:42:59 PDT 
This is actually fixing the symptom of a problem in <http://trac.webkit.org/changeset/130344> and the follow-on <http://trac.webkit.org/changeset/130413>.  It appears that we are turning a null string into an empty string in OpaqueJSString.cpp.  See https://bugs.webkit.org/show_bug.cgi?id=98417.
Comment 5 Michael Saboff 2012-10-10 10:16:44 PDT 
Reopened because https://bugs.webkit.org/show_bug.cgi?id=98417 "After r130344, OpaqueJSString() creates an empty string which should be a null string" was the source of this particular crash.  However the change in this patch (r130424) is actually needed.

Consider the case:

    JSStringRef nullJSON = JSStringCreateWithUTF8CString(0);
    JSValueRef nullJSONObject = JSValueMakeFromJSONString(context, nullJSON);

nullJSON will have a null string which would cause the code before this patch to crash.

The case above has been added to testapi.c in r130832: <http://trac.webkit.org/changeset/130832>.