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
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 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
