[PATCH] D59523: Thread Safety: also look at ObjC methods

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
jfb added reviewers: dexonsmith, erik.pilkington.
Herald added subscribers: cfe-commits, jdoerfert, jkorous.
Herald added a project: clang.
jfb added a comment.

It seems weird how both do pretty similar things and e.g. duplicate `getParamDecl`.

The repro I have is 20M, creduce chokes on it, and manually reducing failed to repro... ☹️


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


Repository:
  rC Clang

https://reviews.llvm.org/D59523

Files:
  lib/Analysis/ThreadSafetyCommon.cpp


Index: lib/Analysis/ThreadSafetyCommon.cpp
===================================================================
--- lib/Analysis/ThreadSafetyCommon.cpp
+++ lib/Analysis/ThreadSafetyCommon.cpp
@@ -276,18 +276,31 @@
 
   // 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);
+    if (isa<FunctionDecl>(PV->getDeclContext())) {
+      unsigned I = PV->getFunctionScopeIndex();
+      const auto *FD =
+          cast<FunctionDecl>(PV->getDeclContext())->getCanonicalDecl();
+      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);
+      }
+      // Map the param back to the param of the original function declaration
+      // for consistent comparisons.
+      VD = FD->getParamDecl(I);
+    } else {
+      unsigned I = PV->getFunctionScopeIndex();
+      const auto *FD =
+          cast<ObjCMethodDecl>(PV->getDeclContext())->getCanonicalDecl();
+      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);
+      }
+      // Map the param back to the param of the original function declaration
+      // for consistent comparisons.
+      VD = FD->getParamDecl(I);
     }
-    // Map the param back to the param of the original function declaration
-    // for consistent comparisons.
-    VD = FD->getParamDecl(I);
   }
 
   // For non-local variables, treat it as a reference to a named object.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59523.191212.patch
Type: text/x-patch
Size: 2113 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190319/bb15729b/attachment-0001.bin>