[PATCH] StopTheWorld in sanitizer_common (first commit)

Sergey Matveev earthdok at google.com
Wed Feb 27 05:50:09 PST 2013



================
Comment at: lib/sanitizer_common/sanitizer_stoptheworld.h:27
@@ +26,3 @@
+// functionality (to be implemented).
+class SuspendedThreadsList {
+ public:
----------------
Kostya Serebryany wrote:
> Why do you need a whole new class for this? 
> Isn't it better to add the Contains method to InternalVector and just use typedef InternalVector<> SuspendedThreadsVector? 
An object of this class is passed to the callback, which can then use it to extract certain information about each thread (such as the register state).

Also, we might want to change the implementation of Contains() to something more efficient than a linear search in an unsorted vector.

================
Comment at: lib/sanitizer_common/sanitizer_stoptheworld_linux.cc:247
@@ +246,3 @@
+void StopTheWorld(StopTheWorldCallback callback, void *argument) {
+  SanitizerVerbosity = 1;
+  BlockingMutexLock lock(&stoptheworld_mutex);
----------------
Kostya Serebryany wrote:
> did you leave this here intentionally? 
oops

================
Comment at: lib/sanitizer_common/tests/sanitizer_stoptheworld_test.cc:85
@@ +84,3 @@
+  // terminate before we can return from this function.
+  int join_status = pthread_join(thread_id, NULL);
+  ASSERT_EQ(join_status, 0);
----------------
Alexey Samsonov wrote:
> Kostya Serebryany wrote:
> > EXPECT_EQ(thread_join(thread_id, NULL), 0)
> EXPECT_EQ(0, pthread_join(thread_id, NULL))
> Generally the first arg of EXPECT_EQ is the expected value.
> EXPECT

Failing to terminate IncrementerThread would case flaky segfaults. Doesn't ASSERT make more sense here?


http://llvm-reviews.chandlerc.com/D469



More information about the llvm-commits mailing list