Ah, thanks a lot for the explanation DeLesley! Makes sense. So, if I define get_foo_copy in terms of get_foo_reference, no safety warnings can be given because the GUARDED_BY was casted away when returning foo_ from get_foo_reference.
Is there some way to eliminate the implicit cast by making the types match? This naive approach didn't work: int GUARDED_BY(foo_lock_)& get_foo_reference() { return foo_; } warning: 'guarded_by' attribute only applies to fields and global variables [-Wignored-attributes] On Wed, May 4, 2016 at 8:47 AM, Delesley Hutchins <deles...@google.com> wrote: > > This is expected behavior. Returning foo by reference does not involve an > actual read from memory; the read doesn't happen until later, when the > reference is used. Thus there is no warning in get_foo_reference(). > > If thread safety annotations were integrated with the C++ type system, > then we could do better. What's actually happening is an implicit type > cast from a guarded reference (int GUARDED_BY(...)&) to an unguarded > reference (int&), which is kind of like a cast from a const to a non-const > pointer. Unfortunately, the attributes are NOT part of the type, so it's > difficult to check such "implicit casts" in a comprehensive manner. If you > turn on -Wthread-safety-reference, it will catch some cases, but not all. > Trying to track implicit casts also involves a lot of false positives (much > like const), so Wthread-safety-reference is not on by default. > > Moreover, even with better type information, you could still get false > negatives. Assume that the analysis issued a warning if it detected an > implicit cast when the lock wasn't held. You could still acquire a lock, > grab the reference (no warning), release the lock, and then use the > reference to write to the underlying memory. > > In order to fix THAT case, you need to have pointer lifetime analysis and > escape analysis (a-la Rust), combined with a type system where the > thread-safety attributes are built-in. An implicit cast should return a > temporary reference, that cannot escape the scope of the lock()/unlock(). > That level of analysis is well beyond the capabilities of the current > implementation. > > -DeLesley > > > On Wed, Apr 27, 2016 at 5:36 PM, RJ Ryan via cfe-dev < > cfe-...@lists.llvm.org> wrote: > >> +cfe-dev -- in case anyone knows why this happens. >> >> Thanks! >> >> On Sat, Apr 23, 2016 at 1:41 PM, RJ Ryan <rr...@alum.mit.edu> wrote: >> >>> Hi there, >>> >>> The following code only produces thread safety warnings for me in >>> get_foo_copy, not get_foo_reference. Is this expected? Sorry if this is a >>> FAQ / known issue. >>> >>> $ clang --version >>> Apple LLVM version 7.0.2 (clang-700.1.81) >>> Target: x86_64-apple-darwin15.3.0 >>> Thread model: posix >>> >>> The problem also seems present on clang 3.8.0: >>> https://godbolt.org/g/37DqNy >>> >>> Best, >>> RJ >>> >>> (with the usual thread annotation definitions) >>> >>> class CAPABILITY("mutex") Mutex { >>> public: >>> Mutex() {} >>> inline void lock() ACQUIRE() {} >>> inline void unlock() RELEASE() {} >>> }; >>> >>> class Foo { >>> public: >>> int& get_foo_reference() { >>> return foo_; >>> } >>> >>> int get_foo_copy() { >>> return foo_; >>> } >>> >>> private: >>> Mutex foo_lock_; >>> int foo_ GUARDED_BY(foo_lock_); >>> }; >>> >>> >> >> _______________________________________________ >> cfe-dev mailing list >> cfe-...@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >> >> > > > -- > DeLesley Hutchins | Software Engineer | deles...@google.com | 505-206-0315 >
_______________________________________________ cfe-users mailing list cfe-users@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users