Author: Donát Nagy Date: 2025-07-09T17:04:28+02:00 New Revision: cd193f4c057ee5005197219df1c646b939a85711
URL: https://github.com/llvm/llvm-project/commit/cd193f4c057ee5005197219df1c646b939a85711 DIFF: https://github.com/llvm/llvm-project/commit/cd193f4c057ee5005197219df1c646b939a85711.diff LOG: [analyzer] Remove redundant bug type DoubleDelete (#147542) This commit removes the DoubleDelete bug type from `MallocChecker.cpp` because it's completely redundant with the `DoubleFree` bug (which is already used for all allocator families, including new/delete). This simplifies the code of the checker and prevents the potential confusion caused by two semantically equivalent and very similar, but not identical bug report messages. Added: Modified: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp clang/test/Analysis/NewDelete-checker-test.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 0a58d7cc2635a..30a04977d906d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -346,10 +346,6 @@ namespace { }; BUGTYPE_PROVIDER(DoubleFree, "Double free") -// TODO: Remove DoubleDelete as a separate bug type and when it would be -// emitted, emit DoubleFree reports instead. (Note that DoubleFree is already -// used for all allocation families, not just malloc/free.) -BUGTYPE_PROVIDER(DoubleDelete, "Double delete") struct Leak : virtual public CheckerFrontend { // Leaks should not be reported if they are post-dominated by a sink: @@ -410,8 +406,7 @@ class MallocChecker DynMemFrontend<DoubleFree, Leak, UseFree, BadFree, FreeAlloca, OffsetFree, UseZeroAllocated> MallocChecker; - DynMemFrontend<DoubleFree, DoubleDelete, UseFree, BadFree, OffsetFree, - UseZeroAllocated> + DynMemFrontend<DoubleFree, UseFree, BadFree, OffsetFree, UseZeroAllocated> NewDeleteChecker; DynMemFrontend<Leak> NewDeleteLeaksChecker; DynMemFrontend<FreeAlloca, MismatchedDealloc> MismatchedDeallocatorChecker; @@ -784,9 +779,6 @@ class MallocChecker void checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C, const Stmt *S) const; - /// If in \p S \p Sym is being freed, check whether \p Sym was already freed. - bool checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const; - /// Check if the function is known to free memory, or if it is /// "interesting" and should be modeled explicitly. /// @@ -848,8 +840,6 @@ class MallocChecker void HandleDoubleFree(CheckerContext &C, SourceRange Range, bool Released, SymbolRef Sym, SymbolRef PrevSym) const; - void HandleDoubleDelete(CheckerContext &C, SymbolRef Sym) const; - void HandleUseZeroAlloc(CheckerContext &C, SourceRange Range, SymbolRef Sym) const; @@ -2737,7 +2727,8 @@ void MallocChecker::HandleDoubleFree(CheckerContext &C, SourceRange Range, (Released ? "Attempt to free released memory" : "Attempt to free non-owned memory"), N); - R->addRange(Range); + if (Range.isValid()) + R->addRange(Range); R->markInteresting(Sym); if (PrevSym) R->markInteresting(PrevSym); @@ -2746,26 +2737,6 @@ void MallocChecker::HandleDoubleFree(CheckerContext &C, SourceRange Range, } } -void MallocChecker::HandleDoubleDelete(CheckerContext &C, SymbolRef Sym) const { - const DoubleDelete *Frontend = getRelevantFrontendAs<DoubleDelete>(C, Sym); - if (!Frontend) - return; - if (!Frontend->isEnabled()) { - C.addSink(); - return; - } - - if (ExplodedNode *N = C.generateErrorNode()) { - - auto R = std::make_unique<PathSensitiveBugReport>( - Frontend->DoubleDeleteBug, "Attempt to delete released memory", N); - - R->markInteresting(Sym); - R->addVisitor<MallocBugVisitor>(Sym); - C.emitReport(std::move(R)); - } -} - void MallocChecker::HandleUseZeroAlloc(CheckerContext &C, SourceRange Range, SymbolRef Sym) const { const UseZeroAllocated *Frontend = @@ -3136,10 +3107,22 @@ void MallocChecker::checkPreCall(const CallEvent &Call, return; } + // If we see a `CXXDestructorCall` (that is, an _implicit_ destructor call) + // to a region that's symbolic and known to be already freed, then it must be + // implicitly triggered by a `delete` expression. In this situation we should + // emit a `DoubleFree` report _now_ (before entering the call to the + // destructor) because otherwise the destructor call can trigger a + // use-after-free bug (by accessing any member variable) and that would be + // (technically valid, but) less user-friendly report than the `DoubleFree`. if (const auto *DC = dyn_cast<CXXDestructorCall>(&Call)) { SymbolRef Sym = DC->getCXXThisVal().getAsSymbol(); - if (!Sym || checkDoubleDelete(Sym, C)) + if (!Sym) return; + if (isReleased(Sym, C)) { + HandleDoubleFree(C, SourceRange(), /*Released=*/true, Sym, + /*PrevSym=*/nullptr); + return; + } } // We need to handle getline pre-conditions here before the pointed region @@ -3321,15 +3304,6 @@ void MallocChecker::checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C, } } -bool MallocChecker::checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const { - - if (isReleased(Sym, C)) { - HandleDoubleDelete(C, Sym); - return true; - } - return false; -} - // Check if the location is a freed symbolic region. void MallocChecker::checkLocation(SVal l, bool isLoad, const Stmt *S, CheckerContext &C) const { diff --git a/clang/test/Analysis/NewDelete-checker-test.cpp b/clang/test/Analysis/NewDelete-checker-test.cpp index da0eef7c52bd8..7c3e142d586bb 100644 --- a/clang/test/Analysis/NewDelete-checker-test.cpp +++ b/clang/test/Analysis/NewDelete-checker-test.cpp @@ -412,7 +412,7 @@ class DerefClass{ void testDoubleDeleteClassInstance() { DerefClass *foo = new DerefClass(); delete foo; - delete foo; // newdelete-warning {{Attempt to delete released memory}} + delete foo; // newdelete-warning {{Attempt to free released memory}} } class EmptyClass{ @@ -424,7 +424,7 @@ class EmptyClass{ void testDoubleDeleteEmptyClass() { EmptyClass *foo = new EmptyClass(); delete foo; - delete foo; // newdelete-warning {{Attempt to delete released memory}} + delete foo; // newdelete-warning {{Attempt to free released memory}} } struct Base { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
