[PATCH] D152504: [clang][ThreadSafety] Analyze cleanup functions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: NoQ, aaron.ballman, krememek, dergachev.a, MikeStump.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When a declaration has a cleanup function attached, add it to the CFG. This is 
basically a destructor on a per-declaration basis.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152504

Files:
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-thread-safety-analysis.c


Index: clang/test/Sema/warn-thread-safety-analysis.c
===================================================================
--- clang/test/Sema/warn-thread-safety-analysis.c
+++ clang/test/Sema/warn-thread-safety-analysis.c
@@ -22,6 +22,7 @@
 #define SHARED_LOCKS_REQUIRED(...) \
   __attribute__ ((shared_locks_required(__VA_ARGS__)))
 #define NO_THREAD_SAFETY_ANALYSIS  __attribute__ ((no_thread_safety_analysis))
+#define CLEANUP(A) __attribute__ ((cleanup(A)))
 
 // Define the mutex struct.
 // Simplified only for test purpose.
@@ -72,6 +73,11 @@
   return *p;
 }
 
+void cleanup_int(int *unused) __attribute__((release_capability(mu1))) {
+  (void)unused;
+  mutex_exclusive_unlock(&mu1);
+}
+
 int main(void) {
 
   Foo_fun1(1); // expected-warning{{calling function 'Foo_fun1' requires holding mutex 'mu2'}} \
@@ -127,6 +133,15 @@
                                 // expected-note@-1{{mutex released here}}
   mutex_shared_unlock(&mu1);    // expected-warning {{releasing mutex 'mu1' that was not held}}
 
+
+  {
+    mutex_exclusive_lock(&mu1);
+    int CLEANUP(cleanup_int) i;
+
+    Bar_fun1(3);
+  }
+  Bar_fun1(4); // expected-warning {{calling function 'Bar_fun1' requires holding mutex 'mu1' exclusively}}
+
   return 0;
 }
 
Index: clang/lib/Analysis/CFG.cpp
===================================================================
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -1907,7 +1907,8 @@
     Decls.push_back(*I);
 
   for (VarDecl *VD : llvm::reverse(Decls)) {
-    if (hasTrivialDestructor(VD)) {
+    bool HasCleanupAttr = VD->hasAttr<CleanupAttr>();
+    if (hasTrivialDestructor(VD) && !HasCleanupAttr) {
       // If AddScopes is enabled and *I is a first variable in a scope, add a
       // ScopeEnd marker in a Block.
       if (BuildOpts.AddScopes && DeclsWithEndedScope.count(VD)) {
@@ -1925,7 +1926,8 @@
     }
     Ty = Context->getBaseElementType(Ty);
 
-    if (Ty->getAsCXXRecordDecl()->isAnyDestructorNoReturn())
+    bool IsCXXRecordType = Ty->getAsCXXRecordDecl() != nullptr;
+    if (IsCXXRecordType && Ty->getAsCXXRecordDecl()->isAnyDestructorNoReturn())
       Block = createNoReturnBlock();
     else
       autoCreateBlock();
@@ -1933,7 +1935,28 @@
     // Add ScopeEnd just after automatic obj destructor.
     if (BuildOpts.AddScopes && DeclsWithEndedScope.count(VD))
       appendScopeEnd(Block, VD, S);
-    appendAutomaticObjDtor(Block, VD, S);
+
+    if (HasCleanupAttr) {
+      // Create a fake CallExpr for the cleanup function.
+      const CleanupAttr *CA = VD->getAttr<CleanupAttr>();
+      FunctionDecl *FD = CA->getFunctionDecl();
+      assert(FD);
+      auto DRE =
+          DeclRefExpr::Create(*Context, {}, {}, VD, false, SourceLocation(),
+                              VD->getType(), VK_PRValue);
+
+      auto F = DeclRefExpr::Create(*Context, {}, {}, FD, false,
+                                   SourceLocation(), FD->getType(), VK_LValue);
+
+      SmallVector<Expr *> Args;
+      Args.push_back(DRE);
+      auto A = CallExpr::Create(*Context, F, Args, FD->getType(), VK_PRValue,
+                                {}, FPOptionsOverride());
+      appendCall(Block, A);
+    }
+
+    if (IsCXXRecordType)
+      appendAutomaticObjDtor(Block, VD, S);
   }
 }
 
@@ -2090,7 +2113,8 @@
     return Scope;
 
   if (BuildOpts.AddImplicitDtors) {
-    if (!hasTrivialDestructor(VD) || BuildOpts.AddScopes) {
+    if (!hasTrivialDestructor(VD) || VD->hasAttr<CleanupAttr>() ||
+        BuildOpts.AddScopes) {
       // Add the variable to scope
       Scope = createOrReuseLocalScope(Scope);
       Scope->addVar(VD);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits