| Summary: | [Cocoa] REGRESSION (r260485): Crash in Legacy WebKit createMenu item function (reproducible under Asan) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Darin Adler <darin> | ||||
| Component: | WebKit Misc. | Assignee: | Darin Adler <darin> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Critical | CC: | achristensen, ap, Lawrence.j, ryanhaddad | ||||
| Priority: | P1 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Mac | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
|
Description
Darin Adler
2020-04-22 16:38:08 PDT
Created attachment 397287 [details]
Patch
Comment on attachment 397287 [details]
Patch
I verified this fixes the issue.
Committed r260545: <https://trac.webkit.org/changeset/260545> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397287 [details]. 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. (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! |