Page MenuHomePhabricator

[analyzer] Add better tracking for RetainCountChecker leak warnings
Needs ReviewPublic

Authored by vsavchenko on Fri, Jun 11, 11:08 AM.

Details

Summary

RetainCountChecker models a group of calls that are interpreted as
"identity" functions. bugreporter::Tracker couldn't see through
such calls and pick the right argument for tracking.

This commit introduces a new tag that helps to keep this information
from the actual analysis and reuse it later for tracking purposes.

So, in short, when we see a call foo(x) that we model as
"identity", we now add a tag that is saying that the result of
foo(x) is essentially x. When the tracker, later on, tries to
track value from foo(x), we point out that it should track x.

Diff Detail

Event Timeline

vsavchenko created this revision.Fri, Jun 11, 11:08 AM
vsavchenko requested review of this revision.Fri, Jun 11, 11:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Jun 11, 11:08 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
NoQ added inline comments.Fri, Jun 11, 11:55 AM
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
953–957

I guess this part should ultimately be written in one place, eg. ExplodedNode::findTag() or something like that.

I'd also really like to explore the possibility to further limit the variety of nodes traversed here. What nodes are typically traversed here? Is it checker-tagged nodes or is it purge dead symbol nodes or something else?

Rebase

clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
953–957

Yes, ExplodedNode::findTag() sounds like a great idea!

I mean it is hard to tell without calculating statistics right here and running it on a bunch of projects. However, it is always possible to write the code that will have it the other way. My take on it is that it is probably a mix of things.

I'd also prefer to traverse less, do you have any specific ideas here?