Merge drd/ changes from branches/MESSAGING_TIDYUP r10464.
See trunk r10465 commit message for details.
git-svn-id: svn://svn.valgrind.org/valgrind/trunk@10466 a5019735-40e9-0310-863c-91ae7b9d1cf9
diff --git a/drd/drd_error.c b/drd/drd_error.c
index 1928b6d..ed93cbf 100644
--- a/drd/drd_error.c
+++ b/drd/drd_error.c
@@ -85,7 +85,7 @@
{
tl_assert(cl->any.first_observed_at);
VG_(message)(Vg_UserMsg,
- "%s 0x%lx was first observed at:",
+ "%s 0x%lx was first observed at:\n",
DRD_(clientobj_type_name)(cl->any.type),
obj);
VG_(pp_ExeContext)(cl->any.first_observed_at);
@@ -96,9 +96,13 @@
void drd_report_data_race(Error* const err, const DataRaceErrInfo* const dri)
{
AddrInfo ai;
- const unsigned descr_size = 256;
- Char* descr1 = VG_(malloc)("drd.error.drdr2.1", descr_size);
- Char* descr2 = VG_(malloc)("drd.error.drdr2.2", descr_size);
+
+ XArray* /* of HChar */ descr1
+ = VG_(newXA)( VG_(malloc), "drd.error.drdr2.1",
+ VG_(free), sizeof(HChar) );
+ XArray* /* of HChar */ descr2
+ = VG_(newXA)( VG_(malloc), "drd.error.drdr2.2",
+ VG_(free), sizeof(HChar) );
tl_assert(dri);
tl_assert(dri->addr);
@@ -106,31 +110,50 @@
tl_assert(descr1);
tl_assert(descr2);
- descr1[0] = 0;
- descr2[0] = 0;
- VG_(get_data_description)(descr1, descr2, descr_size, dri->addr);
- if (descr1[0] == 0)
+ (void) VG_(get_data_description)(descr1, descr2, dri->addr);
+ /* If there's nothing in descr1/2, free them. Why is it safe to to
+ VG_(indexXA) at zero here? Because VG_(get_data_description)
+ guarantees to zero terminate descr1/2 regardless of the outcome
+ of the call. So there's always at least one element in each XA
+ after the call.
+ */
+ if (0 == VG_(strlen)( VG_(indexXA)( descr1, 0 ))) {
+ VG_(deleteXA)( descr1 );
+ descr1 = NULL;
+ }
+ if (0 == VG_(strlen)( VG_(indexXA)( descr2, 0 ))) {
+ VG_(deleteXA)( descr2 );
+ descr2 = NULL;
+ }
+ /* Assume (assert) that VG_(get_data_description) fills in descr1
+ before it fills in descr2 */
+ if (descr1 == NULL)
+ tl_assert(descr2 == NULL);
+ /* So anyway. Do we have something useful? */
+ if (descr1 == NULL)
{
+ /* No. Do Plan B. */
describe_malloced_addr(dri->addr, dri->size, &ai);
}
VG_(message)(Vg_UserMsg,
- "Conflicting %s by thread %d/%d at 0x%08lx size %ld",
+ "Conflicting %s by thread %d/%d at 0x%08lx size %ld\n",
dri->access_type == eStore ? "store" : "load",
DRD_(DrdThreadIdToVgThreadId)(dri->tid),
dri->tid,
dri->addr,
dri->size);
VG_(pp_ExeContext)(VG_(get_error_where)(err));
- if (descr1[0])
+ if (descr1 != NULL)
{
- VG_(message)(Vg_UserMsg, "%s", descr1);
- VG_(message)(Vg_UserMsg, "%s", descr2);
+ VG_(message)(Vg_UserMsg, "%s\n", (HChar*)VG_(indexXA)(descr1, 0));
+ if (descr2 != NULL)
+ VG_(message)(Vg_UserMsg, "%s\n", (HChar*)VG_(indexXA)(descr2, 0));
}
else if (ai.akind == eMallocd && ai.lastchange)
{
VG_(message)(Vg_UserMsg,
"Address 0x%lx is at offset %ld from 0x%lx."
- " Allocation context:",
+ " Allocation context:\n",
dri->addr, ai.rwoffset, dri->addr - ai.rwoffset);
VG_(pp_ExeContext)(ai.lastchange);
}
@@ -144,13 +167,13 @@
if (sect_kind != Vg_SectUnknown)
{
VG_(message)(Vg_UserMsg,
- "Allocation context: %s section of %s",
+ "Allocation context: %s section of %s\n",
VG_(pp_SectKind)(sect_kind),
sect_name);
}
else
{
- VG_(message)(Vg_UserMsg, "Allocation context: unknown.");
+ VG_(message)(Vg_UserMsg, "Allocation context: unknown.\n");
}
}
if (s_show_conflicting_segments)
@@ -160,8 +183,10 @@
dri->access_type);
}
- VG_(free)(descr2);
- VG_(free)(descr1);
+ if (descr2)
+ VG_(deleteXA)(descr2);
+ if (descr1)
+ VG_(deleteXA)(descr1);
}
static Bool drd_tool_error_eq(VgRes res, Error* e1, Error* e2)
@@ -169,6 +194,12 @@
return False;
}
+static void drd_tool_error_before_pp(Error* const e)
+{
+ /* No need to do anything; drd_tool_error_pp does all
+ the work. */
+}
+
static void drd_tool_error_pp(Error* const e)
{
static DrdThreadId s_last_tid_printed = 1;
@@ -178,7 +209,7 @@
if (err_extra && *err_extra != s_last_tid_printed)
{
- VG_UMSG("%s:", DRD_(thread_get_name)(*err_extra));
+ VG_(umsg)("%s:\n", DRD_(thread_get_name)(*err_extra));
s_last_tid_printed = *err_extra;
}
@@ -194,7 +225,7 @@
if (p->recursion_count >= 0)
{
VG_(message)(Vg_UserMsg,
- "%s: mutex 0x%lx, recursion count %d, owner %d.",
+ "%s: mutex 0x%lx, recursion count %d, owner %d.\n",
VG_(get_error_string)(e),
p->mutex,
p->recursion_count,
@@ -203,7 +234,7 @@
else
{
VG_(message)(Vg_UserMsg,
- "The object at address 0x%lx is not a mutex.",
+ "The object at address 0x%lx is not a mutex.\n",
p->mutex);
}
VG_(pp_ExeContext)(VG_(get_error_where)(e));
@@ -213,7 +244,7 @@
case CondErr: {
CondErrInfo* cdei =(CondErrInfo*)(VG_(get_error_extra)(e));
VG_(message)(Vg_UserMsg,
- "%s: cond 0x%lx",
+ "%s: cond 0x%lx\n",
VG_(get_error_string)(e),
cdei->cond);
VG_(pp_ExeContext)(VG_(get_error_where)(e));
@@ -223,7 +254,7 @@
case CondDestrErr: {
CondDestrErrInfo* cdi = (CondDestrErrInfo*)(VG_(get_error_extra)(e));
VG_(message)(Vg_UserMsg,
- "%s: cond 0x%lx, mutex 0x%lx locked by thread %d/%d",
+ "%s: cond 0x%lx, mutex 0x%lx locked by thread %d/%d\n",
VG_(get_error_string)(e),
cdi->cond, cdi->mutex,
DRD_(DrdThreadIdToVgThreadId)(cdi->owner), cdi->owner);
@@ -236,7 +267,7 @@
VG_(message)(Vg_UserMsg,
"Probably a race condition: condition variable 0x%lx has"
" been signaled but the associated mutex 0x%lx is not"
- " locked by the signalling thread.",
+ " locked by the signalling thread.\n",
cei->cond, cei->mutex);
VG_(pp_ExeContext)(VG_(get_error_where)(e));
first_observed(cei->cond);
@@ -246,7 +277,7 @@
case CondWaitErr: {
CondWaitErrInfo* cwei = (CondWaitErrInfo*)(VG_(get_error_extra)(e));
VG_(message)(Vg_UserMsg,
- "%s: condition variable 0x%lx, mutexes 0x%lx and 0x%lx",
+ "%s: condition variable 0x%lx, mutexes 0x%lx and 0x%lx\n",
VG_(get_error_string)(e),
cwei->cond,
cwei->mutex1,
@@ -261,7 +292,7 @@
SemaphoreErrInfo* sei = (SemaphoreErrInfo*)(VG_(get_error_extra)(e));
tl_assert(sei);
VG_(message)(Vg_UserMsg,
- "%s: semaphore 0x%lx",
+ "%s: semaphore 0x%lx\n",
VG_(get_error_string)(e),
sei->semaphore);
VG_(pp_ExeContext)(VG_(get_error_where)(e));
@@ -272,14 +303,14 @@
BarrierErrInfo* bei = (BarrierErrInfo*)(VG_(get_error_extra)(e));
tl_assert(bei);
VG_(message)(Vg_UserMsg,
- "%s: barrier 0x%lx",
+ "%s: barrier 0x%lx\n",
VG_(get_error_string)(e),
bei->barrier);
VG_(pp_ExeContext)(VG_(get_error_where)(e));
if (bei->other_context)
{
VG_(message)(Vg_UserMsg,
- "Conflicting wait call by thread %d/%d:",
+ "Conflicting wait call by thread %d/%d:\n",
DRD_(DrdThreadIdToVgThreadId)(bei->other_tid),
bei->other_tid);
VG_(pp_ExeContext)(bei->other_context);
@@ -291,7 +322,7 @@
RwlockErrInfo* p = (RwlockErrInfo*)(VG_(get_error_extra)(e));
tl_assert(p);
VG_(message)(Vg_UserMsg,
- "%s: rwlock 0x%lx.",
+ "%s: rwlock 0x%lx.\n",
VG_(get_error_string)(e),
p->rwlock);
VG_(pp_ExeContext)(VG_(get_error_where)(e));
@@ -302,10 +333,10 @@
HoldtimeErrInfo* p =(HoldtimeErrInfo*)(VG_(get_error_extra)(e));
tl_assert(p);
tl_assert(p->acquired_at);
- VG_(message)(Vg_UserMsg, "Acquired at:");
+ VG_(message)(Vg_UserMsg, "Acquired at:\n");
VG_(pp_ExeContext)(p->acquired_at);
VG_(message)(Vg_UserMsg,
- "Lock on %s 0x%lx was held during %d ms (threshold: %d ms).",
+ "Lock on %s 0x%lx was held during %d ms (threshold: %d ms).\n",
VG_(get_error_string)(e),
p->synchronization_object,
p->hold_time_ms,
@@ -316,13 +347,13 @@
}
case GenericErr: {
//GenericErrInfo* gei =(GenericErrInfo*)(VG_(get_error_extra)(e));
- VG_(message)(Vg_UserMsg, "%s", VG_(get_error_string)(e));
+ VG_(message)(Vg_UserMsg, "%s\n", VG_(get_error_string)(e));
VG_(pp_ExeContext)(VG_(get_error_where)(e));
break;
}
default:
VG_(message)(Vg_UserMsg,
- "%s",
+ "%s\n",
VG_(get_error_string)(e));
VG_(pp_ExeContext)(VG_(get_error_where)(e));
break;
@@ -436,6 +467,7 @@
{
// Tool error reporting.
VG_(needs_tool_errors)(drd_tool_error_eq,
+ drd_tool_error_before_pp,
drd_tool_error_pp,
False,
drd_tool_error_update_extra,