llvmbot
commented
2 months ago @llvm/issue-subscribers-clang-frontend
Author: Ilya Biryukov (ilya-biryukov)
The thread safety analysis can distinguish between reads and writes to variables of simple types, but falls short when the write happens through a method call (see https://gcc.godbolt.org/z/8dffM6j1e and the code below).
It seems to treat all method calls as reads and never as writes. This is a conservative approach that avoids false positives.
However, it would be useful to have a mode that treat all non-const method calls as writes and const method calls as reads (similarly, passing an object through a const reference is a read and through a reference as a write?). E.g. this would allow to catch things like `vector.push_back()` made under a read lock.
Has this been discussed before?
Should we consider adding a new warning flag to enable this class of warnings? (basically treats capture-by-reference as a write and capture-by-const-reference as a read)
```cpp
struct __attribute__((lockable)) mutex {};
struct __attribute__((scoped_lockable)) lock_guard {
lock_guard(mutex& mu) __attribute__((exclusive_lock_function(mu)));
~lock_guard() __attribute__((unlock_function));
};
struct __attribute__((scoped_lockable)) shared_lock_guard {
shared_lock_guard(mutex& mu) __attribute__((shared_lock_function(mu)));
~shared_lock_guard() __attribute__((unlock_function));
};
// Wraps an int.
struct value {
int get() const;
void set(int);
};
struct Test {
mutable mutex mu;
value val __attribute__((guarded_by(mu)));
int num __attribute__((guarded_by(mu)));
void accidental_write_under_shared_lock() {
shared_lock_guard l(mu);
num = 10; // ok, warning.
val.set(10); // no warning, but it would be nice to have one.
}
};
```
ilya-biryukov
aaronpuchert
johngro
delesley
The thread safety analysis can distinguish between reads and writes to variables of simple types, but falls short when the write happens through a method call (see https://gcc.godbolt.org/z/8dffM6j1e and the code below). It seems to treat all method calls as reads and never as writes. This is a conservative approach that avoids false positives. However, it would be useful to have a mode that treat all non-const method calls as writes and const method calls as reads (similarly, passing an object through a const reference is a read and through a reference as a write?). E.g. this would allow to catch things like
vector.push_back()made under a read lock.Has this been discussed before? Should we consider adding a new warning flag to enable this class of warnings? (basically treats capture-by-reference as a write and capture-by-const-reference as a read)