gz-sim icon indicating copy to clipboard operation
gz-sim copied to clipboard

Use Clang Thread Safety Analysis

Open darksylinc opened this issue 2 years ago • 0 comments

I went nuts trying to decipher VisualizeLidar.cc thread model (I figured out) and in the process discovered #1487 thanks to my use of Clang TSA.

Clang TSA should be implemented everywhere to catch bugs like this.

We don't really care if the projects link with Clang. All it needs is to compile.

To use Clang TSA we need the following:

sudo apt install clang

The following example uses Clang TSA:

CMake

# Use clang instead of gcc
set( CMAKE_C_COMPILER "/usr/bin/clang" )
set( CMAKE_CXX_COMPILER "/usr/bin/clang++" )

# Use libc++ instead of stdlib++
# Enable thread safety warnings (off by default)
# Enable std::mutex annotations (off by default)
set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -stdlib=libc++ -Wthread-safety -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS" )
set( CMAKE_EXE_LINKER_FLAGS "{$CMAKE_EXE_LINKER_FLAGS} -stdlib=libc++ -lc++abi" )

C++

#include <mutex>

#if defined(__clang__)
#  define THREAD_ANNOTATION_ATTRIBUTE__(x) __attribute__((x))
#else
#  define THREAD_ANNOTATION_ATTRIBUTE__(x)  // no-op
#endif
#define GUARDED_BY(x) THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x))
#define PT_GUARDED_BY(x) THREAD_ANNOTATION_ATTRIBUTE__(pt_guarded_by(x))

struct MyStruct
{
	std::mutex m;
	int a GUARDED_BY(m);
};

int main()
{
	MyStruct myStruct;
	
	{
		std::lock_guard<std::mutex> lock( myStruct.m );
		myStruct.a = 5;
	}
	{
		std::lock_guard<std::mutex>( myStruct.m );
		myStruct.a = 5;// warning: writing variable 'a' requires holding mutex 'myStruct.m' exclusively [-Wthread-safety-analysis]
	}
}

When compiled by GCC, these macros are no-op. Clang needs to use libc++ (instead of stdlib++) because std::mutex comes with the necessary extension annotations.

For all the available macros see the bottom of Clang TSA docs

Desired behavior

Incorrect usage of mutexes and data races should be caught at compile time. Additionally, these annotations are like self-documenting code because it forces to write down the threading relationships.

Alternatives considered

There are more thread sanitation functionality (e.g. Clang's Thread Sanitizer or GCC's equivalent TSan which runs at runtime), but this is a good start.

Implementation suggestion

I am implementing a GUI plugin that will show this in action.

Additional context

Clang TSA caught #1487

darksylinc avatar May 14 '22 20:05 darksylinc