182249 2018-01-29 10:17:51 -0800 REGRESSION (r227341): DFG_ASSERT failure at JSC::DFG::AtTailAbstractState::forNode() 2018-04-10 10:20:12 -0700 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=184452 https://bugs.webkit.org/show_bug.cgi?id=184455 InRadar P2 Normal --- 1 msaboff msaboff commit-queue ews-watchlist fpizlo keith_miller mark.lam saam webkit-bug-importer oldest_to_newest 1394135 0 msaboff 2018-01-29 10:17:51 -0800 Change set r227341 (<https://trac.webkit.org/changeset/227341>) changed the abstract interpreter for the non-strict Compare nodes like CompareEq. That change caused a DFG_ASSERT() to fire: DFG ASSERTION FAILED: iter != valuesAtTail.end() ./dfg/DFGAtTailAbstractState.cpp(59) : JSC::DFG::AbstractValue &JSC::DFG::AtTailAbstractState::forNode(JSC::DFG::NodeFlowProjection) There needs to be a corresponding change in clobberize() for UntypedUse as well. 1394136 1 msaboff 2018-01-29 10:18:15 -0800 <rdar://problem/36833641> 1394143 2 332553 msaboff 2018-01-29 10:29:07 -0800 Created attachment 332553 Patch 1394145 3 332553 keith_miller 2018-01-29 10:32:44 -0800 Comment on attachment 332553 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332553&action=review > Source/JavaScriptCore/dfg/DFGClobberize.h:1550 > + if (node->child1().useKind() == UntypedUse || node->child1().useKind() == ObjectUse > + || node->child2().useKind() == UntypedUse || node->child2().useKind() == ObjectUse) { You should have a check for node->isBinaryUseKind(ObjectUse) since that won't convert. Nit: Also, I would make a local for the useKinds... 1394160 4 332556 msaboff 2018-01-29 10:46:54 -0800 Created attachment 332556 Patch with updates from review 1394162 5 332556 keith_miller 2018-01-29 10:50:20 -0800 Comment on attachment 332556 Patch with updates from review r=me. 1394173 6 332556 commit-queue 2018-01-29 11:13:49 -0800 Comment on attachment 332556 Patch with updates from review Clearing flags on attachment: 332556 Committed r227742: <https://trac.webkit.org/changeset/227742> 1394174 7 commit-queue 2018-01-29 11:13:51 -0800 All reviewed patches have been landed. Closing bug. 1413299 8 332556 fpizlo 2018-04-10 10:05:51 -0700 Comment on attachment 332556 Patch with updates from review View in context: https://bugs.webkit.org/attachment.cgi?id=332556&action=review > Source/JavaScriptCore/dfg/DFGClobberize.h:1560 > - if (!node->isBinaryUseKind(UntypedUse)) { > + > + if (node->op() == CompareEq && node->isBinaryUseKind(ObjectUse)) { > def(PureValue(node)); > return; > } > - read(World); > - write(Heap); > + if (node->child1().useKind() == UntypedUse || node->child1().useKind() == ObjectUse > + || node->child2().useKind() == UntypedUse || node->child2().useKind() == ObjectUse) { > + read(World); > + write(Heap); > + return; > + } > + > + def(PureValue(node)); This patch creates a major discrepancy between AI's handling of CompareEq and clobberize's handling of CompareEq. Also, from what I can tell, it's only CompareEq(Untyped:, Untyped:) that can have effects. CompareEq(ObjectUse:, _) cannot have effects. 1413303 9 fpizlo 2018-04-10 10:15:23 -0700 I don't think it's correct to fix assertions in AtTailAbstractState by pretending that something is effectful. It seems that all that this patch did was mask a more fundamental bug by turning off hoisting of CompareEq in this case. But CompareEq is effectless here, so if hoisting it causes a problem then pretending that it's effectful is not the right solution. 1413304 10 fpizlo 2018-04-10 10:18:00 -0700 I see the problem now: in r227341, you incorrectly made ComapreEq(Untyped:, Other:) pretend to have effect even though it doesn't. But this means that attempting to hoist it causes clobberWorld() to get called with AtTailAbstractState, which causes crashes. The correct solution is to fix AI so that it's precise about the effectfulness of CompareEq(Untyped:, Other:). 332553 2018-01-29 10:29:07 -0800 2018-01-29 10:46:54 -0800 Patch 182249.patch text/plain 2868 msaboff SW5kZXg6IEpTVGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvQ2hhbmdlTG9n CShyZXZpc2lvbiAyMjc3MzkpCisrKyBKU1Rlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpA QCAtMSwzICsxLDE0IEBACisyMDE4LTAxLTI5ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBw bGUuY29tPgorCisgICAgICAgIFJFR1JFU1NJT04gKHIyMjczNDEpOiBERkdfQVNTRVJUIGZhaWx1 cmUgYXQgSlNDOjpERkc6OkF0VGFpbEFic3RyYWN0U3RhdGU6OmZvck5vZGUoKQorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTgyMjQ5CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTmV3IHJlZ3Jlc3Npb24gdGVz dC4KKworICAgICAgICAqIHN0cmVzcy9jb21wYXJlLWNsb2JiZXItdW50eXBlZHVzZS5qczogQWRk ZWQuCisKIDIwMTgtMDEtMjkgIE1hdHQgTGV3aXMgIDxqbGV3aXMzQGFwcGxlLmNvbT4KIAogICAg ICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMjI3NzI1LgpJbmRleDogSlNUZXN0cy9zdHJl c3MvY29tcGFyZS1jbG9iYmVyLXVudHlwZWR1c2UuanMKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gSlNUZXN0cy9z dHJlc3MvY29tcGFyZS1jbG9iYmVyLXVudHlwZWR1c2UuanMJKG5vbmV4aXN0ZW50KQorKysgSlNU ZXN0cy9zdHJlc3MvY29tcGFyZS1jbG9iYmVyLXVudHlwZWR1c2UuanMJKHdvcmtpbmcgY29weSkK QEAgLTAsMCArMSwxMiBAQAorLy8gVGVzdCB0aGF0IHdlIHByb3Blcmx5IGNsb2JiZXIgdW50eXBl ZCB1c2VzLiAgVGhpcyB0ZXN0IHNob3VsZCB0aHJvdyBvciBjcmFzaC4KKworbGV0IHZhbDsKKwor Zm9yICh2YXIgaSA9IDA7IGkgPCAxMDAwMDA7IGkrKykKKyAgICB2YWwgPSA0MjsKKworZm9yIChs ZXQgaSA9IDA7IGkgPCAxZTY7IGkrKykgeworICAgIGlmICh2YWwgIT0gbnVsbCAmJiB2YWwgPT0g MikgeworICAgICAgICB0aHJvdyAiVmFsIHNob3VsZCBiZSA0MiwgYnV0IGlzIDIiOworICAgIH0K K30KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g U291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjI3NzM5KQorKysgU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3 IEBACisyMDE4LTAxLTI5ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisg ICAgICAgIFJFR1JFU1NJT04gKHIyMjczNDEpOiBERkdfQVNTRVJUIGZhaWx1cmUgYXQgSlNDOjpE Rkc6OkF0VGFpbEFic3RyYWN0U3RhdGU6OmZvck5vZGUoKQorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTgyMjQ5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ2hhbmdlZCBjbG9iYmVyaXplKCkgaGFuZGxpbmcg b2YgQ29tcGFyZUVxLCBldCBhbCB0byBwcm9wZXJseSBoYW5kbGUgY29tcGFyaXNvbnMgYmV0d2Vl bgorICAgICAgICBVbnR5cGVkIGFuZCBPYmplY3QgdmFsdWVzIHdoZW4gY29tcGFyZWQgYWdhaW5z dCBidWlsdCBpbiB0eXBlcy4gIFN1Y2ggY29tcGFyaXNvbnMgY2FuCisgICAgICAgIGludm9rZSB0 b051bWJlcigpIG9yIG90aGVyIG1ldGhvZHMuCisKKyAgICAgICAgKiBkZmcvREZHQ2xvYmJlcml6 ZS5oOgorICAgICAgICAoSlNDOjpERkc6OmNsb2JiZXJpemUpOgorCiAyMDE4LTAxLTI5ICBNYXR0 IExld2lzICA8amxld2lzM0BhcHBsZS5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGlu ZyBvdXQgcjIyNzcyNS4KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHQ2xvYmJl cml6ZS5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHQ2xvYmJl cml6ZS5oCShyZXZpc2lvbiAyMjc3MzkpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RG R0Nsb2JiZXJpemUuaAkod29ya2luZyBjb3B5KQpAQCAtMTU0NSwxMiArMTU0NSwxNSBAQCB2b2lk IGNsb2JiZXJpemUoR3JhcGgmIGdyYXBoLCBOb2RlKiBub2RlCiAgICAgICAgICAgICB3cml0ZShI ZWFwT2JqZWN0Q291bnQpOwogICAgICAgICAgICAgcmV0dXJuOwogICAgICAgICB9Ci0gICAgICAg IGlmICghbm9kZS0+aXNCaW5hcnlVc2VLaW5kKFVudHlwZWRVc2UpKSB7Ci0gICAgICAgICAgICBk ZWYoUHVyZVZhbHVlKG5vZGUpKTsKKworICAgICAgICBpZiAobm9kZS0+Y2hpbGQxKCkudXNlS2lu ZCgpID09IFVudHlwZWRVc2UgfHwgbm9kZS0+Y2hpbGQxKCkudXNlS2luZCgpID09IE9iamVjdFVz ZQorICAgICAgICAgICAgfHwgbm9kZS0+Y2hpbGQyKCkudXNlS2luZCgpID09IFVudHlwZWRVc2Ug fHwgbm9kZS0+Y2hpbGQyKCkudXNlS2luZCgpID09IE9iamVjdFVzZSkgeworICAgICAgICAgICAg cmVhZChXb3JsZCk7CisgICAgICAgICAgICB3cml0ZShIZWFwKTsKICAgICAgICAgICAgIHJldHVy bjsKICAgICAgICAgfQotICAgICAgICByZWFkKFdvcmxkKTsKLSAgICAgICAgd3JpdGUoSGVhcCk7 CisKKyAgICAgICAgZGVmKFB1cmVWYWx1ZShub2RlKSk7CiAgICAgICAgIHJldHVybjsKIAogICAg IGNhc2UgVG9OdW1iZXI6IHsK 332556 2018-01-29 10:46:54 -0800 2018-01-29 11:13:49 -0800 Patch with updates from review 182249-2.patch text/plain 2976 msaboff SW5kZXg6IEpTVGVzdHMvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIEpTVGVzdHMvQ2hhbmdlTG9n CShyZXZpc2lvbiAyMjc3MzkpCisrKyBKU1Rlc3RzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpA QCAtMSwzICsxLDE0IEBACisyMDE4LTAxLTI5ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBw bGUuY29tPgorCisgICAgICAgIFJFR1JFU1NJT04gKHIyMjczNDEpOiBERkdfQVNTRVJUIGZhaWx1 cmUgYXQgSlNDOjpERkc6OkF0VGFpbEFic3RyYWN0U3RhdGU6OmZvck5vZGUoKQorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTgyMjQ5CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTmV3IHJlZ3Jlc3Npb24gdGVz dC4KKworICAgICAgICAqIHN0cmVzcy9jb21wYXJlLWNsb2JiZXItdW50eXBlZHVzZS5qczogQWRk ZWQuCisKIDIwMTgtMDEtMjkgIE1hdHQgTGV3aXMgIDxqbGV3aXMzQGFwcGxlLmNvbT4KIAogICAg ICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMjI3NzI1LgpJbmRleDogSlNUZXN0cy9zdHJl c3MvY29tcGFyZS1jbG9iYmVyLXVudHlwZWR1c2UuanMKPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gSlNUZXN0cy9z dHJlc3MvY29tcGFyZS1jbG9iYmVyLXVudHlwZWR1c2UuanMJKG5vbmV4aXN0ZW50KQorKysgSlNU ZXN0cy9zdHJlc3MvY29tcGFyZS1jbG9iYmVyLXVudHlwZWR1c2UuanMJKHdvcmtpbmcgY29weSkK QEAgLTAsMCArMSwxMiBAQAorLy8gVGVzdCB0aGF0IHdlIHByb3Blcmx5IGNsb2JiZXIgdW50eXBl ZCB1c2VzLiAgVGhpcyB0ZXN0IHNob3VsZCB0aHJvdyBvciBjcmFzaC4KKworbGV0IHZhbDsKKwor Zm9yICh2YXIgaSA9IDA7IGkgPCAxMDAwMDA7IGkrKykKKyAgICB2YWwgPSA0MjsKKworZm9yIChs ZXQgaSA9IDA7IGkgPCAxZTY7IGkrKykgeworICAgIGlmICh2YWwgIT0gbnVsbCAmJiB2YWwgPT0g MikgeworICAgICAgICB0aHJvdyAiVmFsIHNob3VsZCBiZSA0MiwgYnV0IGlzIDIiOworICAgIH0K K30KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g U291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMjI3NzM5KQorKysgU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3 IEBACisyMDE4LTAxLTI5ICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisg ICAgICAgIFJFR1JFU1NJT04gKHIyMjczNDEpOiBERkdfQVNTRVJUIGZhaWx1cmUgYXQgSlNDOjpE Rkc6OkF0VGFpbEFic3RyYWN0U3RhdGU6OmZvck5vZGUoKQorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTgyMjQ5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkg Tk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ2hhbmdlZCBjbG9iYmVyaXplKCkgaGFuZGxpbmcg b2YgQ29tcGFyZUVxLCBldCBhbCB0byBwcm9wZXJseSBoYW5kbGUgY29tcGFyaXNvbnMgYmV0d2Vl bgorICAgICAgICBVbnR5cGVkIGFuZCBPYmplY3QgdmFsdWVzIHdoZW4gY29tcGFyZWQgYWdhaW5z dCBidWlsdCBpbiB0eXBlcy4gIFN1Y2ggY29tcGFyaXNvbnMgY2FuCisgICAgICAgIGludm9rZSB0 b051bWJlcigpIG9yIG90aGVyIG1ldGhvZHMuCisKKyAgICAgICAgKiBkZmcvREZHQ2xvYmJlcml6 ZS5oOgorICAgICAgICAoSlNDOjpERkc6OmNsb2JiZXJpemUpOgorCiAyMDE4LTAxLTI5ICBNYXR0 IExld2lzICA8amxld2lzM0BhcHBsZS5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGlu ZyBvdXQgcjIyNzcyNS4KSW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHQ2xvYmJl cml6ZS5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9kZmcvREZHQ2xvYmJl cml6ZS5oCShyZXZpc2lvbiAyMjc3MzkpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RG R0Nsb2JiZXJpemUuaAkod29ya2luZyBjb3B5KQpAQCAtMTU0NSwxMiArMTU0NSwxOSBAQCB2b2lk IGNsb2JiZXJpemUoR3JhcGgmIGdyYXBoLCBOb2RlKiBub2RlCiAgICAgICAgICAgICB3cml0ZShI ZWFwT2JqZWN0Q291bnQpOwogICAgICAgICAgICAgcmV0dXJuOwogICAgICAgICB9Ci0gICAgICAg IGlmICghbm9kZS0+aXNCaW5hcnlVc2VLaW5kKFVudHlwZWRVc2UpKSB7CisKKyAgICAgICAgaWYg KG5vZGUtPm9wKCkgPT0gQ29tcGFyZUVxICYmIG5vZGUtPmlzQmluYXJ5VXNlS2luZChPYmplY3RV c2UpKSB7CiAgICAgICAgICAgICBkZWYoUHVyZVZhbHVlKG5vZGUpKTsKICAgICAgICAgICAgIHJl dHVybjsKICAgICAgICAgfQotICAgICAgICByZWFkKFdvcmxkKTsKLSAgICAgICAgd3JpdGUoSGVh cCk7CisgICAgICAgIGlmIChub2RlLT5jaGlsZDEoKS51c2VLaW5kKCkgPT0gVW50eXBlZFVzZSB8 fCBub2RlLT5jaGlsZDEoKS51c2VLaW5kKCkgPT0gT2JqZWN0VXNlCisgICAgICAgICAgICB8fCBu b2RlLT5jaGlsZDIoKS51c2VLaW5kKCkgPT0gVW50eXBlZFVzZSB8fCBub2RlLT5jaGlsZDIoKS51 c2VLaW5kKCkgPT0gT2JqZWN0VXNlKSB7CisgICAgICAgICAgICByZWFkKFdvcmxkKTsKKyAgICAg ICAgICAgIHdyaXRlKEhlYXApOworICAgICAgICAgICAgcmV0dXJuOworICAgICAgICB9CisKKyAg ICAgICAgZGVmKFB1cmVWYWx1ZShub2RlKSk7CiAgICAgICAgIHJldHVybjsKIAogICAgIGNhc2Ug VG9OdW1iZXI6IHsK