Skip to content

Commit f4af60d

Browse files
authored
[analyzer] Fix false double free when including 3rd-party headers with overloaded delete operator as system headers (llvm#85224)
Fixes llvm#62985 Fixes llvm#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.
1 parent 2aed0d9 commit f4af60d

File tree

3 files changed

+32
-2
lines changed

3 files changed

+32
-2
lines changed

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,12 +1091,15 @@ static bool isStandardDelete(const FunctionDecl *FD) {
10911091
if (Kind != OO_Delete && Kind != OO_Array_Delete)
10921092
return false;
10931093

1094+
bool HasBody = FD->hasBody(); // Prefer using the definition.
1095+
10941096
// This is standard if and only if it's not defined in a user file.
10951097
SourceLocation L = FD->getLocation();
1098+
10961099
// If the header for operator delete is not included, it's still defined
10971100
// in an invalid source location. Check to make sure we don't crash.
1098-
return !L.isValid() ||
1099-
FD->getASTContext().getSourceManager().isInSystemHeader(L);
1101+
const auto &SM = FD->getASTContext().getSourceManager();
1102+
return L.isInvalid() || (!HasBody && SM.isInSystemHeader(L));
11001103
}
11011104

11021105
//===----------------------------------------------------------------------===//
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#ifndef OVERLOADED_DELETE_IN_HEADER
2+
#define OVERLOADED_DELETE_IN_HEADER
3+
4+
struct DeleteInHeader {
5+
int data;
6+
static void operator delete(void *ptr);
7+
};
8+
9+
void DeleteInHeader::operator delete(void *ptr) {
10+
DeleteInHeader *self = (DeleteInHeader *)ptr;
11+
self->data = 1; // no-warning: Still alive.
12+
13+
::operator delete(ptr);
14+
15+
self->data = 2; // expected-warning {{Use of memory after it is freed [cplusplus.NewDelete]}}
16+
}
17+
18+
#endif // OVERLOADED_DELETE_IN_SYSTEM_HEADER
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// RUN: %clang_analyze_cc1 -isystem %S/Inputs/ -verify %s \
2+
// RUN: -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete
3+
4+
// RUN: %clang_analyze_cc1 -I %S/Inputs/ -verify %s \
5+
// RUN: -analyzer-checker=core,unix.Malloc,cplusplus.NewDelete
6+
7+
#include "overloaded-delete-in-header.h"
8+
9+
void deleteInHeader(DeleteInHeader *p) { delete p; }

0 commit comments

Comments
 (0)