[clang] [clang][analyzer] Clear `ObjCMethodCall`'s cache between runs (PR #161327)
Alejandro Álvarez Ayllón via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 30 00:21:26 PDT 2025
https://github.com/alejandro-alvarez-sonarsource updated https://github.com/llvm/llvm-project/pull/161327
>From b7a56b629883654252e2eed28d708b343cb627cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Wed, 13 Dec 2023 10:35:39 +0100
Subject: [PATCH 1/2] [clang] Clear ObjCMethodCall's cache between runs
`lookupRuntimeDefinition` assumed that a process would handle only one
TU. This is not true for unit tests, for instance. Multiple snippets of
code would get parsed, and their AST would be unloaded each time.
Since the cache relies on pointers as keys, if the same address happens
to be reused between runs, the cache would return a stale pointer,
potentially causing a segmentation fault. This is not that unlikely if
the snippets are similar, which would trigger similar allocation
patterns.
CPP-4889
---
.../Core/PathSensitive/CallEvent.h | 2 +-
clang/lib/StaticAnalyzer/Core/CallEvent.cpp | 31 +++++++-------
.../StaticAnalyzer/CallEventTest.cpp | 40 +++++++++++++++++++
3 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 5dcf03f7a4648..c233ca1af0256 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1414,7 +1414,7 @@ class CallEventManager {
}
public:
- CallEventManager(llvm::BumpPtrAllocator &alloc) : Alloc(alloc) {}
+ CallEventManager(llvm::BumpPtrAllocator &alloc);
/// Gets an outside caller given a callee context.
CallEventRef<> getCaller(const StackFrameContext *CalleeCtx,
diff --git a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
index 180056cf68b64..06ba01507fa4f 100644
--- a/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -1254,6 +1254,15 @@ template <> struct DenseMapInfo<PrivateMethodKey> {
};
} // end namespace llvm
+// NOTE: This cache is a "global" variable, and it is cleared by
+// CallEventManager's constructor so we do not keep old entries when
+// loading/unloading ASTs. If we are worried about concurrency, we may need to
+// revisit this someday. In terms of memory, this table stays around until clang
+// quits, which also may be bad if we need to release memory.
+using PrivateMethodCacheTy =
+ llvm::DenseMap<PrivateMethodKey, std::optional<const ObjCMethodDecl *>>;
+static PrivateMethodCacheTy PrivateMethodCache;
+
static const ObjCMethodDecl *
lookupRuntimeDefinition(const ObjCInterfaceDecl *Interface,
Selector LookupSelector, bool InstanceMethod) {
@@ -1262,21 +1271,8 @@ lookupRuntimeDefinition(const ObjCInterfaceDecl *Interface,
// that repeated queries on the same ObjCIntefaceDecl and Selector
// don't incur the same cost. On some test cases, we can see the
// same query being issued thousands of times.
- //
- // NOTE: This cache is essentially a "global" variable, but it
- // only gets lazily created when we get here. The value of the
- // cache probably comes from it being global across ExprEngines,
- // where the same queries may get issued. If we are worried about
- // concurrency, or possibly loading/unloading ASTs, etc., we may
- // need to revisit this someday. In terms of memory, this table
- // stays around until clang quits, which also may be bad if we
- // need to release memory.
- using PrivateMethodCache =
- llvm::DenseMap<PrivateMethodKey, std::optional<const ObjCMethodDecl *>>;
-
- static PrivateMethodCache PMC;
std::optional<const ObjCMethodDecl *> &Val =
- PMC[{Interface, LookupSelector, InstanceMethod}];
+ PrivateMethodCache[{Interface, LookupSelector, InstanceMethod}];
// Query lookupPrivateMethod() if the cache does not hit.
if (!Val) {
@@ -1422,6 +1418,13 @@ void ObjCMethodCall::getInitialStackFrameContents(
}
}
+CallEventManager::CallEventManager(llvm::BumpPtrAllocator &alloc)
+ : Alloc(alloc) {
+ // Clear the method cache to avoid hits when multiple AST are loaded/unloaded
+ // within a single process. This can happen with unit tests, for instance.
+ PrivateMethodCache.clear();
+}
+
CallEventRef<>
CallEventManager::getSimpleCall(const CallExpr *CE, ProgramStateRef State,
const LocationContext *LCtx,
diff --git a/clang/unittests/StaticAnalyzer/CallEventTest.cpp b/clang/unittests/StaticAnalyzer/CallEventTest.cpp
index 8b5289ea7472b..9015afdae87fc 100644
--- a/clang/unittests/StaticAnalyzer/CallEventTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallEventTest.cpp
@@ -84,6 +84,46 @@ TEST(CXXDeallocatorCall, SimpleDestructor) {
#endif
}
+TEST(PrivateMethodCache, NeverReturnDanglingPointersWithMultipleASTs) {
+ // Each iteration will load and unload an AST multiple times. Since the code is always the same,
+ // we increase the chance of hitting a bug in the private method cache, returning a dangling pointer and
+ // crashing the process. If the cache is properly cleared between runs, the test should pass.
+ for (int I = 0; I < 100; ++I) {
+ auto const *Code = R"(
+ typedef __typeof(sizeof(int)) size_t;
+
+ extern void *malloc(size_t size);
+ extern void *memcpy(void *dest, const void *src, size_t n);
+
+ @interface SomeMoreData {
+ char const* _buffer;
+ int _size;
+ }
+ @property(nonatomic, readonly) const char* buffer;
+ @property(nonatomic) int size;
+
+ - (void)appendData:(SomeMoreData*)other;
+
+ @end
+
+ @implementation SomeMoreData
+ @synthesize size = _size;
+ @synthesize buffer = _buffer;
+
+ - (void)appendData:(SomeMoreData*)other {
+ int const len = (_size + other.size); // implicit self._length
+ char* d = malloc(sizeof(char) * len);
+ memcpy(d + 20, other.buffer, len);
+ }
+
+ @end
+ )";
+ std::string Diags;
+ EXPECT_TRUE(runCheckerOnCodeWithArgs<addCXXDeallocatorChecker>(
+ Code, {"-x", "objective-c", "-Wno-objc-root-class"}, Diags));
+ }
+}
+
} // namespace
} // namespace ento
} // namespace clang
>From 591542a437fc873affa7129d38413df59670dd36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Tue, 30 Sep 2025 09:21:04 +0200
Subject: [PATCH 2/2] Format
---
clang/unittests/StaticAnalyzer/CallEventTest.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang/unittests/StaticAnalyzer/CallEventTest.cpp b/clang/unittests/StaticAnalyzer/CallEventTest.cpp
index 9015afdae87fc..f42689218bb1a 100644
--- a/clang/unittests/StaticAnalyzer/CallEventTest.cpp
+++ b/clang/unittests/StaticAnalyzer/CallEventTest.cpp
@@ -85,9 +85,10 @@ TEST(CXXDeallocatorCall, SimpleDestructor) {
}
TEST(PrivateMethodCache, NeverReturnDanglingPointersWithMultipleASTs) {
- // Each iteration will load and unload an AST multiple times. Since the code is always the same,
- // we increase the chance of hitting a bug in the private method cache, returning a dangling pointer and
- // crashing the process. If the cache is properly cleared between runs, the test should pass.
+ // Each iteration will load and unload an AST multiple times. Since the code
+ // is always the same, we increase the chance of hitting a bug in the private
+ // method cache, returning a dangling pointer and crashing the process. If the
+ // cache is properly cleared between runs, the test should pass.
for (int I = 0; I < 100; ++I) {
auto const *Code = R"(
typedef __typeof(sizeof(int)) size_t;
More information about the cfe-commits
mailing list