Author: rkovacs Date: Sat Jul 7 10:22:45 2018 New Revision: 336489 URL: http://llvm.org/viewvc/llvm-project?rev=336489&view=rev Log: [analyzer] Highlight container object destruction in MallocChecker.
Extend MallocBugVisitor to place a note at the point where objects with AF_InternalBuffer allocation family are destroyed. Differential Revision: https://reviews.llvm.org/D48521 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp cfe/trunk/test/Analysis/dangling-internal-buffer.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=336489&r1=336488&r2=336489&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Sat Jul 7 10:22:45 2018 @@ -480,8 +480,13 @@ private: inline bool isReleased(const RefState *S, const RefState *SPrev, const Stmt *Stmt) { // Did not track -> released. Other state (allocated) -> released. - return (Stmt && (isa<CallExpr>(Stmt) || isa<CXXDeleteExpr>(Stmt)) && - (S && S->isReleased()) && (!SPrev || !SPrev->isReleased())); + // The statement associated with the release might be missing. + bool IsReleased = (S && S->isReleased()) && + (!SPrev || !SPrev->isReleased()); + assert(!IsReleased || + (Stmt && (isa<CallExpr>(Stmt) || isa<CXXDeleteExpr>(Stmt))) || + (!Stmt && S->getAllocationFamily() == AF_InternalBuffer)); + return IsReleased; } inline bool isRelinquished(const RefState *S, const RefState *SPrev, @@ -2850,8 +2855,17 @@ static bool isReferenceCountingPointerDe std::shared_ptr<PathDiagnosticPiece> MallocChecker::MallocBugVisitor::VisitNode( const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) { + + ProgramStateRef state = N->getState(); + ProgramStateRef statePrev = PrevN->getState(); + + const RefState *RS = state->get<RegionState>(Sym); + const RefState *RSPrev = statePrev->get<RegionState>(Sym); + const Stmt *S = PathDiagnosticLocation::getStmt(N); - if (!S) + // When dealing with containers, we sometimes want to give a note + // even if the statement is missing. + if (!S && (!RS || RS->getAllocationFamily() != AF_InternalBuffer)) return nullptr; const LocationContext *CurrentLC = N->getLocationContext(); @@ -2876,14 +2890,6 @@ std::shared_ptr<PathDiagnosticPiece> Mal } } - ProgramStateRef state = N->getState(); - ProgramStateRef statePrev = PrevN->getState(); - - const RefState *RS = state->get<RegionState>(Sym); - const RefState *RSPrev = statePrev->get<RegionState>(Sym); - if (!RS) - return nullptr; - // FIXME: We will eventually need to handle non-statement-based events // (__attribute__((cleanup))). @@ -2896,7 +2902,22 @@ std::shared_ptr<PathDiagnosticPiece> Mal StackHint = new StackHintGeneratorForSymbol(Sym, "Returned allocated memory"); } else if (isReleased(RS, RSPrev, S)) { - Msg = "Memory is released"; + const auto Family = RS->getAllocationFamily(); + switch(Family) { + case AF_Alloca: + case AF_Malloc: + case AF_CXXNew: + case AF_CXXNewArray: + case AF_IfNameIndex: + Msg = "Memory is released"; + break; + case AF_InternalBuffer: + Msg = "Internal buffer is released because the object was destroyed"; + break; + case AF_None: + default: + llvm_unreachable("Unhandled allocation family!"); + } StackHint = new StackHintGeneratorForSymbol(Sym, "Returning; memory was released"); @@ -2967,8 +2988,19 @@ std::shared_ptr<PathDiagnosticPiece> Mal assert(StackHint); // Generate the extra diagnostic. - PathDiagnosticLocation Pos(S, BRC.getSourceManager(), - N->getLocationContext()); + PathDiagnosticLocation Pos; + if (!S) { + assert(RS->getAllocationFamily() == AF_InternalBuffer); + auto PostImplCall = N->getLocation().getAs<PostImplicitCall>(); + if (!PostImplCall) + return nullptr; + Pos = PathDiagnosticLocation(PostImplCall->getLocation(), + BRC.getSourceManager()); + } else { + Pos = PathDiagnosticLocation(S, BRC.getSourceManager(), + N->getLocationContext()); + } + return std::make_shared<PathDiagnosticEventPiece>(Pos, Msg, true, StackHint); } Modified: cfe/trunk/test/Analysis/dangling-internal-buffer.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dangling-internal-buffer.cpp?rev=336489&r1=336488&r2=336489&view=diff ============================================================================== --- cfe/trunk/test/Analysis/dangling-internal-buffer.cpp (original) +++ cfe/trunk/test/Analysis/dangling-internal-buffer.cpp Sat Jul 7 10:22:45 2018 @@ -26,7 +26,7 @@ void deref_after_scope_char() { { std::string s; c = s.c_str(); - } + } // expected-note {{Internal buffer is released because the object was destroyed}} consume(c); // expected-warning {{Use of memory after it is freed}} // expected-note@-1 {{Use of memory after it is freed}} } @@ -36,7 +36,7 @@ void deref_after_scope_wchar_t() { { std::wstring ws; w = ws.c_str(); - } + } // expected-note {{Internal buffer is released because the object was destroyed}} consume(w); // expected-warning {{Use of memory after it is freed}} // expected-note@-1 {{Use of memory after it is freed}} } @@ -46,7 +46,7 @@ void deref_after_scope_char16_t() { { std::u16string s16; c16 = s16.c_str(); - } + } // expected-note {{Internal buffer is released because the object was destroyed}} consume(c16); // expected-warning {{Use of memory after it is freed}} // expected-note@-1 {{Use of memory after it is freed}} } @@ -56,7 +56,7 @@ void deref_after_scope_char32_t() { { std::u32string s32; c32 = s32.c_str(); - } + } // expected-note {{Internal buffer is released because the object was destroyed}} consume(c32); // expected-warning {{Use of memory after it is freed}} // expected-note@-1 {{Use of memory after it is freed}} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
