aaronpuchert wrote:
We had a discussion on https://reviews.llvm.org/D52395 that might be relevant
here. To quote @delesley:
> When you pass an object to a function, either by pointer or by reference, no
> actual load from memory has yet occurred. Thus, there is a real risk of false
> positives; what you are saying is that the function *might* read or write
> from protected memory, not that it actually will.
Another aspect is that we don't check if the lock is still held when we
dereference the pointer, so there are also false negatives:
```c++
Mutex mu;
int x GUARDED_BY(mu);
void f() {
mu.Lock();
int *p = &x;
mu.Unlock();
*p = 0;
}
```
You use the analogy of C++ references. Interestingly, we don't warn on "taking
a reference", partly because no such thing exists. (You can only initialize
something of reference type with an expression.) We warn on passing a variable
by reference *to a function*, or in other words, initializing a parameter of
reference type with a guarded variable. (Since
https://reviews.llvm.org/D153131, we also warn on returning a reference when
the lock is no longer held after return. Note that the initialization of the
reference might still happen under lock!)
However, we also track references in a way that pointers are not tracked. The
reason is probably that references are immutable (that is, `T&` is semantically
the same thing as `T* const`).
```c++
void g() {
mu.Lock();
int &ref = x;
mu.Unlock();
ref = 0; // warning: writing variable 'x' requires holding mutex 'mu'
exclusively
}
```
If you "take the reference" outside of the lock and do the assignment inside,
there is no warning. Because the assignment is what needs the lock, not taking
the address.
However, with functions it's somewhat reasonable to assume that the function
accesses through the pointer, and that the pointer doesn't escape. (This is of
course not always true, but often enough.) So I wonder whether we should
restrict this to pointers passed as function arguments.
But if the number of false positives is manageable, we can also leave it like
you implemented it. Did you try it out on some code base that uses thread
safety annotations?
https://github.com/llvm/llvm-project/pull/123063
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
