Changes:
- Generalized the behavior of happens-before / happens-after annotations such
that not only 1:1 but also n:m patterns are supported.
- Dropped support for invoking happens-before / happens-after annotations on
POSIX condition variables (pthread_cond_t).
- Report the details about the offending synchronization object in generic
errors.
- Converted a few tl_assert() statements into error messages.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11073 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/drd/drd_cond.c b/drd/drd_cond.c
index 61f73eb..ec98d3c 100644
--- a/drd/drd_cond.c
+++ b/drd/drd_cond.c
@@ -27,12 +27,10 @@
#include "drd_cond.h"
#include "drd_error.h"
#include "drd_mutex.h"
-#include "drd_suppression.h"
#include "pub_tool_errormgr.h" /* VG_(maybe_record_error)() */
#include "pub_tool_libcassert.h" /* tl_assert() */
#include "pub_tool_libcprint.h" /* VG_(printf)() */
#include "pub_tool_machine.h" /* VG_(get_IP)() */
-#include "pub_tool_options.h" /* VG_(clo_backtrace_size) */
#include "pub_tool_threadstate.h" /* VG_(get_running_tid)() */
@@ -63,8 +61,8 @@
void DRD_(cond_initialize)(struct cond_info* const p, const Addr cond)
{
tl_assert(cond != 0);
- tl_assert(p->a1 == cond);
- tl_assert(p->type == ClientCondvar);
+ tl_assert(p->a1 == cond);
+ tl_assert(p->type == ClientCondvar);
p->cleanup = (void(*)(DrdClientobj*))(DRD_(cond_cleanup));
p->delete_thread = 0;
@@ -83,10 +81,13 @@
{
struct mutex_info* q;
q = &(DRD_(clientobj_get)(p->mutex, ClientMutex)->mutex);
- tl_assert(q);
{
- CondDestrErrInfo cde = { DRD_(thread_get_running_tid)(),
- p->a1, q->a1, q->owner };
+ CondDestrErrInfo cde = {
+ DRD_(thread_get_running_tid)(),
+ p->a1,
+ q ? q->a1 : 0,
+ q ? q->owner : DRD_INVALID_THREADID
+ };
VG_(maybe_record_error)(VG_(get_running_tid)(),
CondDestrErr,
VG_(get_IP)(VG_(get_running_tid)()),
@@ -97,17 +98,40 @@
}
}
+/**
+ * Report that the synchronization object at address 'addr' is of the
+ * wrong type.
+ */
+static void wrong_type(const Addr addr)
+{
+ GenericErrInfo gei = {
+ .tid = DRD_(thread_get_running_tid)(),
+ .addr = addr,
+ };
+ VG_(maybe_record_error)(VG_(get_running_tid)(),
+ GenericErr,
+ VG_(get_IP)(VG_(get_running_tid)()),
+ "wrong type of synchronization object",
+ &gei);
+}
+
static struct cond_info* cond_get_or_allocate(const Addr cond)
{
struct cond_info *p;
tl_assert(offsetof(DrdClientobj, cond) == 0);
p = &(DRD_(clientobj_get)(cond, ClientCondvar)->cond);
- if (p == 0)
+ if (p)
+ return p;
+
+ if (DRD_(clientobj_present)(cond, cond + 1))
{
- p = &(DRD_(clientobj_add)(cond, ClientCondvar)->cond);
- DRD_(cond_initialize)(p, cond);
+ wrong_type(cond);
+ return 0;
}
+
+ p = &(DRD_(clientobj_add)(cond, ClientCondvar)->cond);
+ DRD_(cond_initialize)(p, cond);
return p;
}
@@ -184,10 +208,11 @@
DRD_(clientobj_remove)(p->a1, ClientCondvar);
}
-/** Called before pthread_cond_wait(). Note: before this function is called,
- * mutex_unlock() has already been called from drd_clientreq.c.
+/**
+ * Called before pthread_cond_wait(). Note: before this function is called,
+ * mutex_unlock() has already been called from drd_clientreq.c.
*/
-int DRD_(cond_pre_wait)(const Addr cond, const Addr mutex)
+void DRD_(cond_pre_wait)(const Addr cond, const Addr mutex)
{
struct cond_info* p;
struct mutex_info* q;
@@ -201,7 +226,16 @@
}
p = cond_get_or_allocate(cond);
- tl_assert(p);
+ if (!p)
+ {
+ CondErrInfo cei = { .tid = DRD_(thread_get_running_tid)(), .cond = cond };
+ VG_(maybe_record_error)(VG_(get_running_tid)(),
+ CondErr,
+ VG_(get_IP)(VG_(get_running_tid)()),
+ "not a condition variable",
+ &cei);
+ return;
+ }
if (p->waiter_count == 0)
{
@@ -238,11 +272,13 @@
DRD_(not_a_mutex)(p->mutex);
}
- return ++p->waiter_count;
+ ++p->waiter_count;
}
-/** Called after pthread_cond_wait(). */
-int DRD_(cond_post_wait)(const Addr cond)
+/**
+ * Called after pthread_cond_wait().
+ */
+void DRD_(cond_post_wait)(const Addr cond)
{
struct cond_info* p;
@@ -255,55 +291,89 @@
}
p = DRD_(cond_get)(cond);
- if (p)
+ if (!p)
{
- if (p->waiter_count > 0)
+ struct mutex_info* q;
+ q = &(DRD_(clientobj_get)(p->mutex, ClientMutex)->mutex);
{
- --p->waiter_count;
- if (p->waiter_count == 0)
- {
- p->mutex = 0;
- }
+ CondDestrErrInfo cde = {
+ DRD_(thread_get_running_tid)(),
+ p->a1,
+ q ? q->a1 : 0,
+ q ? q->owner : DRD_INVALID_THREADID
+ };
+ VG_(maybe_record_error)(VG_(get_running_tid)(),
+ CondDestrErr,
+ VG_(get_IP)(VG_(get_running_tid)()),
+ "condition variable has been destroyed while"
+ " being waited upon",
+ &cde);
}
- return p->waiter_count;
+ return;
}
- return 0;
+
+ if (p->waiter_count > 0)
+ {
+ --p->waiter_count;
+ if (p->waiter_count == 0)
+ {
+ p->mutex = 0;
+ }
+ }
}
-static void DRD_(cond_signal)(Addr const cond)
+static void cond_signal(const DrdThreadId tid, struct cond_info* const cond_p)
{
const ThreadId vg_tid = VG_(get_running_tid)();
const DrdThreadId drd_tid = DRD_(VgThreadIdToDrdThreadId)(vg_tid);
- struct cond_info* const cond_p = DRD_(cond_get)(cond);
- if (cond_p && cond_p->waiter_count > 0)
+ tl_assert(cond_p);
+
+ if (cond_p->waiter_count > 0)
{
if (DRD_(s_report_signal_unlocked)
- && ! DRD_(mutex_is_locked_by)(cond_p->mutex, drd_tid))
+ && ! DRD_(mutex_is_locked_by)(cond_p->mutex, drd_tid))
{
- /* A signal is sent while the associated mutex has not been locked. */
- /* This can indicate but is not necessarily a race condition. */
- CondRaceErrInfo cei = { .tid = DRD_(thread_get_running_tid)(),
- .cond = cond,
- .mutex = cond_p->mutex,
- };
- VG_(maybe_record_error)(vg_tid,
- CondRaceErr,
- VG_(get_IP)(vg_tid),
- "CondErr",
- &cei);
+ /*
+ * A signal is sent while the associated mutex has not been locked.
+ * This can indicate but is not necessarily a race condition.
+ */
+ CondRaceErrInfo cei = { .tid = DRD_(thread_get_running_tid)(),
+ .cond = cond_p->a1,
+ .mutex = cond_p->mutex,
+ };
+ VG_(maybe_record_error)(vg_tid,
+ CondRaceErr,
+ VG_(get_IP)(vg_tid),
+ "CondErr",
+ &cei);
}
}
else
{
- /* No other thread is waiting for the signal, hence the signal will be */
- /* lost. This is normal in a POSIX threads application. */
+ /*
+ * No other thread is waiting for the signal, hence the signal will
+ * be lost. This is normal in a POSIX threads application.
+ */
}
}
+static void not_initialized(Addr const cond)
+{
+ CondErrInfo cei = { .tid = DRD_(thread_get_running_tid)(), .cond = cond };
+ VG_(maybe_record_error)(VG_(get_running_tid)(),
+ CondErr,
+ VG_(get_IP)(VG_(get_running_tid)()),
+ "condition variable has not been initialized",
+ &cei);
+}
+
/** Called before pthread_cond_signal(). */
void DRD_(cond_pre_signal)(Addr const cond)
{
+ struct cond_info* p;
+
+ p = DRD_(cond_get)(cond);
if (DRD_(s_trace_cond))
{
VG_(message)(Vg_UserMsg,
@@ -312,12 +382,20 @@
cond);
}
- DRD_(cond_signal)(cond);
+ if (!p)
+ {
+ not_initialized(cond);
+ return;
+ }
+
+ cond_signal(DRD_(thread_get_running_tid)(), p);
}
/** Called before pthread_cond_broadcast(). */
void DRD_(cond_pre_broadcast)(Addr const cond)
{
+ struct cond_info* p;
+
if (DRD_(s_trace_cond))
{
VG_(message)(Vg_UserMsg,
@@ -326,5 +404,12 @@
cond);
}
- DRD_(cond_signal)(cond);
+ p = DRD_(cond_get)(cond);
+ if (!p)
+ {
+ not_initialized(cond);
+ return;
+ }
+
+ cond_signal(DRD_(thread_get_running_tid)(), p);
}