223903 2021-03-29 13:54:53 -0700 UBSan: AlternativeTextController::dismiss()/dismissSoon(): runtime error: load of value nnn, which is not a valid value for type 'bool' 2021-04-02 14:05:03 -0700 1 1 1 Unclassified WebKit HTML Editing WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=174604 https://bugs.webkit.org/show_bug.cgi?id=176131 https://bugs.webkit.org/show_bug.cgi?id=223902 InRadar P2 Normal --- 1 ddkilzer ddkilzer ap darin dbates ews-watchlist mifenton ryanhaddad webkit-bug-importer wenson_hsieh oldest_to_newest 1744897 0 ddkilzer 2021-03-29 13:54:53 -0700 Running all layout tests with a Release+UBSan build of WebKit (see Bug 176131) results in ~449 tests hitting this UBSan warning at least once with different values of "nnn": editing/AlternativeTextController.cpp:180:10: runtime error: load of value nnn, which is not a valid value for type 'bool' editing/AlternativeTextController.cpp:190:10: runtime error: load of value nnn, which is not a valid value for type 'bool' This seems to result from AlternativeTextController::m_isActive not being initialized in the constructor. In both cases the code that hits the UBSan runtime error is: if (!m_isActive) // UBSan runtime error on line 180 or 190 of editing/AlternativeTextController.cpp. return; 1744903 1 424576 ddkilzer 2021-03-29 14:00:02 -0700 Created attachment 424576 Patch v1 1744907 2 ddkilzer 2021-03-29 14:05:54 -0700 (In reply to David Kilzer (:ddkilzer) from comment #1) > Created attachment 424576 [details] > Patch for EWS I had a lot of macOS tests (46) fail locally with this patch, but I can't tell which ones were caused by this patch vs. some other issue, so I'm using EWS to check how many tests might be failing with this change in non-UBSan builds. I saw both "EDITING DELEGATE" changes and layout/rendering changes locally with WebKit recompiled with UBSan, which is somewhat scary. 1744908 3 webkit-bug-importer 2021-03-29 14:08:36 -0700 <rdar://problem/75972281> 1744989 4 ddkilzer 2021-03-29 17:16:32 -0700 (In reply to David Kilzer (:ddkilzer) from comment #2) > (In reply to David Kilzer (:ddkilzer) from comment #1) > > Created attachment 424576 [details] > > Patch for EWS > > I had a lot of macOS tests (46) fail locally with this patch, but I can't > tell which ones were caused by this patch vs. some other issue, so I'm using > EWS to check how many tests might be failing with this change in non-UBSan > builds. > > I saw both "EDITING DELEGATE" changes and layout/rendering changes locally > with WebKit recompiled with UBSan, which is somewhat scary. Heh, looks like the failures were just due to UBSan altering the timing of the tests. 1745008 5 ap 2021-03-29 18:32:20 -0700 If it's actually timing, maybe we should mark those 46 as flaky preemptively? 1745365 6 424576 ddkilzer 2021-03-30 15:27:03 -0700 Comment on attachment 424576 Patch v1 Marking this for review. No regressions found in EWS. 1745367 7 ddkilzer 2021-03-30 15:28:57 -0700 (In reply to Alexey Proskuryakov from comment #5) > If it's actually timing, maybe we should mark those 46 as flaky preemptively? Sorry, I already overwrote the results. I'm going to run them again soon, so I'll take a closer look at the results and maybe suggest some suggestions. 1745368 8 424576 ddkilzer 2021-03-30 15:31:39 -0700 Comment on attachment 424576 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=424576&action=review > Source/WebCore/editing/AlternativeTextController.h:125 > + bool m_isActive { }; > + bool m_isDismissedByEditing { }; Note for reviewers: only m_isActive was identified by UBSSan as being used uninitialized. However, m_isDismissedByEditing is also uninitialized in the current constructor, so I'm making this change at the same time. 1745728 9 ddkilzer 2021-03-31 12:30:35 -0700 Review ping! :) 1746657 10 ews-feeder 2021-04-02 14:05:00 -0700 Committed r275436: <https://commits.webkit.org/r275436> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424576. 424576 2021-03-29 14:00:02 -0700 2021-04-02 14:05:01 -0700 Patch v1 bug-223903-20210329140001.patch text/plain 1828 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMjc1MTM3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNGJhOWFjYjdkYTczMGZi MDU3NzRhMGVkY2Y2NjMyMGU4MzA5ODI1My4uZGU5MWU4N2Q1NDMyZjhkMjU3YzNmZGUxOGUwMjg1 YjQ3YjRkNzc0YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDIxLTAzLTI5ICBEYXZp ZCBLaWx6ZXIgIDxkZGtpbHplckBhcHBsZS5jb20+CisKKyAgICAgICAgVUJTYW46IEFsdGVybmF0 aXZlVGV4dENvbnRyb2xsZXI6OmRpc21pc3MoKS9kaXNtaXNzU29vbigpOiBydW50aW1lIGVycm9y OiBsb2FkIG9mIHZhbHVlIG5ubiwgd2hpY2ggaXMgbm90IGEgdmFsaWQgdmFsdWUgZm9yIHR5cGUg J2Jvb2wnCisgICAgICAgIDxodHRwczovL3dlYmtpdC5vcmcvYi8yMjM5MDM+CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ292ZXJlZCBieSA0NDkgbGF5 b3V0IHRlc3RzIHVzaW5nIFdlYktpdCBjb21waWxlZCB3aXRoIFVCU2FuLgorCisgICAgICAgICog ZWRpdGluZy9BbHRlcm5hdGl2ZVRleHRDb250cm9sbGVyLmg6CisgICAgICAgIC0gSW5pdGlhbGl6 ZSBtX2lzQWN0aXZlIGFuZCBtX2lzRGlzbWlzc2VkQnlFZGl0aW5nLgorCiAyMDIxLTAzLTI5ICBE YXZpZCBLaWx6ZXIgIDxkZGtpbHplckBhcHBsZS5jb20+CiAKICAgICAgICAgUmVtb3ZlIHVudXNl ZCBzdHJ1Y3QgV2ViQ29yZTo6QWx0ZXJuYXRpdmVUZXh0Q29udHJvbGxlcjo6QWx0ZXJuYXRpdmVU ZXh0SW5mbwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvZWRpdGluZy9BbHRlcm5hdGl2ZVRl eHRDb250cm9sbGVyLmggYi9Tb3VyY2UvV2ViQ29yZS9lZGl0aW5nL0FsdGVybmF0aXZlVGV4dENv bnRyb2xsZXIuaAppbmRleCAxNjhhOGFiNTQzZGEyZGEwZDJjODk1OTRiM2MzNzhiMmJiNmNkYjY4 Li5iOWQzN2M3OTE4NzA0YjU2ZTMxYTY4ZjE4YjQ5ZWM4YjlkNjg2Y2Y1IDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViQ29yZS9lZGl0aW5nL0FsdGVybmF0aXZlVGV4dENvbnRyb2xsZXIuaAorKysgYi9T b3VyY2UvV2ViQ29yZS9lZGl0aW5nL0FsdGVybmF0aXZlVGV4dENvbnRyb2xsZXIuaApAQCAtMSw1 ICsxLDUgQEAKIC8qCi0gKiBDb3B5cmlnaHQgKEMpIDIwMTAtMjAxNyBBcHBsZSBJbmMuIEFsbCBy aWdodHMgcmVzZXJ2ZWQuCisgKiBDb3B5cmlnaHQgKEMpIDIwMTAtMjAyMSBBcHBsZSBJbmMuIEFs bCByaWdodHMgcmVzZXJ2ZWQuCiAgKgogICogUmVkaXN0cmlidXRpb24gYW5kIHVzZSBpbiBzb3Vy Y2UgYW5kIGJpbmFyeSBmb3Jtcywgd2l0aCBvciB3aXRob3V0CiAgKiBtb2RpZmljYXRpb24sIGFy ZSBwZXJtaXR0ZWQgcHJvdmlkZWQgdGhhdCB0aGUgZm9sbG93aW5nIGNvbmRpdGlvbnMKQEAgLTEy MSw4ICsxMjEsOCBAQCBwcml2YXRlOgogCiAgICAgVGltZXIgbV90aW1lcjsKICAgICBPcHRpb25h bDxTaW1wbGVSYW5nZT4gbV9yYW5nZVdpdGhBbHRlcm5hdGl2ZTsKLSAgICBib29sIG1faXNBY3Rp dmU7Ci0gICAgYm9vbCBtX2lzRGlzbWlzc2VkQnlFZGl0aW5nOworICAgIGJvb2wgbV9pc0FjdGl2 ZSB7IH07CisgICAgYm9vbCBtX2lzRGlzbWlzc2VkQnlFZGl0aW5nIHsgfTsKICAgICBBbHRlcm5h dGl2ZVRleHRUeXBlIG1fdHlwZTsKICAgICBTdHJpbmcgbV9vcmlnaW5hbFRleHQ7CiAgICAgVmFy aWFudDxBdXRvY29ycmVjdGlvblJlcGxhY2VtZW50LCBEaWN0YXRpb25Db250ZXh0PiBtX2RldGFp bHM7Cg==