aaronpuchert updated this revision to Diff 302388.
aaronpuchert marked 2 inline comments as done.
aaronpuchert added a comment.
Rebased on D84604 <https://reviews.llvm.org/D84604>, included static members,
and addressed review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87194/new/
https://reviews.llvm.org/D87194
Files:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-negative.cpp
Index: clang/test/SemaCXX/warn-thread-safety-negative.cpp
===================================================================
--- clang/test/SemaCXX/warn-thread-safety-negative.cpp
+++ clang/test/SemaCXX/warn-thread-safety-negative.cpp
@@ -124,9 +124,9 @@
void prot() EXCLUSIVE_LOCKS_REQUIRED(!protectedMutex);
void priv() EXCLUSIVE_LOCKS_REQUIRED(!privateMutex);
void test() {
- pub();
- prot();
- priv();
+ pub(); // expected-warning {{calling function 'pub' requires negative capability '!publicMutex'}}
+ prot(); // expected-warning {{calling function 'prot' requires negative capability '!protectedMutex'}}
+ priv(); // expected-warning {{calling function 'priv' requires negative capability '!privateMutex'}}
}
static Mutex publicMutex;
@@ -140,7 +140,54 @@
void testStaticMembers() {
StaticMembers x;
- x.pub();
+ x.pub(); // expected-warning {{calling function 'pub' requires negative capability '!publicMutex'}}
+ x.prot();
+ x.priv();
+}
+
+class Base {
+public:
+ void pub() EXCLUSIVE_LOCKS_REQUIRED(!publicMutex);
+ void prot() EXCLUSIVE_LOCKS_REQUIRED(!protectedMutex);
+ void priv() EXCLUSIVE_LOCKS_REQUIRED(!privateMutex);
+ void test() {
+ pub(); // expected-warning {{calling function 'pub' requires negative capability '!publicMutex'}}
+ prot(); // expected-warning {{calling function 'prot' requires negative capability '!protectedMutex'}}
+ priv(); // expected-warning {{calling function 'priv' requires negative capability '!privateMutex'}}
+ }
+
+ static Mutex publicMutex;
+
+protected:
+ static Mutex protectedMutex;
+
+private:
+ static Mutex privateMutex;
+};
+
+class Derived : private Base {
+public:
+ void pubDerived() EXCLUSIVE_LOCKS_REQUIRED(!publicMutex);
+ void protDerived() EXCLUSIVE_LOCKS_REQUIRED(!protectedMutex);
+
+ void inDerivedClass() {
+ pub(); // expected-warning {{calling function 'pub' requires negative capability '!publicMutex'}}
+ prot(); // expected-warning {{calling function 'prot' requires negative capability '!protectedMutex'}}
+ priv();
+ }
+};
+
+class MoreDerived : public Derived {
+public:
+ void inDerivedClassWithInacessibleBase() {
+ pubDerived(); // expected-warning {{calling function 'pubDerived' requires negative capability '!publicMutex'}}
+ protDerived();
+ }
+};
+
+void outside() {
+ Base x;
+ x.pub(); // expected-warning {{calling function 'pub' requires negative capability '!publicMutex'}}
x.prot();
x.priv();
}
Index: clang/lib/Analysis/ThreadSafety.cpp
===================================================================
--- clang/lib/Analysis/ThreadSafety.cpp
+++ clang/lib/Analysis/ThreadSafety.cpp
@@ -16,6 +16,7 @@
#include "clang/Analysis/Analyses/ThreadSafety.h"
#include "clang/AST/Attr.h"
+#include "clang/AST/CXXInheritance.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclGroup.h"
@@ -1265,6 +1266,35 @@
return "mutex";
}
+static bool accessibleMember(const ValueDecl *VD,
+ const CXXMethodDecl *CurrentMethod) {
+ AccessSpecifier Access = VD->getAccess();
+ if (Access == AS_public)
+ return true;
+
+ // FIXME: We are ignoring friends for now.
+ if (!CurrentMethod)
+ return false;
+
+ const auto *ValueDeclRecord = cast<CXXRecordDecl>(VD->getDeclContext()),
+ *MethodRecord =
+ cast<CXXRecordDecl>(CurrentMethod->getDeclContext());
+
+ // If we're in the same class, all members are accessible.
+ if (ValueDeclRecord == MethodRecord)
+ return true;
+ // Otherwise we're out of luck for private members.
+ if (Access == AS_private)
+ return false;
+ // Protected members might be accessible in derived classes.
+ assert(Access == AS_protected);
+ CXXBasePaths Paths;
+ return MethodRecord->isDerivedFrom(ValueDeclRecord, Paths) &&
+ llvm::any_of(Paths, [](const CXXBasePath &Path) {
+ return Path.Access != AS_none;
+ });
+}
+
bool ThreadSafetyAnalyzer::inCurrentScope(const CapabilityExpr &CapE) {
const threadSafety::til::SExpr *SExp = CapE.sexpr();
assert(SExp && "Null expressions should be ignored");
@@ -1274,20 +1304,14 @@
// Variables defined in a function are always inaccessible.
if (!VD->isDefinedOutsideFunctionOrMethod())
return false;
- // For now we consider static class members to be inaccessible.
if (isa<CXXRecordDecl>(VD->getDeclContext()))
- return false;
+ return accessibleMember(VD, CurrentMethod);
// Global variables are always in scope.
return true;
}
- // Members are in scope from methods of the same class.
- if (const auto *P = dyn_cast<til::Project>(SExp)) {
- if (!CurrentMethod)
- return false;
- const ValueDecl *VD = P->clangDecl();
- return VD->getDeclContext() == CurrentMethod->getDeclContext();
- }
+ if (const auto *P = dyn_cast<til::Project>(SExp))
+ return accessibleMember(P->clangDecl(), CurrentMethod);
return false;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
