[clang analysis] Make mutex guard detection more reliable.

[efriedma]

Summary

-Wthread-safety was failing to detect certain AST patterns it should detect. Make the pattern detection a bit more comprehensive.

Due to an unrelated bug involving template instantiation, this showed up as a regression in 10.0 vs. 9.0 in the original bug report. The included testcase fails on older versions of clang, though.

Fixes https://bugs.llvm.org/show_bug.cgi?id=45323 .

[rsmith]

clang/lib/Analysis/ThreadSafety.cpp

2144–2145

Should we also skip over CK_UserDefinedConversion here (for conversion functions that return mutex locks, I suppose)?

[aaronpuchert]

clang/lib/Analysis/ThreadSafety.cpp

2144–2145

Seems reasonable, there could be a CXXMemberCallExpr to an annotated function below that.

[efriedma]

Add support for conversion operators.

[aaron.ballman]

LGTM!

[aaronpuchert]

clang/test/SemaCXX/warn-thread-safety-analysis.cpp

5659

Honestly I'd prefer the annotation to be on this operator instead of the move constructor, but that's an independent issue that existed before this change. (rC322316#294681)

clang/lib/Analysis/ThreadSafety.cpp

for (auto *D : S->getDeclGroup()) {
     if (auto *VD = dyn_cast_or_null<VarDecl>(D)) {
       Expr *E = VD->getInit();
       if (!E)
         continue;
       E = E->IgnoreParens();
 
       // handle constructors that involve temporaries
       if (auto *EWC = dyn_cast<ExprWithCleanups>(E))
-        E = EWC->getSubExpr();
-      if (auto *ICE = dyn_cast<ImplicitCastExpr>(E))
-        if (ICE->getCastKind() == CK_NoOp)
-          E = ICE->getSubExpr();
+        E = EWC->getSubExpr()->IgnoreParens();
+      if (auto *CE = dyn_cast<CastExpr>(E))
+        if (CE->getCastKind() == CK_NoOp ||
+            CE->getCastKind() == CK_ConstructorConversion ||
+            CE->getCastKind() == CK_UserDefinedConversion)
+          E = CE->getSubExpr()->IgnoreParens();
       if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
-        E = BTE->getSubExpr();
+        E = BTE->getSubExpr()->IgnoreParens();
 
       if (const auto *CE = dyn_cast<CXXConstructExpr>(E)) {
         const auto *CtorD = dyn_cast_or_null<NamedDecl>(CE->getConstructor());
         if (!CtorD || !CtorD->hasAttrs())
           continue;
         handleCall(E, CtorD, VD);
       } else if (isa<CallExpr>(E) && E->isRValue()) {
         // If the object is initialized by a function call that returns a

clang/test/SemaCXX/warn-thread-safety-analysis.cpp

struct Object {
     Mutex mutex;
   };
 
   ReadLockedPtr<Object> get();
   int use() {
     auto ptr = get();
     return ptr->f();
   }
+  void use_constructor() {
+    auto ptr = ReadLockedPtr<Object>(nullptr);
+    ptr->f();
+    auto ptr2 = ReadLockedPtr<Object>{nullptr};
+    ptr2->f();
+    auto ptr3 = (ReadLockedPtr<Object>{nullptr});
+    ptr3->f();
+  }
+  struct Convertible {
+    Convertible();
+    operator ReadLockedPtr<Object>();
+  };
+  void use_conversion() {
+    ReadLockedPtr<Object> ptr = Convertible();
+    ptr->f();
+  }
 }
 
 namespace PR38640 {
 void f() {
   // Self-referencing assignment previously caused an infinite loop when thread
   // safety analysis was enabled.
   int &i = i; // expected-warning {{reference 'i' is not yet bound to a value when used within its own initialization}}
 }