[Coverity] Address use-after-free report in MemoryCache
Created attachment 120105 [details] Patch
What does this use-after-free report mean? Is this just a false positive that you're shutting up?
Basically, yes. Coverity is worried that evict(revalidatingResource) can delete revalidatingResource, and then it gets used below. But canDelete() is clearly false if isLoaded() is true. That's beyond Coverity's abilities, but usually that means humans have a hard time deciphering such facts as well. :-)
OK. Maybe that's worth a comment in code in addition to the ASSERT. CC'ing some folks who worked on this code in case something is this function jumps out as incorrect once this assertion for documenting behavior is added.
Created attachment 120191 [details] Patch
Added a comment.
Comment on attachment 120191 [details] Patch + // The evict() call can delete revalidatingResource, which we then use + // below. Assert explicitly that this can't happen. I'd state this differently: // Calling evict() can potentially delete the resource. This won't happen here because <...>
Created attachment 120223 [details] Patch
Done.
Comment on attachment 120223 [details] Patch OK.
Attachment 120223 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource From git://git.webkit.org/WebKit daea471..578c1d3 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 103458 = daea471373d620a3fbc3e7dbc138f927d151a5c9 r103460 = 578c1d3f4da422dcdedb98687c619ba79bfeb36f Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Applying: Inform the scrolling coordinator when scrollbar layers come and go Using index info to reconstruct a base tree... <stdin>:474806: trailing whitespace. [Chromium] DatabaseTrackerChromium: iterating DatabaseSet races with Database disposal on worker thread <stdin>:474827: trailing whitespace. Nothing to test, just removing redundant code. Correct behavior tested by <stdin>:475346: trailing whitespace. warning: 3 lines add whitespace errors. Falling back to patching base and 3-way merge... warning: too many files (created: 167249 deleted: 3), skipping inexact rename detection Auto-merging LayoutTests/ChangeLog CONFLICT (content): Merge conflict in LayoutTests/ChangeLog Auto-merging LayoutTests/platform/chromium/test_expectations.txt Auto-merging Source/WebCore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Auto-merging Source/WebCore/rendering/RenderLayerCompositor.cpp CONFLICT (content): Merge conflict in Source/WebCore/rendering/RenderLayerCompositor.cpp Auto-merging Source/WebKit2/ChangeLog CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog Failed to merge in the changes. Patch failed at 0001 Inform the scrolling coordinator when scrollbar layers come and go When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 158. If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 120223 [details] Patch Clearing flags on attachment: 120223 Committed r103513: <http://trac.webkit.org/changeset/103513>
All reviewed patches have been landed. Closing bug.