llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Donát Nagy (NagyDonat) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/147542.diff 2 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (+4-29) - (modified) clang/test/Analysis/NewDelete-checker-test.cpp (+2-2) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 0a58d7cc2635a..be8f89853bc96 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,7 +406,7 @@ class MallocChecker DynMemFrontend<DoubleFree, Leak, UseFree, BadFree, FreeAlloca, OffsetFree, UseZeroAllocated> MallocChecker; - DynMemFrontend<DoubleFree, DoubleDelete, UseFree, BadFree, OffsetFree, + DynMemFrontend<DoubleFree, UseFree, BadFree, OffsetFree, UseZeroAllocated> NewDeleteChecker; DynMemFrontend<Leak> NewDeleteLeaksChecker; @@ -848,8 +844,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 +2731,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 +2741,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 = @@ -3324,7 +3299,7 @@ void MallocChecker::checkUseZeroAllocated(SymbolRef Sym, CheckerContext &C, bool MallocChecker::checkDoubleDelete(SymbolRef Sym, CheckerContext &C) const { if (isReleased(Sym, C)) { - HandleDoubleDelete(C, Sym); + HandleDoubleFree(C, SourceRange(), /*Released=*/true, Sym, nullptr); return true; } return false; 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 { `````````` </details> https://github.com/llvm/llvm-project/pull/147542 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits