https://github.com/Snape3058 created https://github.com/llvm/llvm-project/pull/85224
Fixes #62985 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. >From b3e6a2273e9c35ac4cc3ac16aebcf4ea0d89ef74 Mon Sep 17 00:00:00 2001 From: Ella Ma <alansnape3...@gmail.com> Date: Thu, 14 Mar 2024 20:41:20 +0800 Subject: [PATCH] wip: the first workaround --- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 3 ++- .../Inputs/overloaded-delete-in-header.h | 17 +++++++++++++ .../overloaded-delete-in-system-header.cpp | 25 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 clang/test/Analysis/Inputs/overloaded-delete-in-header.h create mode 100644 clang/test/Analysis/overloaded-delete-in-system-header.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index 03cb7696707fe2..c7ec2b7cc43b30 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp @@ -1090,7 +1090,8 @@ static bool isStandardNewDelete(const FunctionDecl *FD) { // 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); + (!FD->hasBody() && // FIXME: Still a false alarm after CTU inlining. + FD->getASTContext().getSourceManager().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..5090de0d9bbd6b --- /dev/null +++ b/clang/test/Analysis/Inputs/overloaded-delete-in-header.h @@ -0,0 +1,17 @@ +#ifndef OVERLOADED_DELETE_IN_HEADER +#define OVERLOADED_DELETE_IN_HEADER + +void clang_analyzer_printState(); + +struct DeleteInHeader { + inline void operator delete(void *ptr) { + // No matter whether this header file is included as a system header file + // with -isystem or a user header file with -I, ptr should not be marked as + // released. + clang_analyzer_printState(); + + ::operator delete(ptr); // The first place where ptr is marked as released. + } +}; + +#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..f7780b67e93b99 --- /dev/null +++ b/clang/test/Analysis/overloaded-delete-in-system-header.cpp @@ -0,0 +1,25 @@ +// issue 62985 +// 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, which leads to a false uaf alarm. +// +// The first run, include as system header. False uaf report before fix. +// +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=core,cplusplus.NewDelete,debug.ExprInspection \ +// RUN: -isystem %S/Inputs/ 2>&1 | \ +// RUN: FileCheck %s +// +// The second run, include as user header. Should always silent. +// +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=core,cplusplus.NewDelete,debug.ExprInspection \ +// RUN: -I %S/Inputs/ 2>&1 | \ +// RUN: FileCheck %s + +#include "overloaded-delete-in-header.h" + +void deleteInHeader(DeleteInHeader *p) { delete p; } + +// CHECK-NOT: Released _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits