Bug 160832

Summary: Make JSValue::strictEqual() handle failures to resolve JSRopeStrings.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, fpizlo, ggaren, keith_miller, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch.
mark.lam: review-
proposed patch. ggaren: review+

Mark Lam
Reported 2016-08-12 16:57:34 PDT
Currently, JSValue::strictEqualSlowCaseInline() (and peers) will blindly try to access the StringImpl of a JSRopeString that fails to resolve its rope. As a result, we'll crash with null pointer dereferences. We should fix this.
Attachments
proposed patch. (17.52 KB, patch)
2016-08-12 17:20 PDT, Mark Lam 		mark.lam: review-
DetailsFormatted DiffDiff
proposed patch. (17.54 KB, patch)
2016-08-15 10:33 PDT, Mark Lam 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2016-08-12 17:01:20 PDT
<rdar://problem/27577556>
Mark Lam
Comment 2 2016-08-12 17:20:47 PDT
Created attachment 285984 [details] proposed patch. Let's get some EWS testing and feedback. I don't have a test because the only test case I have so far relies on allocating just the right amount of memory to run out of memory right at the moment of resolving a rope for a strict equality check. The test is brittle and flaky. So far, it only manifests the issue on ARM64, but not on x86_64 yet. So, I think its of questionable value and will leave it out for now.
Mark Lam
Comment 3 2016-08-12 19:31:32 PDT
Comment on attachment 285984 [details] proposed patch. Will fix the build failure.
Mark Lam
Comment 4 2016-08-15 10:33:36 PDT
Created attachment 286065 [details] proposed patch.
Geoffrey Garen
Comment 5 2016-08-15 14:30:20 PDT
Comment on attachment 286065 [details] proposed patch. Let's call this "equal" since the WTF function is "equal". r=me
Mark Lam
Comment 6 2016-08-15 14:55:01 PDT
Thanks for the review. I've replaced "equals" with "equal" (and ditto for the matching slow case function). Landed in r204485: <http://trac.webkit.org/r204485>.
Note You need to log in before you can comment on or make changes to this bug.