From: JF Bastien <jfbastien@apple.com>
Date: Mon, 25 Mar 2019 20:06:32 +0000 (+0000)
Subject: Thread Safety: also look at ObjC methods
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=96ad72202285f4473a4ae19c5d46e5a959ae24e5;p=clang

Thread Safety: also look at ObjC methods

Summary:
SExprBuilder::translateDeclRefExpr was only looking at FunctionDecl and not also looking at ObjCMethodDecl. It should consider both because the attributes can be used on Objective-C as well.

<rdar://problem/48941331>

Reviewers: dexonsmith, erik.pilkington

Subscribers: jkorous, jdoerfert, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D59523

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@356940 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Analysis/ThreadSafetyCommon.cpp b/lib/Analysis/ThreadSafetyCommon.cpp
index e687d96455..373dfc77fa 100644
--- a/lib/Analysis/ThreadSafetyCommon.cpp
+++ b/lib/Analysis/ThreadSafetyCommon.cpp
@@ -276,18 +276,23 @@ til::SExpr *SExprBuilder::translateDeclRefExpr(const DeclRefExpr *DRE,
 
   // Function parameters require substitution and/or renaming.
   if (const auto *PV = dyn_cast_or_null<ParmVarDecl>(VD)) {
-    const auto *FD =
-        cast<FunctionDecl>(PV->getDeclContext())->getCanonicalDecl();
     unsigned I = PV->getFunctionScopeIndex();
-
-    if (Ctx && Ctx->FunArgs && FD == Ctx->AttrDecl->getCanonicalDecl()) {
-      // Substitute call arguments for references to function parameters
-      assert(I < Ctx->NumArgs);
-      return translate(Ctx->FunArgs[I], Ctx->Prev);
+    const DeclContext *D = PV->getDeclContext();
+    if (Ctx && Ctx->FunArgs) {
+      const Decl *Canonical = Ctx->AttrDecl->getCanonicalDecl();
+      if (isa<FunctionDecl>(D)
+              ? (cast<FunctionDecl>(D)->getCanonicalDecl() == Canonical)
+              : (cast<ObjCMethodDecl>(D)->getCanonicalDecl() == Canonical)) {
+        // Substitute call arguments for references to function parameters
+        assert(I < Ctx->NumArgs);
+        return translate(Ctx->FunArgs[I], Ctx->Prev);
+      }
     }
     // Map the param back to the param of the original function declaration
     // for consistent comparisons.
-    VD = FD->getParamDecl(I);
+    VD = isa<FunctionDecl>(D)
+             ? cast<FunctionDecl>(D)->getCanonicalDecl()->getParamDecl(I)
+             : cast<ObjCMethodDecl>(D)->getCanonicalDecl()->getParamDecl(I);
   }
 
   // For non-local variables, treat it as a reference to a named object.
diff --git a/test/SemaObjCXX/no-crash-thread-safety-analysis.mm b/test/SemaObjCXX/no-crash-thread-safety-analysis.mm
new file mode 100644
index 0000000000..6abd391dc2
--- /dev/null
+++ b/test/SemaObjCXX/no-crash-thread-safety-analysis.mm
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wthread-safety -Wno-objc-root-class %s
+
+// Thread safety analysis used to crash when used with ObjC methods.
+
+#include "thread-safety-analysis.h"
+
+@interface MyInterface
+- (void)doIt:(class Lock *)myLock;
+@end
+
+@implementation MyInterface
+- (void)doIt:(class Lock *)myLock {
+  AutoLock lock(*myLock);
+}
+@end
diff --git a/test/SemaObjCXX/thread-safety-analysis.h b/test/SemaObjCXX/thread-safety-analysis.h
new file mode 100644
index 0000000000..f657b7e50a
--- /dev/null
+++ b/test/SemaObjCXX/thread-safety-analysis.h
@@ -0,0 +1,17 @@
+class __attribute__((lockable)) Lock {
+public:
+  void Acquire() __attribute__((exclusive_lock_function())) {}
+  void Release() __attribute__((unlock_function())) {}
+};
+
+class __attribute__((scoped_lockable)) AutoLock {
+public:
+  AutoLock(Lock &lock) __attribute__((exclusive_lock_function(lock)))
+  : lock_(lock) {
+    lock.Acquire();
+  }
+  ~AutoLock() __attribute__((unlock_function())) { lock_.Release(); }
+
+private:
+  Lock &lock_;
+};
diff --git a/test/SemaObjCXX/warn-thread-safety-analysis.mm b/test/SemaObjCXX/warn-thread-safety-analysis.mm
index 262ab7d108..a50c6f2ee1 100644
--- a/test/SemaObjCXX/warn-thread-safety-analysis.mm
+++ b/test/SemaObjCXX/warn-thread-safety-analysis.mm
@@ -1,22 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta -Wno-objc-root-class %s
 
-class __attribute__((lockable)) Lock {
-public:
-  void Acquire() __attribute__((exclusive_lock_function())) {}
-  void Release() __attribute__((unlock_function())) {}
-};
-
-class __attribute__((scoped_lockable)) AutoLock {
-public:
-  AutoLock(Lock &lock) __attribute__((exclusive_lock_function(lock)))
-  : lock_(lock) {
-    lock.Acquire();
-  }
-  ~AutoLock() __attribute__((unlock_function())) { lock_.Release(); }
-
-private:
-  Lock &lock_;
-};
+#include "thread-safety-analysis.h"
 
 @interface MyInterface {
 @private