Bug 201857 – [JSC] CheckArray+NonArray is not filtering out Array in AI

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] CheckArray+NonArray is not filtering out Array in AI

[Yusuke Suzuki]

Created attachment 378928 [details]
Patch

[Yusuke Suzuki]

<rdar://problem/54194820>

[Keith Miller]

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?

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Committed r249976: <https://trac.webkit.org/changeset/249976>