[cfe-dev] [libc++] thread safety annotations for unique_lock

Wed Nov 9 08:03:10 PST 2016

Am 09.11.16, 01:32 schrieb "Richard Trieu" <rtrieu at google.com>:

>
>
>On Tue, Nov 8, 2016 at 10:54 AM, christoph at ruediger.engineering via cfe-dev
><cfe-dev at lists.llvm.org> wrote:
>
>Sorry for the incomplete mail; hit the wrong shortcut. The mail continues down below.
>
>Am 08.11.16, 19:38 schrieb "cfe-dev im Auftrag von 
>cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>" <cfe-dev-bounces at lists.llvm.org im Auftrag von
>cfe-dev at lists.llvm.org>:
>
>>Hi everyone,
>>
>>this is my first post to this list, so please remind me if I’m violating your netiquette in some way.
>>
>>I’m trying to introduce the thread safety analysis to one of our projects and stumble upon the missing thread safety annotations for unique_lock. I thought this might be easy, so let’s provide a patch. But now I’m stuck with correctly annotating the move constructor
> and the lock() and unlock() methods of unique_lock.
>>
>>Here is a very basic implementation of my annotated unique_lock:
>>
>>class SCOPED_CAPABILITY unique_lock {
>>  ::bluebox::mutex *mu;
>>  bool lock_held;
>>public:
>>  unique_lock() = delete;
>>  unique_lock(unique_lock &&) = delete;   // We cannot annotate move semantics
>>
>>  explicit unique_lock(::bluebox::mutex &m) ACQUIRE(m) : mu(&m), lock_held(true) { m.lock(); }
>>  explicit unique_lock(::bluebox::mutex *m) ACQUIRE(m) : mu(m), lock_held(true) { m->lock(); }
>
>
> Change this to ACQUIRE(mu) instead of ACQUIRE(m).  The ACQUIRE and RELEASE annotations need
> to refer to the same variable for the thread safety analysis to work.

This doesn’t work either. When putting ACQUIRE(mu) in the constructor annotation, I get this error:

$ clang++-3.8 -Wthread-safety -O0 -std=c++11 mutex.cpp
mutex.cpp:65:12: warning: writing variable 'any_number' requires holding mutex 'mtx' exclusively [-Wthread-safety-analysis]
  return ++any_number;
           ^
mutex.cpp:70:3: warning: writing variable 'any_number' requires holding mutex 'mtx' exclusively [-Wthread-safety-analysis]
  any_number = 42;
  ^
2 warnings generated.

I’ve put together a very minimalistic example which can be found at https://dl.dropboxusercontent.com/u/13595729/mutex.cpp

The basic code fragment from the example is (with the mutex and unique locker as described in my previous mail):
mutex mtx;
int any_number GUARDED_BY(mtx) = 0;

int increment() EXCLUDES(mtx) {
  unique_lock lock(mtx);
  return ++any_number;   // <- First warning here
}

int main(int argc, char const *argv[]) {
  unique_lock lock(mtx);
  any_number = 42;  // <- Second warning here
  lock.unlock();
  int i = increment();
  lock.lock();
  return i;
}

To me it looks like the analyzer does not get it that unique_lock::mu is essentially the same mutex as mtx.

Just for reference, the warnings are exactly the same when using pointers instead of references in unique_lock.

> 
>
>>
>>  void unlock() RELEASE(mu) NO_THREAD_SAFETY_ANALYSIS {
>>    if (lock_held) {
>>      lock_held = false;
>>      mu->unlock();
>>    }
>>  }
>>
>>  void lock() ACQUIRE(mu) NO_THREAD_SAFETY_ANALYSIS {
>>    if (!lock_held) {
>>      mu->lock();
>>      lock_held = true;
>>    }
>>  }
>>
>>  ~unique_lock() RELEASE() NO_THREAD_SAFETY_ANALYSIS {
>>    if (lock_held)
>>      mu->unlock();
>>  }
>>};
>>
>>
>>And this is my test case:
>>
>>static std::mutex mtx;
>>void someFunction() {
>>  my::unique_lock lock(mtx);
>>  // … Here you would normally do something meaningful
>>  lock.unlock();
>>  // … Call a method that might modify your protected data
>>  lock.lock();
>>  // … Continue with super important work
>>}
>>
>>And here is my list of issues I can’t get around with:
>>1. How can I correctly annotate the unlock and lock methods? When using unlock() in the above example, clang throws this error: fatal error: releasing mutex
>>      'lock.mu <http://lock.mu>' that was not held [-Wthread-safety-analysis]
>>2. How can I correctly annotate the move constructor of unique_lock?
>
>
>
>3. I couldn’t find really that much valuable information about the annotations except for the one page in the clang documentation and a talk from DeLesly Hutchins. Plus the information, that the guys at Google are making heavy use of the thread safety analyzer.
> But are you really limiting yourself to std::mutex and std::lock_guard? You can’t even use std::condition_variable. Maybe one of the Google engineers on this list can shade some light without revealing confidential information.
>
>With these first tests in mind, I’m having troubles to use this really helpful feature in my projects. Anybody who wants to share some experience is appreciated. I will try to update the documentation accordingly or provide any other help.
>
>Thanks and regards,
>Christoph
>
>--
>rüdiger.engineering
>Christoph Rüdiger
>Düsseldorfer Str. 12
>45145 Essen
>Germany
>
>phone: +49 201 458 478 58 <tel:%2B49%20201%20458%20478%2058>
>VAT ID/Ust. ID: DE304572498
>
>
>_______________________________________________
>cfe-dev mailing list
>cfe-dev at lists.llvm.org
>http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>_______________________________________________
>cfe-dev mailing list
>cfe-dev at lists.llvm.org
>http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
>
>
>
>
>
>