Bug 160832 - Make JSValue::strictEqual() handle failures to resolve JSRopeStrings.
Summary: Make JSValue::strictEqual() handle failures to resolve JSRopeStrings.
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
Keywords: InRadar
Depends on:
Reported: 2016-08-12 16:57 PDT by Mark Lam
Modified: 2016-08-15 14:55 PDT (History)
7 users (show)

See Also:

proposed patch. (17.52 KB, patch)
2016-08-12 17:20 PDT, Mark Lam
mark.lam: review-
Details | Formatted Diff | Diff
proposed patch. (17.54 KB, patch)
2016-08-15 10:33 PDT, Mark Lam
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 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.
Comment 1 Mark Lam 2016-08-12 17:01:20 PDT
Comment 2 Mark Lam 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.
Comment 3 Mark Lam 2016-08-12 19:31:32 PDT
Comment on attachment 285984 [details]
proposed patch.

Will fix the build failure.
Comment 4 Mark Lam 2016-08-15 10:33:36 PDT
Created attachment 286065 [details]
proposed patch.
Comment 5 Geoffrey Garen 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".

Comment 6 Mark Lam 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>.