diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -493,8 +493,8 @@ "that neither deallocated it, or have taken responsibility " "of the ownership are noted, similarly to " "NoStoreFuncVisitor.", - "false", - InAlpha, + "true", + Released, Hide> ]>, Dependencies<[CStringModeling]>, diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -52,6 +52,8 @@ #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/ParentMap.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Analysis/ProgramPoint.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceManager.h" @@ -791,9 +793,28 @@ return ""; } + bool doesFnIntendToHandleOwnership(const Decl *Callee, ASTContext &ACtx) { + using namespace clang::ast_matchers; + const FunctionDecl *FD = dyn_cast(Callee); + if (!FD) + return false; + // TODO: Operator delete is hardly the only deallocator -- Can we reuse + // isFreeingCall() or something thats already here? + auto Deallocations = match( + stmt(hasDescendant(cxxDeleteExpr().bind("delete")) + ), *FD->getBody(), ACtx); + // TODO: Ownership my change with an attempt to store the allocated memory. + return !Deallocations.empty(); + } + virtual bool wasModifiedInFunction(const ExplodedNode *CallEnterN, const ExplodedNode *CallExitEndN) override { + if (!doesFnIntendToHandleOwnership( + CallExitEndN->getFirstPred()->getLocationContext()->getDecl(), + CallExitEndN->getState()->getAnalysisManager().getASTContext())) + return true; + if (CallEnterN->getState()->get(Sym) != CallExitEndN->getState()->get(Sym)) return true; diff --git a/clang/test/Analysis/NewDeleteLeaks.cpp b/clang/test/Analysis/NewDeleteLeaks.cpp --- a/clang/test/Analysis/NewDeleteLeaks.cpp +++ b/clang/test/Analysis/NewDeleteLeaks.cpp @@ -20,15 +20,16 @@ bool coin(); +// TODO: AST analysis of sink would reveal that it doesn't intent to free the +// allocated memory, but in this instance, its also the only function with +// the ability to do so, we should see a note here. namespace memory_allocated_in_fn_call { void sink(int *P) { -} // ownership-note {{Returning without deallocating memory or storing the pointer for later deallocation}} +} void foo() { sink(new int(5)); // expected-note {{Memory is allocated}} - // ownership-note@-1 {{Calling 'sink'}} - // ownership-note@-2 {{Returning from 'sink'}} } // expected-warning {{Potential memory leak [cplusplus.NewDeleteLeaks]}} // expected-note@-1 {{Potential memory leak}} @@ -109,17 +110,14 @@ } // namespace memory_shared_with_ptr_of_same_lifetime -// TODO: We don't want a note here. sink() doesn't seem like a function that -// even attempts to take care of any memory ownership problems. namespace memory_passed_into_fn_that_doesnt_intend_to_free { void sink(int *P) { -} // ownership-note {{Returning without deallocating memory or storing the pointer for later deallocation}} +} void foo() { int *ptr = new int(5); // expected-note {{Memory is allocated}} - sink(ptr); // ownership-note {{Calling 'sink'}} - // ownership-note@-1 {{Returning from 'sink'}} + sink(ptr); } // expected-warning {{Potential leak of memory pointed to by 'ptr' [cplusplus.NewDeleteLeaks]}} // expected-note@-1 {{Potential leak}} diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -117,7 +117,7 @@ // CHECK-NEXT: suppress-null-return-paths = true // CHECK-NEXT: track-conditions = true // CHECK-NEXT: track-conditions-debug = false -// CHECK-NEXT: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes = false +// CHECK-NEXT: unix.DynamicMemoryModeling:AddNoOwnershipChangeNotes = true // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false // CHECK-NEXT: unroll-loops = false // CHECK-NEXT: verbose-report-filename = false