[clang][analyzer] Clear ObjCMethodCall's cache between runs

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,

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,

clang/unittests/StaticAnalyzer/CallEventTest.cpp

@@ -84,6 +84,47 @@ 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