From: Josef Weidendorfer <Josef.Weidendorfer@gmx.de>
Date: Sat, 27 May 2006 15:30:58 +0000 (+0000)
Subject: Fix a failed assertion on retranslation of rep or cmov instructions.
X-Git-Tag: svn/VALGRIND_3_2_0~17
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fb75a5cece9310b14a387f89ce180ef24102610a;p=thirdparty%2Fvalgrind.git

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 a6c8ebadcf..c489560462 100644
--- 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 c031f1f9b0..e646d0ee9d 100644
--- 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)