Bug 212430

Created attachment 400375 [details]
Diff2

<rdar://problem/63688805>

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.

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.

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.

Created attachment 400390 [details]
Patch

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?

Comment on attachment 400390 [details]
Patch

r=me

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.

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

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