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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to