210888 2020-04-22 16:38:08 -0700 [Cocoa] REGRESSION (r260485): Crash in Legacy WebKit createMenu item function (reproducible under Asan) 2020-04-22 17:51:49 -0700 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build Mac All RESOLVED FIXED InRadar P1 Critical --- 1 darin darin achristensen ap Lawrence.j ryanhaddad oldest_to_newest 1644572 0 darin 2020-04-22 16:38:08 -0700 [Cocoa] REGRESSION (r260485): Crash in Legacy WebKit createMenu item function (reproducible under Asan) 1644573 1 397287 darin 2020-04-22 16:39:07 -0700 Created attachment 397287 Patch 1644574 2 darin 2020-04-22 16:39:37 -0700 <rdar://problem/62207161> 1644596 3 397287 achristensen 2020-04-22 17:25:53 -0700 Comment on attachment 397287 Patch I verified this fixes the issue. 1644597 4 ews-feeder 2020-04-22 17:28:02 -0700 Committed r260545: <https://trac.webkit.org/changeset/260545> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397287. 1644598 5 achristensen 2020-04-22 17:31:41 -0700 View in context: https://bugs.webkit.org/attachment.cgi?id=397287&action=review > Source/WebKitLegacy/mac/ChangeLog:11 > + (createMenuItem): Speculative fix: Go back to using a local variable. Apparently > + the Objective-C for loop doesn't extend the lifetime of its argument the way the > + C++ range-based for loop does, so the local variable is needed. You can actually replace this ObjC for-in loop with a C++ range-based for loop and that doesn't fix the bug. The bug is that the RetainPtr is a temporary that goes out of scope immediately. The for loop (C++ or ObjC) iterates over the result of .get() so it keeps a raw NSMutableArray * in scope for the entirety of the iteration, but it's pointing to an array that has been freed. Just another bug that ARC would fix. 1644605 6 darin 2020-04-22 17:51:49 -0700 (In reply to Alex Christensen from comment #5) > You can actually replace this ObjC for-in loop with a C++ range-based for > loop and that doesn't fix the bug. Oh, right, because of the ".get()" because C++ extends the life of the expression, but not the subexpression before ".get()". Darn, frustrated I got that wrong! 397287 2020-04-22 16:39:07 -0700 2020-04-22 17:28:02 -0700 Patch bug-210888-20200422163906.patch text/plain 1910 darin U3VidmVyc2lvbiBSZXZpc2lvbjogMjYwNTM1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0TGVn YWN5L21hYy9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0TGVnYWN5L21hYy9DaGFuZ2VMb2cKaW5k ZXggZTdlNWFmYTM0YTk4NWJiZGM4YTVmNzBlMDNlMTc1MjQzNjI5NjM0ZS4uNjliYzVlYzg3NmY3 N2U4ZGQwZTQzZWFhMDRiOGY1ZDJlMjA4ODNmNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdExl Z2FjeS9tYWMvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXRMZWdhY3kvbWFjL0NoYW5nZUxv ZwpAQCAtMSwzICsxLDE1IEBACisyMDIwLTA0LTIyICBEYXJpbiBBZGxlciAgPGRhcmluQGFwcGxl LmNvbT4KKworICAgICAgICBbQ29jb2FdIFJFR1JFU1NJT04gKHIyNjA0ODUpOiBDcmFzaCBpbiBM ZWdhY3kgV2ViS2l0IGNyZWF0ZU1lbnUgaXRlbSBmdW5jdGlvbiAocmVwcm9kdWNpYmxlIHVuZGVy IEFzYW4pCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0y MTA4ODgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAq IFdlYlZpZXcvV2ViSFRNTFZpZXcubW06CisgICAgICAgIChjcmVhdGVNZW51SXRlbSk6IFNwZWN1 bGF0aXZlIGZpeDogR28gYmFjayB0byB1c2luZyBhIGxvY2FsIHZhcmlhYmxlLiBBcHBhcmVudGx5 CisgICAgICAgIHRoZSBPYmplY3RpdmUtQyBmb3IgbG9vcCBkb2Vzbid0IGV4dGVuZCB0aGUgbGlm ZXRpbWUgb2YgaXRzIGFyZ3VtZW50IHRoZSB3YXkgdGhlCisgICAgICAgIEMrKyByYW5nZS1iYXNl ZCBmb3IgbG9vcCBkb2VzLCBzbyB0aGUgbG9jYWwgdmFyaWFibGUgaXMgbmVlZGVkLgorCiAyMDIw LTA0LTIxICBQZW5nIExpdSAgPHBlbmcubGl1NkBhcHBsZS5jb20+CiAKICAgICAgICAgRml4IE1B Q0NBVEFMWVNUIGJ1aWxkIGZhaWx1cmVzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0TGVnYWN5 L21hYy9XZWJWaWV3L1dlYkhUTUxWaWV3Lm1tIGIvU291cmNlL1dlYktpdExlZ2FjeS9tYWMvV2Vi Vmlldy9XZWJIVE1MVmlldy5tbQppbmRleCA0YjZhMTE1ZmM5MzYzN2QzODNlYWRjZGU1OGRlYmM5 ZDVhZmVkOWE2Li5hYzZiNzEzOGI4ODk3YzM4OTI1M2UyNTI0NTA0ZDM0MGUwNjY5YjdhIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViS2l0TGVnYWN5L21hYy9XZWJWaWV3L1dlYkhUTUxWaWV3Lm1tCisr KyBiL1NvdXJjZS9XZWJLaXRMZWdhY3kvbWFjL1dlYlZpZXcvV2ViSFRNTFZpZXcubW0KQEAgLTM2 NzIsOSArMzY3MiwxMSBAQCBzdGF0aWMgUmV0YWluUHRyPE5TTWVudUl0ZW0+IGNyZWF0ZU1lbnVJ dGVtKGNvbnN0IFdlYkNvcmU6OkhpdFRlc3RSZXN1bHQmIGhpdFRlcwogCiAgICAgY2FzZSBXZWJD b3JlOjpTdWJtZW51VHlwZTogewogICAgICAgICBhdXRvIG1lbnUgPSBhZG9wdE5TKFtbTlNNZW51 IGFsbG9jXSBpbml0XSk7Ci0KLSAgICAgICAgZm9yIChOU01lbnVJdGVtICptZW51SXRlbSBpbiBj cmVhdGVNZW51SXRlbXMoaGl0VGVzdFJlc3VsdCwgaXRlbS5zdWJNZW51SXRlbXMoKSkuZ2V0KCkp Ci0gICAgICAgICAgICBbbWVudSBhZGRJdGVtOm1lbnVJdGVtXTsKKyAgICAgICAgeworICAgICAg ICAgICAgYXV0byBzdWJtZW51SXRlbXMgPSBjcmVhdGVNZW51SXRlbXMoaGl0VGVzdFJlc3VsdCwg aXRlbS5zdWJNZW51SXRlbXMoKSk7CisgICAgICAgICAgICBmb3IgKE5TTWVudUl0ZW0gKm1lbnVJ dGVtIGluIHN1Ym1lbnVJdGVtcy5nZXQoKSkKKyAgICAgICAgICAgICAgICBbbWVudSBhZGRJdGVt Om1lbnVJdGVtXTsKKyAgICAgICAgfQogCiAgICAgICAgIGF1dG8gbWVudUl0ZW0gPSBhZG9wdE5T KFtbTlNNZW51SXRlbSBhbGxvY10gaW5pdFdpdGhUaXRsZTppdGVtLnRpdGxlKCkgYWN0aW9uOm51 bGxwdHIga2V5RXF1aXZhbGVudDpAIiJdKTsKIAo=