Skip to content

Commit 3c8c0d4

Browse files
committed
Thread Safety Analysis: Handle address-of followed by dereference
Correctly analyze expressions where the address of a guarded variable is taken and immediately dereferenced, such as (*(type-specifier *)&x). Previously, such patterns would result in false negatives. Pull Request: #127396
1 parent 5c8e22b commit 3c8c0d4

File tree

2 files changed

+30
-0
lines changed

2 files changed

+30
-0
lines changed

clang/lib/Analysis/ThreadSafety.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1765,6 +1765,8 @@ void ThreadSafetyAnalyzer::checkAccess(const FactSet &FSet, const Expr *Exp,
17651765
void ThreadSafetyAnalyzer::checkPtAccess(const FactSet &FSet, const Expr *Exp,
17661766
AccessKind AK,
17671767
ProtectedOperationKind POK) {
1768+
// Strip off paren- and cast-expressions, checking if we encounter any other
1769+
// operator that should be delegated to checkAccess() instead.
17681770
while (true) {
17691771
if (const auto *PE = dyn_cast<ParenExpr>(Exp)) {
17701772
Exp = PE->getSubExpr();
@@ -1783,6 +1785,15 @@ void ThreadSafetyAnalyzer::checkPtAccess(const FactSet &FSet, const Expr *Exp,
17831785
break;
17841786
}
17851787

1788+
if (const auto *UO = dyn_cast<UnaryOperator>(Exp)) {
1789+
if (UO->getOpcode() == UO_AddrOf) {
1790+
// Pointer access via pointer taken of variable, so the dereferenced
1791+
// variable is not actually a pointer.
1792+
checkAccess(FSet, UO->getSubExpr(), AK, POK);
1793+
return;
1794+
}
1795+
}
1796+
17861797
// Pass by reference warnings are under a different flag.
17871798
ProtectedOperationKind PtPOK = POK_VarDereference;
17881799
if (POK == POK_PassByRef) PtPOK = POK_PtPassByRef;

clang/test/Sema/warn-thread-safety-analysis.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
__attribute__ ((shared_locks_required(__VA_ARGS__)))
2525
#define NO_THREAD_SAFETY_ANALYSIS __attribute__ ((no_thread_safety_analysis))
2626

27+
#define __READ_ONCE(x) (*(const volatile __typeof__(x) *)&(x))
28+
#define __WRITE_ONCE(x, val) do { *(volatile __typeof__(x) *)&(x) = (val); } while (0)
29+
2730
// Define the mutex struct.
2831
// Simplified only for test purpose.
2932
struct LOCKABLE Mutex {};
@@ -142,9 +145,25 @@ int main(void) {
142145
(void)(get_value(b_) == 1);
143146
mutex_unlock(foo_.mu_);
144147

148+
a_ = 0; // expected-warning{{writing variable 'a_' requires holding mutex 'foo_.mu_'}}
149+
__WRITE_ONCE(a_, 0); // expected-warning{{writing variable 'a_' requires holding mutex 'foo_.mu_'}}
150+
(void)(a_ == 0); // expected-warning{{reading variable 'a_' requires holding mutex 'foo_.mu_'}}
151+
(void)(__READ_ONCE(a_) == 0); // expected-warning{{reading variable 'a_' requires holding mutex 'foo_.mu_'}}
152+
*b_ = 0; // expected-warning{{writing the value pointed to by 'b_' requires holding mutex 'foo_.mu_' exclusively}}
153+
__WRITE_ONCE(*b_, 0); // expected-warning{{writing the value pointed to by 'b_' requires holding mutex 'foo_.mu_' exclusively}}
154+
(void)(*b_ == 0); // expected-warning{{reading the value pointed to by 'b_' requires holding mutex 'foo_.mu_'}}
155+
(void)(__READ_ONCE(*b_) == 0); // expected-warning{{reading the value pointed to by 'b_' requires holding mutex 'foo_.mu_'}}
145156
c_ = 0; // expected-warning{{writing variable 'c_' requires holding any mutex exclusively}}
146157
(void)(*d_ == 0); // expected-warning{{reading the value pointed to by 'd_' requires holding any mutex}}
147158
mutex_exclusive_lock(foo_.mu_);
159+
a_ = 0;
160+
__WRITE_ONCE(a_, 0);
161+
(void)(a_ == 0);
162+
(void)(__READ_ONCE(a_) == 0);
163+
*b_ = 0;
164+
__WRITE_ONCE(*b_, 0);
165+
(void)(*b_ == 0);
166+
(void)(__READ_ONCE(*b_) == 0);
148167
c_ = 1;
149168
(void)(*d_ == 1);
150169
mutex_unlock(foo_.mu_);

0 commit comments

Comments
 (0)