Bug 180274 - Having a bad time needs to handle ArrayClass indexing type as well
Summary: Having a bad time needs to handle ArrayClass indexing type as well
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-01 13:21 PST by Saam Barati
Modified: 2017-12-01 15:40 PST (History)
13 users (show)

See Also:


Attachments
patch (5.67 KB, patch)
2017-12-01 13:29 PST, Saam Barati
mark.lam: review+
Details | Formatted Diff | Diff
patch for landing (5.69 KB, patch)
2017-12-01 13:59 PST, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2017-12-01 13:21:06 PST
...
Comment 1 Saam Barati 2017-12-01 13:27:58 PST
<rdar://problem/35667869>
Comment 2 Saam Barati 2017-12-01 13:29:29 PST
Created attachment 328147 [details]
patch
Comment 3 Keith Miller 2017-12-01 13:32:07 PST
Comment on attachment 328147 [details]
patch

r=me.
Comment 4 Mark Lam 2017-12-01 13:33:08 PST
Comment on attachment 328147 [details]
patch

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

> Source/JavaScriptCore/runtime/JSObject.cpp:1611
> +        switchToSlowPutArrayStorage(vm);

Isn't this infinitely recursing?
Comment 5 Mark Lam 2017-12-01 13:34:03 PST
Comment on attachment 328147 [details]
patch

oops, didn't mean to set the r? flag
Comment 6 Mark Lam 2017-12-01 13:40:12 PST
Comment on attachment 328147 [details]
patch

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

r=me too.

>> Source/JavaScriptCore/runtime/JSObject.cpp:1611
>> +        switchToSlowPutArrayStorage(vm);
> 
> Isn't this infinitely recursing?

OK, I see what's happening.  The ensureArrayStorage() ensures that the indexingType is no longer ArrayClass before recursing.  Nothing to see here.  Moving along.
Comment 7 Mark Lam 2017-12-01 13:42:04 PST
Comment on attachment 328147 [details]
patch

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

> Source/JavaScriptCore/ChangeLog:13
> +        in non empty indexing types as broken, instead of having to opt out all

I suggest replacing "non empty" with "non-empty" to be consistent with your usage below.  Plus it's easier to read.
Comment 8 Saam Barati 2017-12-01 13:59:32 PST
Created attachment 328153 [details]
patch for landing
Comment 9 WebKit Commit Bot 2017-12-01 15:40:16 PST
Comment on attachment 328153 [details]
patch for landing

Clearing flags on attachment: 328153

Committed r225423: <https://trac.webkit.org/changeset/225423>
Comment 10 WebKit Commit Bot 2017-12-01 15:40:18 PST
All reviewed patches have been landed.  Closing bug.