r356940 - Thread Safety: also look at ObjC methods

[JF Bastien via cfe-commits]

Author: jfb
Date: Mon Mar 25 13:06:32 2019
New Revision: 356940

URL: http://llvm.org/viewvc/llvm-project?rev=356940&view=rev
Log:
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

Added:
    cfe/trunk/test/SemaObjCXX/no-crash-thread-safety-analysis.mm
    cfe/trunk/test/SemaObjCXX/thread-safety-analysis.h
Modified:
    cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
    cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm

Modified: cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp?rev=356940&r1=356939&r2=356940&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp Mon Mar 25 13:06:32 2019
@@ -276,18 +276,23 @@ til::SExpr *SExprBuilder::translateDeclR
 
   // 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.

Added: cfe/trunk/test/SemaObjCXX/no-crash-thread-safety-analysis.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/no-crash-thread-safety-analysis.mm?rev=356940&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjCXX/no-crash-thread-safety-analysis.mm (added)
+++ cfe/trunk/test/SemaObjCXX/no-crash-thread-safety-analysis.mm Mon Mar 25 13:06:32 2019
@@ -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"
+
+ at interface MyInterface
+- (void)doIt:(class Lock *)myLock;
+ at end
+
+ at implementation MyInterface
+- (void)doIt:(class Lock *)myLock {
+  AutoLock lock(*myLock);
+}
+ at end

Added: cfe/trunk/test/SemaObjCXX/thread-safety-analysis.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/thread-safety-analysis.h?rev=356940&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjCXX/thread-safety-analysis.h (added)
+++ cfe/trunk/test/SemaObjCXX/thread-safety-analysis.h Mon Mar 25 13:06:32 2019
@@ -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_;
+};

Modified: cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm?rev=356940&r1=356939&r2=356940&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm (original)
+++ cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm Mon Mar 25 13:06:32 2019
@@ -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