Author: Ella Ma Date: 2024-10-31T17:02:28+01:00 New Revision: f4af60dfbb6a2e3d5628b8f07b4895ddbe24d459
URL: https://github.com/llvm/llvm-project/commit/f4af60dfbb6a2e3d5628b8f07b4895ddbe24d459 DIFF: https://github.com/llvm/llvm-project/commit/f4af60dfbb6a2e3d5628b8f07b4895ddbe24d459.diff LOG: [analyzer] Fix false double free when including 3rd-party headers with overloaded delete operator as system headers (#85224) Fixes #62985 Fixes #58820 When 3rd-party header files are included as system headers, their overloaded `new` and `delete` operators are also considered as the std ones. However, those overloaded operator functions will also be inlined. This makes the same symbolic memory marked as released twice: during `checkPreCall` of the overloaded `delete` operator and when calling `::operator delete` after inlining the overloaded operator function (if it has). This patch attempts to fix this bug by adjusting the strategy of verifying whether the callee is a standard `new` or `delete` operator in the `isStandardNewDelete` function. Added: clang/test/Analysis/Inputs/overloaded-delete-in-header.h clang/test/Analysis/overloaded-delete-in-system-header.cpp Modified: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 3e95db7e97fac8..4166cf14391e2d 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1091,12 +1091,15 @@ static bool isStandardDelete(const FunctionDecl *FD) { if (Kind != OO_Delete && Kind != OO_Array_Delete) return false; + bool HasBody = FD->hasBody(); // Prefer using the definition. + // This is standard if and only if it's not defined in a user file. SourceLocation L = FD->getLocation(); + // If the header for operator delete is not included, it's still defined // in an invalid source location. Check to make sure we don't crash. - return !L.isValid() || - FD->getASTContext().getSourceManager().isInSystemHeader(L); + const auto &SM = FD->getASTContext().getSourceManager(); + return L.isInvalid() || (!HasBody && SM.isInSystemHeader(L)); } //===----------------------------------------------------------------------===// diff --git a/clang/test/Analysis/Inputs/overloaded-delete-in-header.h b/clang/test/Analysis/Inputs/overloaded-delete-in-header.h new file mode 100644 index 00000000000000..8243961d84830b --- /dev/null +++ b/clang/test/Analysis/Inputs/overloaded-delete-in-header.h @@ -0,0 +1,18 @@ +#ifndef OVERLOADED_DELETE_IN_HEADER +#define OVERLOADED_DELETE_IN_HEADER + +struct DeleteInHeader { + int data; + static void operator delete(void *ptr); +}; + +void DeleteInHeader::operator delete(void *ptr) { + DeleteInHeader *self = (DeleteInHeader *)ptr; + self->data = 1; // no-warning: Still alive. + + ::operator delete(ptr); + + self->data = 2; // expected-warning {{Use of memory after it is freed [cplusplus.NewDelete]}} +} + +#endif // OVERLOADED_DELETE_IN_SYSTEM_HEADER diff --git a/clang/test/Analysis/overloaded-delete-in-system-header.cpp b/clang/test/Analysis/overloaded-delete-in-system-header.cpp new file mode 100644 index 00000000000000..c284a942063061 --- /dev/null +++ b/clang/test/Analysis/overloaded-delete-in-system-header.cpp @@ -0,0 +1,9 @@ +// RUN: %clang_analyze_cc1 -isystem %S/Inputs/ -verify %s \ +// RUN: -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete + +// RUN: %clang_analyze_cc1 -I %S/Inputs/ -verify %s \ +// RUN: -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete + +#include "overloaded-delete-in-header.h" + +void deleteInHeader(DeleteInHeader *p) { delete p; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
