Fixes http://reviews.llvm.org/rL230691

[chaoren]

Summary

OS X doesn't implement pthread barriers, using a simple atomic flag instead.

[ki.stfu]

test/functionalities/watchpoint/hello_watchlocation/main.cpp

107

it was useful.

[chaoren]

Accidentally removed the memory leak fix.

[ki.stfu]

Looks good. I haven't tested it yet, but you can commit it if you have.

I'll check it a bit later.

[ki.stfu]

In D7933#131048, @ki.stfu wrote:

I'll check it a bit later.

Yep. It works on OS X.

[clayborg]

Someone else already posted a fix for this that used pthread_condition_t and pthread_mutex_t, so don't commit this.

[zturner]

lldb/trunk/test/functionalities/watchpoint/hello_watchlocation/main.cpp

58 ↗

(On Diff #20832)

std::condition_variable?

65 ↗

(On Diff #20832)

I understand this isn't your code, but RAND_MAX is not guaranteed to be > 32768. On Windows, it isn't. std::random would be better here.

93–95 ↗

(On Diff #20832)

Since we're already bringing in std::atomic, how about std::thread while we're at it? That would make this testcase compile on Windows.

[chaoren]

@Greg, the author of the other fix and I have sort of decided among
ourselves to abandon his fix and use this instead.

@zach, would it be better if I just rewrote this entire test with C++11
instead of C? Should that be a general rule for all tests in the future?

[clayborg]

I like your fix better. The C++ way will spin one CPU while waiting for a global to change. I much prefer this solution the other thread will park itself instead of spin locking.

[clayborg]

I meant to say I like the other fix that uses pthread_condion_t and pthread_mutex_t since it won't spin like this fix will. Please use the other one.

[chaoren]

I'm rewriting the test to use std::condition_variable and other C++11
alternatives.

[zturner]

In D7933#131286, @clayborg wrote:

I meant to say I like the other fix that uses pthread_condion_t and pthread_mutex_t since it won't spin like this fix will. Please use the other one.

Just to be clear, are you saying you don't want C++11 at all? Or that you just want a condition variable instead of a spinlock? Because if you just want a condition variable, then we should go with the C++11 and std::condition_variable as I suggested, since the test will work on platforms that don't have pthreads.

[zturner]

In D7933#131281, @chaoren wrote:

@zach, would it be better if I just rewrote this entire test with C++11
instead of C? Should that be a general rule for all tests in the future?

It's not a general rule for *all* tests, but I think it should be a general rule for all tests that aren't specifically testing a property of the debugger that requires only C code. So I think it should be a general rule for *almost* all tests, but there may occasionally be valid reasons to test strict C.

[clayborg]

As I commented in the other fix for this bug, we should move to using the correct C++11 primitives to abstract us from the host system. pthread_ types and functions obviously don't do this.

I strongly feel we should use std::condition_variable and std::mutex and anything else the C++11 libraries can do for us.

[zturner]

Cool, sounds like we're in agreement then!

test/functionalities/watchpoint/hello_watchlocation/main.cpp

 //===-- main.cpp ------------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // C includes
 #include <pthread.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
 #include <unistd.h>
+#include <atomic>
 
 pthread_t g_thread_1 = NULL;
 pthread_t g_thread_2 = NULL;
 pthread_t g_thread_3 = NULL;
 
-pthread_barrier_t g_barrier;
+std::atomic_bool g_ready(false);
 
 char *g_char_ptr = NULL;
 
 void
 do_bad_thing_with_location(char *char_ptr, char new_val)
 {
     unsigned what = new_val;
     printf("new value written to location(%p) = %u\n", char_ptr, what);

 }
 
 void *
 thread_func (void *arg)
 {
     uint32_t thread_index = *((uint32_t *)arg);
     printf ("%s (thread index = %u) startng...\n", __FUNCTION__, thread_index);
 
-    pthread_barrier_wait(&g_barrier);
+    while (!g_ready);
 
     uint32_t count = 0;
     uint32_t val;
     while (count++ < 15)
     {
         // random micro second sleep from zero to 3 seconds
         int usec = ::rand() % 3000000;
         printf ("%s (thread = %u) doing a usleep (%d)...\n", __FUNCTION__, thread_index, usec);
int main (int argc, char const *argv[])
     void *thread_result = NULL;
     uint32_t thread_index_1 = 1;
     uint32_t thread_index_2 = 2;
     uint32_t thread_index_3 = 3;
 
     g_char_ptr = (char *)malloc (1);
     *g_char_ptr = 0;
 
-    pthread_barrier_init(&g_barrier, NULL, 4);
-
     // Create 3 threads
     err = ::pthread_create (&g_thread_1, NULL, thread_func, &thread_index_1);
     err = ::pthread_create (&g_thread_2, NULL, thread_func, &thread_index_2);
     err = ::pthread_create (&g_thread_3, NULL, thread_func, &thread_index_3);
 
     printf ("Before turning all three threads loose...\n"); // Set break point at this line.
-    pthread_barrier_wait(&g_barrier);
+    g_ready = true;
 
     // Join all of our threads
     err = ::pthread_join (g_thread_1, &thread_result);
     err = ::pthread_join (g_thread_2, &thread_result);
     err = ::pthread_join (g_thread_3, &thread_result);
 
-    pthread_barrier_destroy(&g_barrier);
-    free(g_char_ptr);
-
     return 0;
 }