Bug 187827 – DFG AbstractInterpreter: CheckArray filters array modes for DirectArguments only using NonArray

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

In the Abstract Interpreter, when we process ArrayCheck nodes, for DirectArgument array types we filter array modes using the mask of 1 (NonIndexingShape).  DirectArguments has a ArrayStorageShape indexing type.

[Michael Saboff]

<rdar://problem/42146858>

[Michael Saboff]

Actually, DirectArguments structure can have either a NonArray indexingType or a ArrayStorageShape.  We need to allow for both possibilities.

[Michael Saboff]

Created attachment 345466 [details]
Patch

[Saam Barati]

Comment on attachment 345466 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:9
> +        When filtering array modes for DirectArguments and ScopedArguments, we need to allow for the possibility that

not ScopedArguments.

> Source/JavaScriptCore/ChangeLog:10
> +        they can either be NonArray or NonArrayWithArrayStorage (aka ArrayStorageShape).

Might be worth a comment why this is. Also might worth noting why DirectArguments won't end up with Int32/Double/etc shapes.
(I'd put that explanation here or in a comment in the code perhaps, or both)

[Saam Barati]

Actually, it might be worth checking out what ScopedArguments does for out of line arguments (the ones past the length of the arguments object). If like DirectArguments, it stores it in the Butterfly, you actually may need to handle ScopedArguments too

[Michael Saboff]

(In reply to Saam Barati from comment #5)
> Actually, it might be worth checking out what ScopedArguments does for out
> of line arguments (the ones past the length of the arguments object). If
> like DirectArguments, it stores it in the Butterfly, you actually may need
> to handle ScopedArguments too

As we discussed, I added the change for ScopedArguments and a test as well.

[Michael Saboff]

Created attachment 345481 [details]
Updated patch that includes ScopedArguments change

[Saam Barati]

Comment on attachment 345481 [details]
Updated patch that includes ScopedArguments change

r=me

[Michael Saboff]

Committed r234075: <https://trac.webkit.org/changeset/234075>