diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ b/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, diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ b/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(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()) state = state->assume(*L, /*assumption=*/true); - } } - C.addTransition(state); + C.addTransition(state, Tag); return true; } diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp @@ -666,6 +666,44 @@ const LocationContext *InInterestingMethodContext) : N(InN), R(InR), InterestingMethodContext(InInterestingMethodContext) {} }; + +// 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(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(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 {}; + } +}; } // end anonymous namespace static AllocationInfo GetAllocationSite(ProgramStateManager &StateMgr, @@ -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(), - AllocBindingToReport, *this); + auto AllocatedValueTracker = bugreporter::Tracker::create(*this); + AllocatedValueTracker->addHighPriorityHandler(); + AllocatedValueTracker->track(AllVarBindings[0].second, + AllocBindingToReport); } else { AllocBindingToReport = AllocFirstBinding; } diff --git a/clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist b/clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist --- a/clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist +++ b/clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist @@ -25213,6 +25213,35 @@ message Call to function 'CFCreateSomething' returns a Core Foundation object of type 'CFTypeRef' with a +1 retain count + + kindevent + location + + line2285 + col3 + file0 + + ranges + + + + line2285 + col3 + file0 + + + line2285 + col15 + file0 + + + + depth0 + extended_message + 'obj' initialized here + message + 'obj' initialized here + kindcontrol edges @@ -25500,6 +25529,35 @@ message Call to function 'CFArrayCreateMutable' returns a Core Foundation object of type 'CFMutableArrayRef' with a +1 retain count + + kindevent + location + + line2305 + col3 + file0 + + ranges + + + + line2305 + col3 + file0 + + + line2305 + col16 + file0 + + + + depth0 + extended_message + 'arr' initialized here + message + 'arr' initialized here + kindcontrol edges diff --git a/clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist b/clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist --- a/clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist +++ b/clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist @@ -25282,6 +25282,35 @@ message Call to function 'CFCreateSomething' returns a Core Foundation object of type 'CFTypeRef' with a +1 retain count + + kindevent + location + + line2285 + col3 + file0 + + ranges + + + + line2285 + col3 + file0 + + + line2285 + col15 + file0 + + + + depth0 + extended_message + 'obj' initialized here + message + 'obj' initialized here + kindcontrol edges @@ -25569,6 +25598,35 @@ message Call to function 'CFArrayCreateMutable' returns a Core Foundation object of type 'CFMutableArrayRef' with a +1 retain count + + kindevent + location + + line2305 + col3 + file0 + + ranges + + + + line2305 + col3 + file0 + + + line2305 + col16 + file0 + + + + depth0 + extended_message + 'arr' initialized here + message + 'arr' initialized here + kindcontrol edges diff --git a/clang/test/Analysis/osobject-retain-release.cpp b/clang/test/Analysis/osobject-retain-release.cpp --- a/clang/test/Analysis/osobject-retain-release.cpp +++ b/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}}