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