187669 2018-07-13 16:04:23 -0700 WTF's internal std::optional implementation should release assert on all bad accesses 2018-08-17 10:36:12 -0700 1 1 1 Unclassified WebKit Web Template Framework WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=186536 https://bugs.webkit.org/show_bug.cgi?id=185159 https://bugs.webkit.org/show_bug.cgi?id=187672 https://bugs.webkit.org/show_bug.cgi?id=187744 InRadar P2 Normal --- 1 ddkilzer ddkilzer benjamin cdumez cmarcelo commit-queue dbates ddkilzer ews-watchlist fred.wang mcatanzaro rniwa webkit-bug-importer oldest_to_newest 1442086 0 ddkilzer 2018-07-13 16:04:23 -0700 Bug 186536 changed some WTF std::optional methods to release assert when accessing invalid (garbage) values, but those methods that use the TR2_OPTIONAL_ASSERTED_EXPRESSION macro still only assert on Debug builds. To match std::optional from the C++ standard library, we should release assert any time invalid (garbage) values are accessed. 1442093 1 webkit-bug-importer 2018-07-13 16:19:53 -0700 <rdar://problem/42185658> 1442106 2 ddkilzer 2018-07-13 16:58:22 -0700 Oh, maybe this is a dupe of Bug 186536. I thought a fix landed for that, but I see it was only Debug assertions. 1442184 3 fred.wang 2018-07-13 23:38:01 -0700 This was initially done in https://bugs.webkit.org/show_bug.cgi?id=186536#c27 but some people think it should not assert in release mode so an alternative debug-only version was landed. 1442220 4 345037 ddkilzer 2018-07-14 10:23:51 -0700 Created attachment 345037 Patch v1 1442221 5 ddkilzer 2018-07-14 10:24:29 -0700 (In reply to David Kilzer (:ddkilzer) from comment #4) > Created attachment 345037 [details] > Patch v1 Just posting a patch to do this; not going to review/land yet. 1442608 6 rniwa 2018-07-16 19:58:20 -0700 This caused a bunch of real crashes which broke a bunch of manual QA testing scenarios so all those cases need to be first fixed before this patch can be re-landed. Also, someone should get this through Speedometer 2, Dromaeo, etc... on macOS and iOS to make sure there are no perf impact. 1442672 7 mcatanzaro 2018-07-17 08:40:24 -0700 (In reply to Ryosuke Niwa from comment #6) > This caused a bunch of real crashes which broke a bunch of manual QA testing > scenarios so all those cases need to be first fixed before this patch can be > re-landed. Then I insist on switching GTK/WPE back to C++ 14. JF, sorry, but I don't want GTK/WPE to be dogfooding these crashes until you're ready to switch the XCode build over as well. I don't think it makes any sense that we've been building different ports with different C++ standards versions for several months now. When you're ready to switch XCode over to C++ 17 (at which point you'll have no choice but to fix these crashes :) then we can do so for the CMake ports as well. Does that sound OK? 1442871 8 ddkilzer 2018-07-17 17:41:31 -0700 (In reply to Michael Catanzaro from comment #7) > (In reply to Ryosuke Niwa from comment #6) > > This caused a bunch of real crashes which broke a bunch of manual QA testing > > scenarios so all those cases need to be first fixed before this patch can be > > re-landed. > > Then I insist on switching GTK/WPE back to C++ 14. JF, sorry, but I don't > want GTK/WPE to be dogfooding these crashes until you're ready to switch the > XCode build over as well. I don't think it makes any sense that we've been > building different ports with different C++ standards versions for several > months now. When you're ready to switch XCode over to C++ 17 (at which point > you'll have no choice but to fix these crashes :) then we can do so for the > CMake ports as well. Does that sound OK? I won’t land this until we’ve fixed known issues. 1444059 9 ddkilzer 2018-07-21 10:07:32 -0700 (In reply to David Kilzer (:ddkilzer) from comment #8) > I won’t land this until we’ve fixed known issues. I've been living on a build of WebKit with this patch applied, and haven't run into any crashes. I suspect we've fixed whatever (common) situations there were where unset values were being used. 1446792 10 rniwa 2018-07-31 19:38:27 -0700 Please make sure <rdar://problem/41860280> is fixed. Also, please run all perf tests: PLT, Speedometer, MotionMark, etc... to make sure there is no perf regression from this change. 1449463 11 ddkilzer 2018-08-10 20:25:41 -0700 (In reply to Ryosuke Niwa from comment #10) > Please make sure <rdar://problem/41860280> is fixed. This is fixed. Verified by source code inspection, and steps to reproduce: 1. Launch Safari 2. Navigate to drive.google.com 3. Open sheets. 4. Refresh drive.google.com 5. Repeat steps 2-4. > Also, please run all perf tests: PLT, Speedometer, MotionMark, etc... to > make sure there is no perf regression from this change. Running those now. 1450929 12 ddkilzer 2018-08-16 16:05:18 -0700 (In reply to David Kilzer (:ddkilzer) from comment #11) > (In reply to Ryosuke Niwa from comment #10) > > Please make sure <rdar://problem/41860280> is fixed. > > This is fixed. Verified by source code inspection, and steps to reproduce: > > 1. Launch Safari > 2. Navigate to drive.google.com > 3. Open sheets. > 4. Refresh drive.google.com > 5. Repeat steps 2-4. > > > Also, please run all perf tests: PLT, Speedometer, MotionMark, etc... to > > make sure there is no perf regression from this change. > > Running those now. Speedometer was statistically insignificant (0.13% worse for Time : Total; 0.07% worse for Score). MotionMark was 1.13% better (statistically significant). PLT was 0.31% better for arithmetic mean, and 0.32% better for geometric mean (both statistically significant). 1450975 13 345037 commit-queue 2018-08-16 17:48:58 -0700 Comment on attachment 345037 Patch v1 Rejecting attachment 345037 from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 345037, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=345037&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=187669&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Processing patch 345037 from bug 187669. Fetching: https://bugs.webkit.org/attachment.cgi?id=345037 Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Ryosuke Niwa']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Parsed 3 diffs from patch file(s). patching file Source/WTF/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WTF/wtf/Assertions.h Hunk #1 FAILED at 517. 1 out of 1 hunk FAILED -- saving rejects to file Source/WTF/wtf/Assertions.h.rej patching file Source/WTF/wtf/Optional.h Hunk #1 succeeded at 516 (offset -4 lines). Hunk #2 succeeded at 661 (offset -4 lines). Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Ryosuke Niwa']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: https://webkit-queues.webkit.org/results/8886784 1451151 14 ddkilzer 2018-08-17 10:36:12 -0700 Committed r234980: <https://trac.webkit.org/changeset/234980> 345037 2018-07-14 10:23:51 -0700 2018-08-16 17:48:58 -0700 Patch v1 bug-187669-20180714102343.patch text/plain 4836 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMjMzODM2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IGU5ZmVmMjlkMjk1ODQ3Y2Y0NDg5ZjEz OWU5YmQ2YTUwYzhjNDRjNzkuLjY1ODAyN2ZiNzVkZTY3Y2YwZjU0YTk2MjViNGE2OTE3YjZjNzJk OGQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMjUgQEAKKzIwMTgtMDctMTMgIERhdmlkIEtpbHplciAgPGRka2ls emVyQGFwcGxlLmNvbT4KKworICAgICAgICBXVEYncyBpbnRlcm5hbCBzdGQ6Om9wdGlvbmFsIGlt cGxlbWVudGF0aW9uIHNob3VsZCByZWxlYXNlIGFzc2VydCBvbiBhbGwgYmFkIGFjY2Vzc2VzCisg ICAgICAgIDxodHRwczovL3dlYmtpdC5vcmcvYi8xODc2Njk+CisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiB3dGYvQXNzZXJ0aW9ucy5oOgorICAgICAg ICAoUkVMRUFTRV9BU1NFUlRfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQpOiBBZGQgbWFjcm8gZGVm aW5pdGlvbnMuCisgICAgICAgICogd3RmL09wdGlvbmFsLmg6CisgICAgICAgIChzdGQ6Om9wdGlv bmFsOjpvcGVyYXRvciAtPiBjb25zdCk6CisgICAgICAgIChzdGQ6Om9wdGlvbmFsOjpvcGVyYXRv ciAtPik6CisgICAgICAgIChzdGQ6Om9wdGlvbmFsOjpvcGVyYXRvciAqIGNvbnN0KToKKyAgICAg ICAgKHN0ZDo6b3B0aW9uYWw6Om9wZXJhdG9yICopOgorICAgICAgICAoc3RkOjpvcHRpb25hbDo6 dmFsdWUgY29uc3QpOgorICAgICAgICAoc3RkOjpvcHRpb25hbDo6dmFsdWUpOgorICAgICAgICAo c3RkOjpvcHRpb25hbDxUOjpvcGVyYXRvci0+IGNvbnN0KToKKyAgICAgICAgKHN0ZDo6b3B0aW9u YWw8VDo6b3BlcmF0b3IqIGNvbnN0KToKKyAgICAgICAgKHN0ZDo6b3B0aW9uYWw8VDo6dmFsdWUg Y29uc3QpOgorICAgICAgICAtIENoYW5nZSBBU1NFUlRfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQo KSBtYWNyb3MgdG8KKyAgICAgICAgICBSRUxFQVNFX0FTU0VSVF9VTkRFUl9DT05TVEVYUFJfQ09O VEVYVCgpIG1hY3Jvcy4KKwogMjAxOC0wNy0xNCAgRGF2aWQgS2lsemVyICA8ZGRraWx6ZXJAYXBw bGUuY29tPgogCiAgICAgICAgIFJlcGxhY2UgVFIyX09QVElPTkFMX0FTU0VSVEVEX0VYUFJFU1NJ T04gbWFjcm8gaW4gPHd0Zi9PcHRpb25hbC5oPgpkaWZmIC0tZ2l0IGEvU291cmNlL1dURi93dGYv QXNzZXJ0aW9ucy5oIGIvU291cmNlL1dURi93dGYvQXNzZXJ0aW9ucy5oCmluZGV4IDYwYWQ0Yzc3 NTcxNjdkZTVjNzE1OTQ1ODgzNWY2MjQzZDY0MTU5YzkuLjBjN2Q4YWMwZGRkYTZlYzllMTBlMTdm YTVjZWUxM2ViMjk2MzkxYjAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL0Fzc2VydGlvbnMu aAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9Bc3NlcnRpb25zLmgKQEAgLTUxNywxMSArNTE3LDE3IEBA IFdURl9FWFBPUlRfUFJJVkFURSBOT19SRVRVUk5fRFVFX1RPX0NSQVNIIHZvaWQgV1RGQ3Jhc2hX aXRoU2VjdXJpdHlJbXBsaWNhdGlvbih2CiAjZGVmaW5lIFJFTEVBU0VfQVNTRVJUX1dJVEhfTUVT U0FHRShhc3NlcnRpb24sIC4uLikgUkVMRUFTRV9BU1NFUlQoYXNzZXJ0aW9uKQogI2RlZmluZSBS RUxFQVNFX0FTU0VSVF9XSVRIX1NFQ1VSSVRZX0lNUExJQ0FUSU9OKGFzc2VydGlvbikgUkVMRUFT RV9BU1NFUlQoYXNzZXJ0aW9uKQogI2RlZmluZSBSRUxFQVNFX0FTU0VSVF9OT1RfUkVBQ0hFRCgp IENSQVNIKCkKKyNkZWZpbmUgUkVMRUFTRV9BU1NFUlRfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQo YXNzZXJ0aW9uKSBkbyB7IFwKKyAgICBpZiAoVU5MSUtFTFkoIShhc3NlcnRpb24pKSkgeyBcCisg ICAgICAgIENSQVNIX1VOREVSX0NPTlNURVhQUl9DT05URVhUKCk7IFwKKyAgICB9IFwKK30gd2hp bGUgKDApCiAjZWxzZQogI2RlZmluZSBSRUxFQVNFX0FTU0VSVChhc3NlcnRpb24pIEFTU0VSVChh c3NlcnRpb24pCiAjZGVmaW5lIFJFTEVBU0VfQVNTRVJUX1dJVEhfTUVTU0FHRShhc3NlcnRpb24s IC4uLikgQVNTRVJUX1dJVEhfTUVTU0FHRShhc3NlcnRpb24sIF9fVkFfQVJHU19fKQogI2RlZmlu ZSBSRUxFQVNFX0FTU0VSVF9XSVRIX1NFQ1VSSVRZX0lNUExJQ0FUSU9OKGFzc2VydGlvbikgQVNT RVJUX1dJVEhfU0VDVVJJVFlfSU1QTElDQVRJT04oYXNzZXJ0aW9uKQogI2RlZmluZSBSRUxFQVNF X0FTU0VSVF9OT1RfUkVBQ0hFRCgpIEFTU0VSVF9OT1RfUkVBQ0hFRCgpCisjZGVmaW5lIFJFTEVB U0VfQVNTRVJUX1VOREVSX0NPTlNURVhQUl9DT05URVhUKGFzc2VydGlvbikgQVNTRVJUX1VOREVS X0NPTlNURVhQUl9DT05URVhUKGFzc2VydGlvbikKICNlbmRpZgogCiAvKiBVTlJFQUNIQUJMRV9G T1JfUExBVEZPUk0gKi8KZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvd3RmL09wdGlvbmFsLmggYi9T b3VyY2UvV1RGL3d0Zi9PcHRpb25hbC5oCmluZGV4IGU2NGE4MGE1ZDZkNzRiYzRiMzIzMmM2YWU2 YjFlOWU4M2UwOTRlYjAuLjhlM2E4MDk0MmUyNWM0ZWUzMzY1OWQ5Mzg1ZThmN2NiMTMxY2QyNGUg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL09wdGlvbmFsLmgKKysrIGIvU291cmNlL1dURi93 dGYvT3B0aW9uYWwuaApAQCAtNTIwLDQyICs1MjAsNDIgQEAgcHVibGljOgogICBjb25zdGV4cHIg Ym9vbCBoYXNfdmFsdWUoKSBjb25zdCBfX05PRVhDRVBUIHsgcmV0dXJuIGluaXRpYWxpemVkKCk7 IH0KIAogICBjb25zdGV4cHIgVCBjb25zdCogb3BlcmF0b3IgLT4oKSBjb25zdCB7Ci0gICAgQVNT RVJUX1VOREVSX0NPTlNURVhQUl9DT05URVhUKGluaXRpYWxpemVkKCkpOworICAgIFJFTEVBU0Vf QVNTRVJUX1VOREVSX0NPTlNURVhQUl9DT05URVhUKGluaXRpYWxpemVkKCkpOwogICAgIHJldHVy biBkYXRhcHRyKCk7CiAgIH0KIAogICBPUFRJT05BTF9NVVRBQkxFX0NPTlNURVhQUiBUKiBvcGVy YXRvciAtPigpIHsKLSAgICBBU1NFUlRfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQoaW5pdGlhbGl6 ZWQoKSk7CisgICAgUkVMRUFTRV9BU1NFUlRfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQoaW5pdGlh bGl6ZWQoKSk7CiAgICAgcmV0dXJuIGRhdGFwdHIoKTsKICAgfQogCiAgIGNvbnN0ZXhwciBUIGNv bnN0JiBvcGVyYXRvciAqKCkgY29uc3QmIHsKLSAgICBBU1NFUlRfVU5ERVJfQ09OU1RFWFBSX0NP TlRFWFQoaW5pdGlhbGl6ZWQoKSk7CisgICAgUkVMRUFTRV9BU1NFUlRfVU5ERVJfQ09OU1RFWFBS X0NPTlRFWFQoaW5pdGlhbGl6ZWQoKSk7CiAgICAgcmV0dXJuIGNvbnRhaW5lZF92YWwoKTsKICAg fQogCiAgIE9QVElPTkFMX01VVEFCTEVfQ09OU1RFWFBSIFQmIG9wZXJhdG9yICooKSAmIHsKLSAg ICBBU1NFUlRfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQoaW5pdGlhbGl6ZWQoKSk7CisgICAgUkVM RUFTRV9BU1NFUlRfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQoaW5pdGlhbGl6ZWQoKSk7CiAgICAg cmV0dXJuIGNvbnRhaW5lZF92YWwoKTsKICAgfQogCiAgIE9QVElPTkFMX01VVEFCTEVfQ09OU1RF WFBSIFQmJiBvcGVyYXRvciAqKCkgJiYgewotICAgIEFTU0VSVF9VTkRFUl9DT05TVEVYUFJfQ09O VEVYVChpbml0aWFsaXplZCgpKTsKKyAgICBSRUxFQVNFX0FTU0VSVF9VTkRFUl9DT05TVEVYUFJf Q09OVEVYVChpbml0aWFsaXplZCgpKTsKICAgICByZXR1cm4gZGV0YWlsXzo6Y29uc3RleHByX21v dmUoY29udGFpbmVkX3ZhbCgpKTsKICAgfQogCiAgIGNvbnN0ZXhwciBUIGNvbnN0JiB2YWx1ZSgp IGNvbnN0JiB7Ci0gICAgQVNTRVJUX1VOREVSX0NPTlNURVhQUl9DT05URVhUKGluaXRpYWxpemVk KCkpOworICAgIFJFTEVBU0VfQVNTRVJUX1VOREVSX0NPTlNURVhQUl9DT05URVhUKGluaXRpYWxp emVkKCkpOwogICAgIHJldHVybiBjb250YWluZWRfdmFsKCk7CiAgIH0KIAogICBPUFRJT05BTF9N VVRBQkxFX0NPTlNURVhQUiBUJiB2YWx1ZSgpICYgewotICAgIEFTU0VSVF9VTkRFUl9DT05TVEVY UFJfQ09OVEVYVChpbml0aWFsaXplZCgpKTsKKyAgICBSRUxFQVNFX0FTU0VSVF9VTkRFUl9DT05T VEVYUFJfQ09OVEVYVChpbml0aWFsaXplZCgpKTsKICAgICByZXR1cm4gY29udGFpbmVkX3ZhbCgp OwogICB9CiAKICAgT1BUSU9OQUxfTVVUQUJMRV9DT05TVEVYUFIgVCYmIHZhbHVlKCkgJiYgewot ICAgIEFTU0VSVF9VTkRFUl9DT05TVEVYUFJfQ09OVEVYVChpbml0aWFsaXplZCgpKTsKKyAgICBS RUxFQVNFX0FTU0VSVF9VTkRFUl9DT05TVEVYUFJfQ09OVEVYVChpbml0aWFsaXplZCgpKTsKICAg ICByZXR1cm4gc3RkOjptb3ZlKGNvbnRhaW5lZF92YWwoKSk7CiAgIH0KIApAQCAtNjY1LDE3ICs2 NjUsMTcgQEAgcHVibGljOgogCiAgIC8vIDIwLjUuNS4zLCBvYnNlcnZlcnMKICAgY29uc3RleHBy IFQqIG9wZXJhdG9yLT4oKSBjb25zdCB7Ci0gICAgQVNTRVJUX1VOREVSX0NPTlNURVhQUl9DT05U RVhUKHJlZik7CisgICAgUkVMRUFTRV9BU1NFUlRfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQocmVm KTsKICAgICByZXR1cm4gcmVmOwogICB9CiAKICAgY29uc3RleHByIFQmIG9wZXJhdG9yKigpIGNv bnN0IHsKLSAgICBBU1NFUlRfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQocmVmKTsKKyAgICBSRUxF QVNFX0FTU0VSVF9VTkRFUl9DT05TVEVYUFJfQ09OVEVYVChyZWYpOwogICAgIHJldHVybiAqcmVm OwogICB9CiAKICAgY29uc3RleHByIFQmIHZhbHVlKCkgY29uc3QgewotICAgIEFTU0VSVF9VTkRF Ul9DT05TVEVYUFJfQ09OVEVYVChyZWYoKSk7CisgICAgUkVMRUFTRV9BU1NFUlRfVU5ERVJfQ09O U1RFWFBSX0NPTlRFWFQocmVmKCkpOwogICAgIHJldHVybiAqcmVmOwogICB9CiAK