Fix bug 246152

When unwinding needs to be done because the stack pointer is reset
(e.g. by a longjmp), it makes no sense to interprete the control
flow change as call, but should be seen as a return.

This indirectly fixes bug 246152. Unwinding potentially changes the
exec state, which is unique for threads, but also for signal handlers.
E.g. this is true for a longjmp out of a signal handler. Exec state
changes modify members of struct CLG_(current_state), such as
CLG_(current_state).bbcc and CLG_(current_state).jmps_passed, which
are backed in CLG_(setup_bbcc)() by last_bbcc and passed, respectivly.
On a exec state change, these local vars go out of sync, and lead
to invalid data passed to CLG_(push_call_stack)() for handling a call,
which triggered data corruption, and the symptoms seen in bug 246152.
As in the given situation, there is no call anymore, there is no call
into CLG_(push_call_stack)(), and the corruption (or since last commit
the failed assertion) is not triggered any more.

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

diff --git a/callgrind/bbcc.c b/callgrind/bbcc.c
index 24862a80caaacbee8ba9fef40a36856f2c4ec05e..bab48585445c078db770e4b7dd68be5b030760b6 100644 (file)
--- a/callgrind/bbcc.c
+++ b/callgrind/bbcc.c
@@ -741,7 +741,11 @@ void CLG_(setup_bbcc)(BB* bb)
     }
   }
   else {
-    CLG_(unwind_call_stack)(sp, 0);
+    Int unwind_count = CLG_(unwind_call_stack)(sp, 0);
+    if (unwind_count > 0) {
+      /* if unwinding was done, this actually is a return */
+      jmpkind = Ijk_Ret;
+    }
     
     if (jmpkind == Ijk_Call) {
       delayed_push = True;

diff --git a/callgrind/callstack.c b/callgrind/callstack.c
index 24087f5bc38b68c387bec51157ea39a608234a19..ec48369c730f9b13d35b17e685b1ebb85ffe0a27 100644 (file)
--- a/callgrind/callstack.c
+++ b/callgrind/callstack.c
@@ -394,11 +394,13 @@ void CLG_(pop_call_stack)()
 }
 
 
-/* remove CallStack items to sync with current SP
+/* Unwind enough CallStack items to sync with current stack pointer.
+ * Returns the number of stack frames unwinded.
  */
-void CLG_(unwind_call_stack)(Addr sp, Int minpops)
+Int CLG_(unwind_call_stack)(Addr sp, Int minpops)
 {
     Int csp;
+    Int unwind_count = 0;
     CLG_DEBUG(4,"+ unwind_call_stack(sp %#lx, minpops %d): frame %d\n",
              sp, minpops, CLG_(current_call_stack).sp);
 
@@ -415,6 +417,7 @@ void CLG_(unwind_call_stack)(Addr sp, Int minpops)
            ((top_ce->sp == sp) && minpops>0)) {
 
            minpops--;
+           unwind_count++;
            CLG_(pop_call_stack)();
            csp=CLG_(current_call_stack).sp;
            continue;
@@ -423,4 +426,5 @@ void CLG_(unwind_call_stack)(Addr sp, Int minpops)
     }
 
     CLG_DEBUG(4,"- unwind_call_stack\n");
+    return unwind_count;
 }

diff --git a/callgrind/global.h b/callgrind/global.h
index bfb5a48168cf7a31e604bd98e61add5d5ee2d766..d66e631e31d188559fa9fefd635921d96bdb10d6 100644 (file)
--- a/callgrind/global.h
+++ b/callgrind/global.h
@@ -779,7 +779,7 @@ call_entry* CLG_(get_call_entry)(Int n);
 
 void CLG_(push_call_stack)(BBCC* from, UInt jmp, BBCC* to, Addr sp, Bool skip);
 void CLG_(pop_call_stack)(void);
-void CLG_(unwind_call_stack)(Addr sp, Int);
+Int CLG_(unwind_call_stack)(Addr sp, Int);
 
 /* from context.c */
 void CLG_(init_fn_stack)(fn_stack*);