Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp @@ -113,13 +113,11 @@ return true; } -static void generateDiagnosticsForCallLike( - ProgramStateRef CurrSt, - const LocationContext *LCtx, - const RefVal &CurrV, - SymbolRef &Sym, - const Stmt *S, - llvm::raw_string_ostream &os) { +static void generateDiagnosticsForCallLike(ProgramStateRef CurrSt, + const LocationContext *LCtx, + const RefVal &CurrV, SymbolRef &Sym, + const Stmt *S, + llvm::raw_string_ostream &os) { if (const CallExpr *CE = dyn_cast(S)) { // Get the name of the callee (if it is available) // from the tracked SVal. @@ -137,8 +135,8 @@ } else { os << "function call"; } - } else if (isa(S)){ - os << "Operator new"; + } else if (isa(S)) { + os << "Operator 'new'"; } else { assert(isa(S)); CallEventManager &Mgr = CurrSt->getStateManager().getCallEventManager(); @@ -230,9 +228,94 @@ } // end namespace ento } // end namespace clang + +/// Find teh first node with the parent stack frame. +static const ExplodedNode *getCalleeNode(const ExplodedNode *Pred) { + const StackFrameContext *SC = Pred->getStackFrame(); + const StackFrameContext *PC = + SC->getParent() ? SC->getParent()->getStackFrame() : nullptr; + if (!PC) + return nullptr; + + const ExplodedNode *N = Pred; + while (N && N->getStackFrame() != PC) { + N = N->getFirstPred(); + } + return N; +} + + +/// Insert a diagnostic piece at function exit +/// if a function parameter is annotated as "os_consumed", +/// but it does not actually consume the reference. +static std::shared_ptr +annotateConsumedSummaryMismatch(const ExplodedNode *N, + CallExitBegin &CallExitLoc, + const SourceManager &SM, + CallEventManager &CEMgr) { + + const ExplodedNode *CN = getCalleeNode(N); + if (!CN) + return nullptr; + + CallEventRef<> Call = CEMgr.getCaller(N->getStackFrame(), N->getState()); + + std::string sbuf; + llvm::raw_string_ostream os(sbuf); + ArrayRef Parameters = Call->parameters(); + for (unsigned I=0; I < Call->getNumArgs() && I < Parameters.size(); ++I) { + const ParmVarDecl *PVD = Parameters[I]; + SVal S = Call->getArgSVal(I); + + if (!PVD->hasAttr()) + return nullptr; + + if (SymbolRef SR = S.getAsLocSymbol()) { + const RefVal *CountBeforeCall = getRefBinding(CN->getState(), SR); + const RefVal *CountAtExit = getRefBinding(N->getState(), SR); + + if (!CountBeforeCall || !CountAtExit) + continue; + + unsigned CountBefore = CountBeforeCall->getCount(); + unsigned CountAfter = CountAtExit->getCount(); + + bool AsExpected = CountBefore > 0 && CountAfter == CountBefore - 1; + if (!AsExpected) { + os << "Parameter '"; + PVD->getNameForDiagnostic(os, PVD->getASTContext().getPrintingPolicy(), + /*Qualified=*/false); + os << "' is marked as consuming, but the function does not consume " + << "the reference\n"; + } + } + } + + if (os.str().empty()) + return nullptr; + + // FIXME: remove the code duplication with NoStoreFuncVisitor. + PathDiagnosticLocation L; + if (const ReturnStmt *RS = CallExitLoc.getReturnStmt()) { + L = PathDiagnosticLocation::createBegin(RS, SM, N->getLocationContext()); + } else { + L = PathDiagnosticLocation( + Call->getRuntimeDefinition().getDecl()->getSourceRange().getEnd(), SM); + } + + return std::make_shared(L, os.str()); +} + std::shared_ptr CFRefReportVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC, BugReport &BR) { + const SourceManager &SM = BRC.getSourceManager(); + CallEventManager &CEMgr = BRC.getStateManager().getCallEventManager(); + if (auto CE = N->getLocationAs()) { + if (auto PD = annotateConsumedSummaryMismatch(N, *CE, SM, CEMgr)) + return PD; + } + // FIXME: We will eventually need to handle non-statement-based events // (__attribute__((cleanup))). if (!N->getLocation().getAs()) @@ -293,15 +376,13 @@ generateDiagnosticsForCallLike(CurrSt, LCtx, CurrV, Sym, S, os); } - PathDiagnosticLocation Pos(S, BRC.getSourceManager(), - N->getLocationContext()); + PathDiagnosticLocation Pos(S, SM, N->getLocationContext()); return std::make_shared(Pos, os.str()); } // Gather up the effects that were performed on the object at this // program point SmallVector AEffects; - const ExplodedNode *OrigNode = BRC.getNodeResolver().getOriginalNode(N); if (const RetainSummary *Summ = SummaryLog.lookup(OrigNode)) { // We only have summaries attached to nodes after evaluating CallExpr and Index: clang/test/Analysis/osobject-retain-release.cpp =================================================================== --- clang/test/Analysis/osobject-retain-release.cpp +++ clang/test/Analysis/osobject-retain-release.cpp @@ -90,6 +90,32 @@ }; void escape(void *); +bool coin(); + +bool os_consume_violation(OS_CONSUME OSObject *obj) { + if (coin()) { // expected-note{{Assuming the condition is false}} + // expected-note@-1{{Taking false branch}} + escape(obj); + return true; + } + return false; // expected-note{{Parameter 'obj' is marked as consuming, but the function does not consume the reference}} +} + +void os_consume_ok(OS_CONSUME OSObject *obj) { + escape(obj); +} + +void use_os_consume_violation() { + OSObject *obj = new OSObject; // expected-note{{Operator 'new' returns an OSObject of type OSObject with a +1 retain count}} + os_consume_violation(obj); // expected-note{{Calling 'os_consume_violation'}} + // expected-note@-1{{Returning from 'os_consume_violation'}} +} // expected-note{{Object leaked: object allocated and stored into 'obj' is not referenced later in this execution path and has a retain count of +1}} + // expected-warning@-1{{Potential leak of an object stored into 'obj'}} + +void use_os_consume_ok() { + OSObject *obj = new OSObject; + os_consume_ok(obj); +} void test_escaping_into_voidstar() { OSObject *obj = new OSObject; @@ -152,7 +178,7 @@ } unsigned int check_leak_explicit_new() { - OSArray *arr = new OSArray; // expected-note{{Operator new returns an OSObject of type OSArray with a +1 retain count}} + OSArray *arr = new OSArray; // expected-note{{Operator 'new' returns an OSObject of type OSArray with a +1 retain count}} return arr->getCount(); // expected-note{{Object leaked: object allocated and stored into 'arr' is not referenced later in this execution path and has a retain count of +1}} // expected-warning@-1{{Potential leak of an object stored into 'arr'}} }