|
From: Josef W. <Jos...@gm...> - 2006-05-26 18:18:42
|
On Friday 26 May 2006 01:22, Julian Seward wrote:
> Naturally, if there is any critical breakage that should be fixed
> before 3.2.0, please shout now.
I have an important bug fix for callgrind here to close 4 bug reports
(privately by mail): Callgrind fails with an assertion failure when
reinstrumenting rep or cmov instructions (e.g. because of plugin reloading).
Since some time I was not able to reproduce it, as it happened only with big
programs.
See attached patch; Julian: may I commit it now or later?
Josef
==
Fix a failed assertion on retranslation of rep or cmov instructions.
Bug description: Very similar to cachegrind, callgrind stores
metainformation per guest instruction; this meta information is
given when calling into the simulator. In contrast to cachegrind,
callgrind keeps this info when the source is discarded, and checks
on retranslation whether the same meta info is generated.
This check sometimes fails: E.g. for rep x86 instructions, 2 simulator calls
are usually generated for one x86 instruction (the instruction fetch and a
data access), thus overwriting the data_size meta information for one x86
instruction first with 0, and afterwards e.g. with 1. The check on retranslation
fails because of this. The fix is to only write/check data_size values >0.
===================================================================
--- bb.c (Revision 5933)
+++ bb.c (Arbeitskopie)
@@ -120,16 +120,17 @@
UInt instr_count, UInt cjmp_count, Bool cjmp_inverted)
{
BB* new;
- UInt new_idx;
+ UInt new_idx, size;
/* check fill degree of bb hash table and resize if needed (>80%) */
bbs.entries++;
if (10 * bbs.entries / bbs.size > 8)
resize_bb_table();
- new = (BB*) CLG_MALLOC(sizeof(BB) +
- instr_count * sizeof(InstrInfo) +
- (cjmp_count+1) * sizeof(CJmpInfo));
+ size = sizeof(BB) + instr_count * sizeof(InstrInfo)
+ + (cjmp_count+1) * sizeof(CJmpInfo);
+ new = (BB*) CLG_MALLOC(size);
+ VG_(memset)(new, 0, size);
new->obj = obj;
new->offset = offset;
Index: main.c
===================================================================
--- main.c (Revision 5933)
+++ main.c (Arbeitskopie)
@@ -272,29 +272,44 @@
es = insert_simcall(bbOut, ii, dataSize, instrIssued,
loadAddrExpr, storeAddrExpr);
+ CLG_DEBUG(5, " Instr +%2d (Size %d, DSize %d): ESet %s (Size %d)\n",
+ instr_offset, instrLen, dataSize,
+ es ? es->name : (Char*)"(no instrumentation)",
+ es ? es->size : 0);
+
if (bb_seen_before) {
+ CLG_DEBUG(5, " before: Instr +%2d (Size %d, DSize %d)\n",
+ ii->instr_offset, ii->instr_size, ii->data_size);
+
CLG_ASSERT(ii->instr_offset == instr_offset);
CLG_ASSERT(ii->instr_size == instrLen);
- CLG_ASSERT(ii->data_size == dataSize);
CLG_ASSERT(ii->cost_offset == *cost_offset);
CLG_ASSERT(ii->eventset == es);
+
+ /* Only check size if data size >0.
+ * This is needed: e.g. for rep or cmov x86 instructions, the same InstrInfo
+ * is used both for 2 simulator calls: for the pure instruction fetch and
+ * separately for an memory access (which may not happen depending on flags).
+ * If checked always, this triggers an assertion failure on retranslation.
+ */
+ if (dataSize>0) CLG_ASSERT(ii->data_size == dataSize);
+
}
else {
ii->instr_offset = instr_offset;
ii->instr_size = instrLen;
- ii->data_size = dataSize;
ii->cost_offset = *cost_offset;
ii->eventset = es;
+
+ /* data size only relevant if >0 */
+ if (dataSize > 0) ii->data_size = dataSize;
+
CLG_(stat).distinct_instrs++;
}
*cost_offset += es ? es->size : 0;
- CLG_DEBUG(5, " Instr +%2d (Size %d, DSize %d): ESet %s (Size %d)\n",
- instr_offset, instrLen, dataSize,
- es ? es->name : (Char*)"(no Instr)",
- es ? es->size : 0);
}
|