Bug 201857 - [JSC] CheckArray+NonArray is not filtering out Array in AI
Summary: [JSC] CheckArray+NonArray is not filtering out Array in AI
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-09-16 19:20 PDT by Yusuke Suzuki
Modified: 2019-09-17 12:52 PDT (History)
7 users (show)

See Also:


Attachments
Patch (6.92 KB, patch)
2019-09-16 19:23 PDT, Yusuke Suzuki
keith_miller: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-09-16 19:20:35 PDT
[JSC] CheckArray+NonArray is not filtering out Array in AI
Comment 1 Yusuke Suzuki 2019-09-16 19:23:12 PDT
Created attachment 378928 [details]
Patch
Comment 2 Yusuke Suzuki 2019-09-16 19:23:14 PDT
<rdar://problem/54194820>
Comment 3 Keith Miller 2019-09-17 11:22:40 PDT
Comment on attachment 378928 [details]
Patch

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

r=me with some comments.

> Source/JavaScriptCore/ChangeLog:10
> +        While we are assuming that CheckArray+NonArray can ensure that it only passes non-array inputs, DFG::ArrayMode::alreadyChecked

Nit: While we assume CheckArray+NonArray ensures it only...

> Source/JavaScriptCore/dfg/DFGArrayMode.cpp:441
> +    // But here, we already filtered TypedArrays. So, just handling it like NonArray.

Nit: So, just handle it like a NonArray.

> Source/JavaScriptCore/dfg/DFGArrayMode.cpp:478
> +        for (unsigned i = value.m_structure.size(); i--;) {
> +            RegisteredStructure structure = value.m_structure[i];
> +            if (structure.get() != originalStructure)

Isn't this a set so it should have size 1? How is it possible to have more than one copy of the same structure?
Comment 4 Yusuke Suzuki 2019-09-17 12:33:32 PDT
Comment on attachment 378928 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/ChangeLog:10
>> +        While we are assuming that CheckArray+NonArray can ensure that it only passes non-array inputs, DFG::ArrayMode::alreadyChecked
> 
> Nit: While we assume CheckArray+NonArray ensures it only...

Fixed.

>> Source/JavaScriptCore/dfg/DFGArrayMode.cpp:441
>> +    // But here, we already filtered TypedArrays. So, just handling it like NonArray.
> 
> Nit: So, just handle it like a NonArray.

Fixed.

>> Source/JavaScriptCore/dfg/DFGArrayMode.cpp:478
>> +            if (structure.get() != originalStructure)
> 
> Isn't this a set so it should have size 1? How is it possible to have more than one copy of the same structure?

Yeah, we can just check the size, getting onlySttructure, and comparing it with this originalStructure. Fixed.
Comment 5 Yusuke Suzuki 2019-09-17 12:52:52 PDT
Committed r249976: <https://trac.webkit.org/changeset/249976>