vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus,
manas, RedDocMD.
Herald added subscribers: ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D104136
Files:
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
clang/test/Analysis/osobject-retain-release.cpp
Index: clang/test/Analysis/osobject-retain-release.cpp
===================================================================
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -536,6 +536,7 @@
void check_dynamic_cast_alias() {
OSObject *originalPtr = OSObject::generateObject(1); // expected-note {{Call to method 'OSObject::generateObject' returns an OSObject}}
+ // expected-note@-1 {{'originalPtr' initialized here}}
OSArray *newPtr = OSDynamicCast(OSArray, originalPtr); // expected-note {{'newPtr' initialized to the value of 'originalPtr'}}
if (newPtr) { // expected-note {{'newPtr' is non-null}}
// expected-note@-1 {{Taking true branch}}
@@ -548,6 +549,7 @@
void check_dynamic_cast_alias_cond() {
OSObject *originalPtr = OSObject::generateObject(1); // expected-note {{Call to method 'OSObject::generateObject' returns an OSObject}}
+ // expected-note@-1 {{'originalPtr' initialized here}}
OSArray *newPtr = 0;
if ((newPtr = OSDynamicCast(OSArray, originalPtr))) { // expected-note {{The value of 'originalPtr' is assigned to 'newPtr'}}
// expected-note@-1 {{'newPtr' is non-null}}
@@ -561,7 +563,8 @@
void check_dynamic_cast_alias_intermediate() {
OSObject *originalPtr = OSObject::generateObject(1); // expected-note {{Call to method 'OSObject::generateObject' returns an OSObject of type 'OSObject' with a +1 retain count}}
- OSObject *intermediate = originalPtr; // TODO: add note here as well
+ // expected-note@-1 {{'originalPtr' initialized here}}
+ OSObject *intermediate = originalPtr; // expected-note {{'intermediate' initialized to the value of 'originalPtr'}}
OSArray *newPtr = 0;
if ((newPtr = OSDynamicCast(OSArray, intermediate))) { // expected-note {{The value of 'intermediate' is assigned to 'newPtr'}}
// expected-note@-1 {{'newPtr' is non-null}}
@@ -575,7 +578,8 @@
void check_dynamic_cast_alias_intermediate_2() {
OSObject *originalPtr = OSObject::generateObject(1); // expected-note {{Call to method 'OSObject::generateObject' returns an OSObject of type 'OSObject' with a +1 retain count}}
- OSObject *intermediate = originalPtr; // TODO: add note here as well
+ // expected-note@-1 {{'originalPtr' initialized here}}
+ OSObject *intermediate = originalPtr; // expected-note {{'intermediate' initialized to the value of 'originalPtr'}}
OSArray *newPtr = 0;
if ((newPtr = OSDynamicCast(OSArray, intermediate))) { // expected-note {{Value assigned to 'newPtr'}}
// expected-note@-1 {{'newPtr' is non-null}}
Index: clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
===================================================================
--- clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
+++ clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
@@ -25282,6 +25282,35 @@
<key>message</key>
<string>Call to function 'CFCreateSomething' returns a Core Foundation object of type 'CFTypeRef' with a +1 retain count</string>
</dict>
+ <dict>
+ <key>kind</key><string>event</string>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>2285</integer>
+ <key>col</key><integer>3</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>ranges</key>
+ <array>
+ <array>
+ <dict>
+ <key>line</key><integer>2285</integer>
+ <key>col</key><integer>3</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>2285</integer>
+ <key>col</key><integer>15</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </array>
+ <key>depth</key><integer>0</integer>
+ <key>extended_message</key>
+ <string>'obj' initialized here</string>
+ <key>message</key>
+ <string>'obj' initialized here</string>
+ </dict>
<dict>
<key>kind</key><string>control</string>
<key>edges</key>
@@ -25569,6 +25598,35 @@
<key>message</key>
<string>Call to function 'CFArrayCreateMutable' returns a Core Foundation object of type 'CFMutableArrayRef' with a +1 retain count</string>
</dict>
+ <dict>
+ <key>kind</key><string>event</string>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>2305</integer>
+ <key>col</key><integer>3</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>ranges</key>
+ <array>
+ <array>
+ <dict>
+ <key>line</key><integer>2305</integer>
+ <key>col</key><integer>3</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>2305</integer>
+ <key>col</key><integer>16</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </array>
+ <key>depth</key><integer>0</integer>
+ <key>extended_message</key>
+ <string>'arr' initialized here</string>
+ <key>message</key>
+ <string>'arr' initialized here</string>
+ </dict>
<dict>
<key>kind</key><string>control</string>
<key>edges</key>
Index: clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
===================================================================
--- clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
+++ clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
@@ -25213,6 +25213,35 @@
<key>message</key>
<string>Call to function 'CFCreateSomething' returns a Core Foundation object of type 'CFTypeRef' with a +1 retain count</string>
</dict>
+ <dict>
+ <key>kind</key><string>event</string>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>2285</integer>
+ <key>col</key><integer>3</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>ranges</key>
+ <array>
+ <array>
+ <dict>
+ <key>line</key><integer>2285</integer>
+ <key>col</key><integer>3</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>2285</integer>
+ <key>col</key><integer>15</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </array>
+ <key>depth</key><integer>0</integer>
+ <key>extended_message</key>
+ <string>'obj' initialized here</string>
+ <key>message</key>
+ <string>'obj' initialized here</string>
+ </dict>
<dict>
<key>kind</key><string>control</string>
<key>edges</key>
@@ -25500,6 +25529,35 @@
<key>message</key>
<string>Call to function 'CFArrayCreateMutable' returns a Core Foundation object of type 'CFMutableArrayRef' with a +1 retain count</string>
</dict>
+ <dict>
+ <key>kind</key><string>event</string>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>2305</integer>
+ <key>col</key><integer>3</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>ranges</key>
+ <array>
+ <array>
+ <dict>
+ <key>line</key><integer>2305</integer>
+ <key>col</key><integer>3</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>2305</integer>
+ <key>col</key><integer>16</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </array>
+ <key>depth</key><integer>0</integer>
+ <key>extended_message</key>
+ <string>'arr' initialized here</string>
+ <key>message</key>
+ <string>'arr' initialized here</string>
+ </dict>
<dict>
<key>kind</key><string>control</string>
<key>edges</key>
Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -934,6 +934,44 @@
}
}
+// A handler designed to help the tracker with identity function calls.
+//
+// The checker models a good number of functions as "identity", i.e.
+// returning the same value it was given. The tracker doesn't understand
+// these semantics and calls to such functions are opaque. Here we aid
+// the tracker by promoting the tracking process through such calls.
+class IdentityHandler final : public bugreporter::ExpressionHandler {
+public:
+ using bugreporter::ExpressionHandler::ExpressionHandler;
+
+ bugreporter::Tracker::Result
+ handle(const Expr *E, const ExplodedNode *Original,
+ const ExplodedNode *ExprNode,
+ bugreporter::TrackingOptions Opts) override {
+ // We care only for calls.
+ if (const auto *CE = dyn_cast<CallExpr>(E)) {
+ // Let's traverse...
+ for (const ExplodedNode *N = ExprNode;
+ // ...all the nodes corresponding to the given expression...
+ N != nullptr && N->getStmtForDiagnostics() == E;
+ N = N->getFirstPred()) {
+
+ ProgramPoint PP = N->getLocation();
+ // ...looking for a node that was marked as a call to identity function.
+ if (const IdentityTag *T = dyn_cast_or_null<IdentityTag>(PP.getTag())) {
+ // Validate (just in case) that it was indeed about this call.
+ if (T->getIdentityCall() != CE)
+ continue;
+
+ // And keep tracker going!
+ return getParentTracker().track(T->getSource(), N, Opts);
+ }
+ }
+ }
+ return {};
+ }
+};
+
void RefLeakReport::findBindingToReport(CheckerContext &Ctx,
ExplodedNode *Node) {
if (!AllocFirstBinding)
@@ -984,8 +1022,10 @@
// something like derived regions if we want to construct SVal from
// Sym. Instead, we take the value that is definitely stored in that
// region, thus guaranteeing that trackStoredValue will work.
- bugreporter::trackStoredValue(AllVarBindings[0].second.castAs<KnownSVal>(),
- AllocBindingToReport, *this);
+ auto AllocatedValueTracker = bugreporter::Tracker::create(*this);
+ AllocatedValueTracker->addHighPriorityHandler<IdentityHandler>();
+ AllocatedValueTracker->track(AllVarBindings[0].second,
+ AllocBindingToReport);
} else {
AllocBindingToReport = AllocFirstBinding;
}
Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
+++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
@@ -235,6 +235,32 @@
void print(raw_ostream &Out) const;
};
+/// A special tag to mark calls to identity functions.
+///
+/// It helps checker to produce better notes.
+class IdentityTag : public DataTag {
+private:
+ static int Kind;
+ const CallExpr *IdentityCall;
+ const Expr *Source;
+
+ IdentityTag(const CallExpr *IdentityCall, const Expr *Source)
+ : DataTag(&Kind), IdentityCall(IdentityCall), Source(Source) {}
+
+public:
+ static bool classof(const ProgramPointTag *T) {
+ return T->getTagKind() == &Kind;
+ }
+
+ /// Return a call to identity function.
+ const CallExpr *getIdentityCall() const { return IdentityCall; }
+
+ /// Return the argument expression that is equal to the call's return value.
+ const Expr *getSource() const { return Source; }
+
+ friend class Factory;
+};
+
class RetainCountChecker
: public Checker< check::Bind,
check::DeadSymbols,
Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -33,6 +33,8 @@
} // end namespace ento
} // end namespace clang
+int IdentityTag::Kind = 0;
+
static ProgramStateRef setRefBinding(ProgramStateRef State, SymbolRef Sym,
RefVal Val) {
assert(Sym != nullptr);
@@ -914,6 +916,8 @@
if (!BSmr)
return false;
+ const ProgramPointTag *Tag = nullptr;
+
// Bind the return value.
if (BSmr == BehaviorSummary::Identity ||
BSmr == BehaviorSummary::IdentityOrZero ||
@@ -938,6 +942,9 @@
// Bind the value.
state = state->BindExpr(CE, LCtx, RetVal, /*Invalidate=*/false);
+ ExprEngine &Eng = C.getStateManager().getOwningEngine();
+ // Let's mark this place with a special tag.
+ Tag = Eng.getDataTags().make<IdentityTag>(CE, BindReturnTo);
if (BSmr == BehaviorSummary::IdentityOrZero) {
// Add a branch where the output is zero.
@@ -952,11 +959,10 @@
// output are non-zero.
if (auto L = RetVal.getAs<DefinedOrUnknownSVal>())
state = state->assume(*L, /*assumption=*/true);
-
}
}
- C.addTransition(state);
+ C.addTransition(state, Tag);
return true;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
