Bug 212430 – [ macOS iOS ] REGRESSION(r261600?): imported/w3c/web-platform-tests/html/dom/reflection-embedded.html & imported/w3c/web-platform-tests/html/dom/reflection-forms.html are flaky failures

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Jacob Uphoff]

Created attachment 400374 [details]
Diff1

The following tests recently became flaky failures:
imported/w3c/web-platform-tests/html/dom/reflection-forms.html
imported/w3c/web-platform-tests/html/dom/reflection-embedded.html

These tests recently became flaky failures for all of macOS and iOS with the first failure seen on r261600.

History:

https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fhtml%2Fdom%2Freflection-forms.html&test=imported%2Fw3c%2Fweb-platform-tests%2Fhtml%2Fdom%2Freflection-embedded.html&platform=ios&platform=mac

Diffs:

Attached

[Jacob Uphoff]

Created attachment 400375 [details]
Diff2

[Radar WebKit Bug Importer]

<rdar://problem/63688805>

[Chris Dumez]

Seems like a potential regression from r261600. The fact that the results from the tests are flaky and not consistently failing is kinda scary too.

[Chris Dumez]

Diff looks like:
-FAIL legend.autofocus: IDL set to false assert_equals: hasAttribute() expected false but got true
-FAIL legend.autofocus: IDL set to object "[object Object]" assert_equals: IDL get expected (boolean) true but got (object) object "[object Object]"
-FAIL legend.autofocus: IDL set to NaN assert_equals: hasAttribute() expected false but got true
-FAIL legend.autofocus: IDL set to Infinity assert_equals: IDL get expected (boolean) true but got (number) Infinity
-FAIL legend.autofocus: IDL set to -Infinity assert_equals: IDL get expected (boolean) true but got (number) -Infinity
-FAIL legend.autofocus: IDL set to "\0" assert_equals: IDL get expected (boolean) true but got (string) "\0"
-FAIL legend.autofocus: IDL set to object "test-toString" assert_equals: IDL get expected (boolean) true but got (object) object "test-toString"
-FAIL legend.autofocus: IDL set to object "test-valueOf" assert_equals: IDL get expected (boolean) true but got (object) object "test-valueOf"
+FAIL legend.autofocus: IDL set to false |this|.constructor[Symbol.species] is not a constructor
+FAIL legend.autofocus: IDL set to object "[object Object]" |this|.constructor[Symbol.species] is not a constructor
+FAIL legend.autofocus: IDL set to NaN |this|.constructor[Symbol.species] is not a constructor
+FAIL legend.autofocus: IDL set to Infinity |this|.constructor[Symbol.species] is not a constructor
+FAIL legend.autofocus: IDL set to -Infinity |this|.constructor[Symbol.species] is not a constructor
+FAIL legend.autofocus: IDL set to "\0" |this|.constructor[Symbol.species] is not a constructor
+FAIL legend.autofocus: IDL set to object "test-toString" |this|.constructor[Symbol.species] is not a constructor
+FAIL legend.autofocus: IDL set to object "test-valueOf" |this|.constructor[Symbol.species] is not a constructor

I believe this means we used to consistently pass the is-constructor checks. However, after r261600, we appear to flakily fail those checks.

This may need to be investigated by a JSC person.

[Yusuke Suzuki]

I think

 	1736	        case IsConstructor:
 	1737	            // FIXME: We can speculate constructability from child's m_structure.
 	1738	            // https://bugs.webkit.org/show_bug.cgi?id=211796
 	1739	            if (!(child.m_type & (SpecFunction | SpecProxyObject))) {
 	1740	                setConstant(node, jsBoolean(false));
 	1741	                constantWasSet = true;
 	1742	                break;
 	1743	            }
 	1744	
 	1745	            break;

this looks wrong in https://trac.webkit.org/changeset/261600/webkit. We do not have such an invariant.
This also explains why this failure is flaky: this is DFG / FTL code.

[Yusuke Suzuki]

Created attachment 400390 [details]
Patch

[Saam Barati]

Comment on attachment 400390 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1737
>          case IsConstructor:

this looks wrong? Why is it not setting its result type as SpecBoolean?

[Saam Barati]

Comment on attachment 400390 [details]
Patch

r=me

[Yusuke Suzuki]

Comment on attachment 400390 [details]
Patch

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

>> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:1737
>>          case IsConstructor:
> 
> this looks wrong? Why is it not setting its result type as SpecBoolean?

Discussed in slack. After breaking from this switch, we have generic path which is setting SpecBoolean for the result.

[EWS]

Committed r262231: <https://trac.webkit.org/changeset/262231>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400390 [details].