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.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@5942

diff --git a/callgrind/bb.c b/callgrind/bb.c
index a6c8ebadcfcc6d945821a140c8322b500dd3ef71..c489560462b1f8a13d0191db922bff7bcf1566e6 100644 (file)
--- a/callgrind/bb.c
+++ b/callgrind/bb.c
@@ -120,16 +120,17 @@ static BB* new_bb(obj_node* obj, OffT offset,
                  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;

diff --git a/callgrind/main.c b/callgrind/main.c
index c031f1f9b0532c32b3f8d5e31a3563c81c812110..e646d0ee9d5fa8cb9a6fd603110e0b94869be2e8 100644 (file)
--- a/callgrind/main.c
+++ b/callgrind/main.c
@@ -272,29 +272,44 @@ void endOfInstr(IRBB* bbOut, InstrInfo* ii, Bool bb_seen_before,
    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);
 }
 
 #if defined(VG_BIGENDIAN)