Add thread_annotations for clang targets.

TESTED: As expected clang bots catched a few issues which are fixed with this CL, other bots ignore the annotations and compile fine.

R=niklas.enbom@webrtc.org, phoglund@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/6209004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5328 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/build/common.gypi

@@ -186,6 +186,11 @@
               '-Woverloaded-virtual',
             ],
           }],
+          ['clang==1', {
+            'cflags': [
+              '-Wthread-safety',
+            ],
+          }],
         ],
       }],
       ['target_arch=="arm" or target_arch=="armv7"', {

webrtc/modules/audio_device/linux/audio_device_alsa_linux.h

@@ -174,8 +174,8 @@ private:
     bool KeyPressed() const;
 
 private:
-    void Lock() { _critSect.Enter(); };
+    void Lock() EXCLUSIVE_LOCK_FUNCTION(_critSect) { _critSect.Enter(); };
-    void UnLock() { _critSect.Leave(); };
+    void UnLock() UNLOCK_FUNCTION(_critSect) { _critSect.Leave(); };
 private:
     inline int32_t InputSanityCheckAfterUnlockedPeriod() const;
     inline int32_t OutputSanityCheckAfterUnlockedPeriod() const;

webrtc/modules/audio_device/linux/audio_device_pulse_linux.cc

@@ -2613,7 +2613,7 @@ int32_t AudioDeviceLinuxPulse::ReadRecordedData(const void* bufferData,
 int32_t AudioDeviceLinuxPulse::ProcessRecordedData(
     int8_t *bufferData,
     uint32_t bufferSizeInSamples,
-    uint32_t recDelay)
+    uint32_t recDelay) EXCLUSIVE_LOCKS_REQUIRED(_critSect)
 {
     uint32_t currentMicLevel(0);
     uint32_t newMicLevel(0);

webrtc/modules/audio_device/linux/audio_device_pulse_linux.h

@@ -224,16 +224,12 @@ public:
     virtual void AttachAudioBuffer(AudioDeviceBuffer* audioBuffer) OVERRIDE;
 
 private:
-    void Lock()
+    void Lock() EXCLUSIVE_LOCK_FUNCTION(_critSect) {
-    {
         _critSect.Enter();
     }
-    ;
+    void UnLock() UNLOCK_FUNCTION(_critSect) {
-    void UnLock()
-    {
         _critSect.Leave();
     }
-    ;
     void WaitForOperationCompletion(pa_operation* paOperation) const;
     void WaitForSuccess(pa_operation* paOperation) const;
 

webrtc/modules/pacing/paced_sender.cc

@@ -290,7 +290,8 @@ int32_t PacedSender::Process() {
 }
 
 // MUST have critsect_ when calling.
-bool PacedSender::SendPacketFromList(paced_sender::PacketList* packet_list) {
+bool PacedSender::SendPacketFromList(paced_sender::PacketList* packet_list)
+    EXCLUSIVE_LOCKS_REQUIRED(critsect_) {
   paced_sender::Packet packet = GetNextPacketFromList(packet_list);
   critsect_->Leave();
 

webrtc/modules/rtp_rtcp/source/fec_receiver_impl.cc

@@ -223,6 +223,7 @@ int32_t FecReceiverImpl::ProcessReceivedFec() {
       crit_sect_->Enter();
     }
     if (fec_->DecodeFEC(&received_packet_list_, &recovered_packet_list_) != 0) {
+      crit_sect_->Leave();
       return -1;
     }
     assert(received_packet_list_.empty());

webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc

@@ -480,11 +480,12 @@ RTCPReceiver::HandleSenderReceiverReport(RTCPUtility::RTCPParserV2& rtcpParser,
 }
 
 // no need for critsect we have _criticalSectionRTCPReceiver
-void
+void RTCPReceiver::HandleReportBlock(
-RTCPReceiver::HandleReportBlock(const RTCPUtility::RTCPPacket& rtcpPacket,
+    const RTCPUtility::RTCPPacket& rtcpPacket,
     RTCPPacketInformation& rtcpPacketInformation,
     const uint32_t remoteSSRC,
-                                const uint8_t numberOfReportBlocks) {
+    const uint8_t numberOfReportBlocks)
+    EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver) {
   // This will be called once per report block in the RTCP packet.
   // We filter out all report blocks that are not for us.
   // Each packet has max 31 RR blocks.
@@ -940,7 +941,8 @@ void RTCPReceiver::HandleXrDlrrReportBlock(
 
 void RTCPReceiver::HandleXrDlrrReportBlockItem(
     const RTCPUtility::RTCPPacket& packet,
-    RTCPPacketInformation& rtcpPacketInformation) {
+    RTCPPacketInformation& rtcpPacketInformation)
+    EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver) {
   if (registered_ssrcs_.find(packet.XRDLRRReportBlockItem.SSRC) ==
       registered_ssrcs_.end()) {
     // Not to us.

webrtc/system_wrappers/interface/critical_section_wrapper.h

@@ -15,9 +15,10 @@
 // read/write locks instead.
 
 #include "webrtc/common_types.h"
+#include "webrtc/system_wrappers/interface/thread_annotations.h"
 
 namespace webrtc {
-class CriticalSectionWrapper {
+class LOCKABLE CriticalSectionWrapper {
  public:
   // Factory method, constructor disabled
   static CriticalSectionWrapper* CreateCriticalSection();
@@ -26,33 +27,25 @@ class CriticalSectionWrapper {
 
   // Tries to grab lock, beginning of a critical section. Will wait for the
   // lock to become available if the grab failed.
-  virtual void Enter() = 0;
+  virtual void Enter() EXCLUSIVE_LOCK_FUNCTION() = 0;
 
   // Returns a grabbed lock, end of critical section.
-  virtual void Leave() = 0;
+  virtual void Leave() UNLOCK_FUNCTION() = 0;
 };
 
 // RAII extension of the critical section. Prevents Enter/Leave mismatches and
 // provides more compact critical section syntax.
-class CriticalSectionScoped {
+class SCOPED_LOCKABLE CriticalSectionScoped {
  public:
   explicit CriticalSectionScoped(CriticalSectionWrapper* critsec)
+      EXCLUSIVE_LOCK_FUNCTION(critsec)
       : ptr_crit_sec_(critsec) {
     ptr_crit_sec_->Enter();
   }
 
-  ~CriticalSectionScoped() {
+  ~CriticalSectionScoped() UNLOCK_FUNCTION() { ptr_crit_sec_->Leave(); }
-    if (ptr_crit_sec_) {
-      Leave();
-    }
-  }
 
  private:
-  void Leave() {
-    ptr_crit_sec_->Leave();
-    ptr_crit_sec_ = 0;
-  }
-
   CriticalSectionWrapper* ptr_crit_sec_;
 };
 

webrtc/system_wrappers/interface/rw_lock_wrapper.h

@@ -11,35 +11,36 @@
 #ifndef WEBRTC_SYSTEM_WRAPPERS_INTERFACE_RW_LOCK_WRAPPER_H_
 #define WEBRTC_SYSTEM_WRAPPERS_INTERFACE_RW_LOCK_WRAPPER_H_
 
+#include "webrtc/system_wrappers/interface/thread_annotations.h"
+
 // Note, Windows pre-Vista version of RW locks are not supported natively. For
 // these OSs regular critical sections have been used to approximate RW lock
 // functionality and will therefore have worse performance.
 
 namespace webrtc {
 
-class RWLockWrapper {
+class LOCKABLE RWLockWrapper {
  public:
   static RWLockWrapper* CreateRWLock();
   virtual ~RWLockWrapper() {}
 
-  virtual void AcquireLockExclusive() = 0;
+  virtual void AcquireLockExclusive() EXCLUSIVE_LOCK_FUNCTION() = 0;
-  virtual void ReleaseLockExclusive() = 0;
+  virtual void ReleaseLockExclusive() UNLOCK_FUNCTION() = 0;
 
-  virtual void AcquireLockShared() = 0;
+  virtual void AcquireLockShared() SHARED_LOCK_FUNCTION() = 0;
-  virtual void ReleaseLockShared() = 0;
+  virtual void ReleaseLockShared() UNLOCK_FUNCTION() = 0;
 };
 
 // RAII extensions of the RW lock. Prevents Acquire/Release missmatches and
 // provides more compact locking syntax.
-class ReadLockScoped {
+class SCOPED_LOCKABLE ReadLockScoped {
  public:
-  ReadLockScoped(RWLockWrapper& rw_lock)
+  ReadLockScoped(RWLockWrapper& rw_lock) SHARED_LOCK_FUNCTION(rw_lock)
-    :
+      : rw_lock_(rw_lock) {
-    rw_lock_(rw_lock) {
     rw_lock_.AcquireLockShared();
   }
 
-  ~ReadLockScoped() {
+  ~ReadLockScoped() UNLOCK_FUNCTION() {
     rw_lock_.ReleaseLockShared();
   }
 
@@ -47,15 +48,14 @@ class ReadLockScoped {
   RWLockWrapper& rw_lock_;
 };
 
-class WriteLockScoped {
+class SCOPED_LOCKABLE WriteLockScoped {
  public:
-  WriteLockScoped(RWLockWrapper& rw_lock)
+  WriteLockScoped(RWLockWrapper& rw_lock) EXCLUSIVE_LOCK_FUNCTION(rw_lock)
-    :
+      : rw_lock_(rw_lock) {
-    rw_lock_(rw_lock) {
     rw_lock_.AcquireLockExclusive();
   }
 
-  ~WriteLockScoped() {
+  ~WriteLockScoped() UNLOCK_FUNCTION() {
     rw_lock_.ReleaseLockExclusive();
   }
 

webrtc/system_wrappers/interface/thread_annotations.h

@@ -0,0 +1,99 @@
+//
+// Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
+//
+// Use of this source code is governed by a BSD-style license
+// that can be found in the LICENSE file in the root of the source
+// tree. An additional intellectual property rights grant can be found
+// in the file PATENTS.  All contributing project authors may
+// be found in the AUTHORS file in the root of the source tree.
+//
+// Borrowed from
+// https://code.google.com/p/gperftools/source/browse/src/base/thread_annotations.h
+// but adapted for clang attributes instead of the gcc.
+//
+// This header file contains the macro definitions for thread safety
+// annotations that allow the developers to document the locking policies
+// of their multi-threaded code. The annotations can also help program
+// analysis tools to identify potential thread safety issues.
+
+#ifndef BASE_THREAD_ANNOTATIONS_H_
+#define BASE_THREAD_ANNOTATIONS_H_
+
+#if defined(__clang__) && (!defined(SWIG))
+#define THREAD_ANNOTATION_ATTRIBUTE__(x) __attribute__((x))
+#else
+#define THREAD_ANNOTATION_ATTRIBUTE__(x)  // no-op
+#endif
+
+// Document if a shared variable/field needs to be protected by a lock.
+// GUARDED_BY allows the user to specify a particular lock that should be
+// held when accessing the annotated variable, while GUARDED_VAR only
+// indicates a shared variable should be guarded (by any lock). GUARDED_VAR
+// is primarily used when the client cannot express the name of the lock.
+#define GUARDED_BY(x) THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x))
+#define GUARDED_VAR THREAD_ANNOTATION_ATTRIBUTE__(guarded)
+
+// Document if the memory location pointed to by a pointer should be guarded
+// by a lock when dereferencing the pointer. Similar to GUARDED_VAR,
+// PT_GUARDED_VAR is primarily used when the client cannot express the name
+// of the lock. Note that a pointer variable to a shared memory location
+// could itself be a shared variable. For example, if a shared global pointer
+// q, which is guarded by mu1, points to a shared memory location that is
+// guarded by mu2, q should be annotated as follows:
+//     int *q GUARDED_BY(mu1) PT_GUARDED_BY(mu2);
+#define PT_GUARDED_BY(x) THREAD_ANNOTATION_ATTRIBUTE__(point_to_guarded_by(x))
+#define PT_GUARDED_VAR THREAD_ANNOTATION_ATTRIBUTE__(point_to_guarded)
+
+// Document the acquisition order between locks that can be held
+// simultaneously by a thread. For any two locks that need to be annotated
+// to establish an acquisition order, only one of them needs the annotation.
+// (i.e. You don't have to annotate both locks with both ACQUIRED_AFTER
+// and ACQUIRED_BEFORE.)
+#define ACQUIRED_AFTER(x) THREAD_ANNOTATION_ATTRIBUTE__(acquired_after(x))
+#define ACQUIRED_BEFORE(x) THREAD_ANNOTATION_ATTRIBUTE__(acquired_before(x))
+
+// The following three annotations document the lock requirements for
+// functions/methods.
+
+// Document if a function expects certain locks to be held before it is called
+#define EXCLUSIVE_LOCKS_REQUIRED(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(exclusive_locks_required(__VA_ARGS__))
+
+#define SHARED_LOCKS_REQUIRED(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(shared_locks_required(__VA_ARGS__))
+
+// Document the locks acquired in the body of the function. These locks
+// cannot be held when calling this function (as google3's Mutex locks are
+// non-reentrant).
+#define LOCKS_EXCLUDED(x) THREAD_ANNOTATION_ATTRIBUTE__(locks_excluded(x))
+
+// Document the lock the annotated function returns without acquiring it.
+#define LOCK_RETURNED(x) THREAD_ANNOTATION_ATTRIBUTE__(lock_returned(x))
+
+// Document if a class/type is a lockable type (such as the Mutex class).
+#define LOCKABLE THREAD_ANNOTATION_ATTRIBUTE__(lockable)
+
+// Document if a class is a scoped lockable type (such as the MutexLock class).
+#define SCOPED_LOCKABLE THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable)
+
+// The following annotations specify lock and unlock primitives.
+#define EXCLUSIVE_LOCK_FUNCTION(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(exclusive_lock_function(__VA_ARGS__))
+
+#define SHARED_LOCK_FUNCTION(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(shared_lock_function(__VA_ARGS__))
+
+#define EXCLUSIVE_TRYLOCK_FUNCTION(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(exclusive_trylock_function(__VA_ARGS__))
+
+#define SHARED_TRYLOCK_FUNCTION(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(shared_trylock_function(__VA_ARGS__))
+
+#define UNLOCK_FUNCTION(...) \
+  THREAD_ANNOTATION_ATTRIBUTE__(unlock_function(__VA_ARGS__))
+
+// An escape hatch for thread safety analysis to ignore the annotated function.
+#define NO_THREAD_SAFETY_ANALYSIS \
+  THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis)
+
+#endif  // BASE_THREAD_ANNOTATIONS_H_

webrtc/system_wrappers/source/critical_section_unittest.cc

@@ -81,7 +81,7 @@ bool LockUnlockThenStopRunFunction(void* obj) {
   return false;
 }
 
-TEST_F(CritSectTest, ThreadWakesOnce) {
+TEST_F(CritSectTest, ThreadWakesOnce) NO_THREAD_SAFETY_ANALYSIS {
   CriticalSectionWrapper* crit_sect =
       CriticalSectionWrapper::CreateCriticalSection();
   ProtectedCount count(crit_sect);
@@ -110,7 +110,7 @@ bool LockUnlockRunFunction(void* obj) {
   return true;
 }
 
-TEST_F(CritSectTest, ThreadWakesTwice) {
+TEST_F(CritSectTest, ThreadWakesTwice) NO_THREAD_SAFETY_ANALYSIS {
   CriticalSectionWrapper* crit_sect =
       CriticalSectionWrapper::CreateCriticalSection();
   ProtectedCount count(crit_sect);

webrtc/system_wrappers/source/system_wrappers.gyp

@@ -48,6 +48,7 @@
         '../interface/sort.h',
         '../interface/static_instance.h',
         '../interface/stringize_macros.h',
+        '../interface/thread_annotations.h',
         '../interface/thread_wrapper.h',
         '../interface/tick_util.h',
         '../interface/trace.h',

webrtc/video_engine/test/libvietest/testbed/tb_external_transport.cc

@@ -404,7 +404,6 @@ bool TbExternalTransport::ViEExternalTransportProcess()
             {
                 waitTime = (unsigned int) timeToReceive;
             }
-            _crit.Leave();
             break;
         }
         _rtpPackets.pop_front();
@@ -488,7 +487,6 @@ bool TbExternalTransport::ViEExternalTransportProcess()
             {
                 waitTime = (unsigned int) timeToReceive;
             }
-            _crit.Leave();
             break;
         }
         _rtcpPackets.pop_front();

webrtc/video_engine/vie_manager_base.h

@@ -11,11 +11,13 @@
 #ifndef WEBRTC_VIDEO_ENGINE_VIE_MANAGER_BASE_H_
 #define WEBRTC_VIDEO_ENGINE_VIE_MANAGER_BASE_H_
 
+#include "webrtc/system_wrappers/interface/thread_annotations.h"
+
 namespace webrtc {
 
 class RWLockWrapper;
 
-class ViEManagerBase {
+class LOCKABLE ViEManagerBase {
   friend class ViEManagedItemScopedBase;
   friend class ViEManagerScopedBase;
   friend class ViEManagerWriteScoped;
@@ -25,24 +27,25 @@ class ViEManagerBase {
 
  private:
   // Exclusive lock, used by ViEManagerWriteScoped.
-  void WriteLockManager();
+  void WriteLockManager() EXCLUSIVE_LOCK_FUNCTION();
 
   // Releases exclusive lock, used by ViEManagerWriteScoped.
-  void ReleaseWriteLockManager();
+  void ReleaseWriteLockManager() UNLOCK_FUNCTION();
 
   // Increases lock count, used by ViEManagerScopedBase.
-  void ReadLockManager() const;
+  void ReadLockManager() const SHARED_LOCK_FUNCTION();
 
   // Releases the lock count, used by ViEManagerScopedBase.
-  void ReleaseLockManager() const;
+  void ReleaseLockManager() const UNLOCK_FUNCTION();
 
   RWLockWrapper& instance_rwlock_;
 };
 
-class ViEManagerWriteScoped {
+class SCOPED_LOCKABLE ViEManagerWriteScoped {
  public:
-  explicit ViEManagerWriteScoped(ViEManagerBase* vie_manager);
+  explicit ViEManagerWriteScoped(ViEManagerBase* vie_manager)
-  ~ViEManagerWriteScoped();
+      EXCLUSIVE_LOCK_FUNCTION(vie_manager);
+  ~ViEManagerWriteScoped() UNLOCK_FUNCTION();
 
  private:
   ViEManagerBase* vie_manager_;