Blob Blame History Raw
commit f250c4d3241c156f8e65398e2af76e3e2ee1ccb5
Author: philippe <philippe@a5019735-40e9-0310-863c-91ae7b9d1cf9>
Date:   Wed Nov 18 20:56:55 2015 +0000

    Fix incorrect (or infinite loop) unwind on RHEL7 x86 32 bits.
    
    On RHEL7 x86 32 bits, Valgrind unwinder cannot properly unwind
    the stack just after a thread creation : the unwinder always retrieves
    the same pc/sp/bp.
    See below for an example.
    This has as consequences that some stack traces are bigger than
    needed (i.e. they always fill up the ips array). If
    --merge-recursive-frames is given, then the unwinder enters in an
    infinite loop (as identical frames will be merged, and the ips array
    will never be filled in).
    Thi patch adds an additional exit condition : after unwinding
    a frame, if the previous sp is >= new sp, then unwinding stops.
    Patch has been tested on debian 8/x86, RHEL7/x86.
    
    
    
       0x0417db67 <+55>:    mov    0x18(%esp),%ebx
       0x0417db6b <+59>:    mov    0x28(%esp),%edi
       0x0417db6f <+63>:    mov    $0x78,%eax
       0x0417db74 <+68>:    mov    %ebx,(%ecx)
       0x0417db76 <+70>:    int    $0x80
    => 0x0417db78 <+72>:    pop    %edi
       0x0417db79 <+73>:    pop    %esi
       0x0417db7a <+74>:    pop    %ebx
       0x0417db7b <+75>:    test   %eax,%eax
    
    Valgrind stacktrace gives:
    ==21261==    at 0x417DB78: clone (clone.S:110)
    ==21261==    by 0x424702F: ???
    ==21261==    by 0x424702F: ???
    ==21261==    by 0x424702F: ???
    ==21261==    by 0x424702F: ???
    ==21261==    by 0x424702F: ???
    ==21261==    by 0x424702F: ???
    ==21261==    by 0x424702F: ???
    ...
    (till the array of ips is full)
    
    while gdb stacktrace gives:
    (gdb) bt
    #0  clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:110
    #1  0x00000000 in ?? ()
    (gdb) p $pc
    $2 = (void (*)()) 0x417db78 <clone+72>
    (gdb)
    
    
    With the fix, valgrind gives:
    ==21261==    at 0x417DB78: clone (clone.S:110)
    ==21261==    by 0x424702F: ???
    which looks more reasonable.
    
    
    
    
    git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15729 a5019735-40e9-0310-863c-91ae7b9d1cf9

diff --git a/coregrind/m_stacktrace.c b/coregrind/m_stacktrace.c
index 8c1e9a4..137e780 100644
--- a/coregrind/m_stacktrace.c
+++ b/coregrind/m_stacktrace.c
@@ -350,6 +350,8 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
           uregs.xbp <= fp_max - 1 * sizeof(UWord)/*see comment below*/ &&
           VG_IS_4_ALIGNED(uregs.xbp))
       {
+         Addr old_xsp;
+
          /* fp looks sane, so use it. */
          uregs.xip = (((UWord*)uregs.xbp)[1]);
          // We stop if we hit a zero (the traditional end-of-stack
@@ -382,6 +384,7 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
             }
          }
 
+         old_xsp = uregs.xsp;
          uregs.xsp = uregs.xbp + sizeof(Addr) /*saved %ebp*/ 
                                + sizeof(Addr) /*ra*/;
          uregs.xbp = (((UWord*)uregs.xbp)[0]);
@@ -393,6 +396,12 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
                if (debug) VG_(printf)("     cache FPUNWIND >2\n");
                if (debug) unwind_case = "FO";
                if (do_stats) stats.FO++;
+               if (old_xsp >= uregs.xsp) {
+                  if (debug)
+                    VG_(printf) ("     FO end of stack old_xsp %p >= xsp %p\n",
+                                 (void*)old_xsp, (void*)uregs.xsp);
+                  break;
+               }
             } else {
                fp_CF_verif_cache [hash] = xip_verified ^ CFUNWIND;
                if (debug) VG_(printf)("     cache CFUNWIND >2\n");
@@ -406,6 +415,12 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
          } else {
             if (debug) unwind_case = "FF";
             if (do_stats) stats.FF++;
+            if (old_xsp >= uregs.xsp) {
+               if (debug)
+                  VG_(printf) ("     FF end of stack old_xsp %p >= xsp %p\n",
+                               (void*)old_xsp, (void*)uregs.xsp);
+               break;
+            }
          }
          goto unwind_done;
       } else {
commit 4520d562975820aced0fda6ed503379f337da66e
Author: philippe <philippe@a5019735-40e9-0310-863c-91ae7b9d1cf9>
Date:   Wed Feb 17 22:41:14 2016 +0000

    Fix incorrect (or infinite loop) unwind on RHEL7 amd64 64 bits.
    
    Same kind of problems as explained and fixed in revision 15720:
    In some cases, unwinding always retrieves the same pc/sp/bp.
    
    Fix for 64 bits is similar: stop unwinding if the previous sp is >= new sp
    
    
    
    git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15794 a5019735-40e9-0310-863c-91ae7b9d1cf9

diff --git a/coregrind/m_stacktrace.c b/coregrind/m_stacktrace.c
index 137e780..ef4984c 100644
--- a/coregrind/m_stacktrace.c
+++ b/coregrind/m_stacktrace.c
@@ -607,16 +607,25 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
     * next function which is completely wrong.
     */
    while (True) {
+      Addr old_xsp;
 
       if (i >= max_n_ips)
          break;
 
+      old_xsp = uregs.xsp;
+
       /* Try to derive a new (ip,sp,fp) triple from the current set. */
 
       /* First off, see if there is any CFI info to hand which can
          be used. */
       if ( VG_(use_CF_info)( &uregs, fp_min, fp_max ) ) {
          if (0 == uregs.xip || 1 == uregs.xip) break;
+         if (old_xsp >= uregs.xsp) {
+            if (debug)
+               VG_(printf) ("     CF end of stack old_xsp %p >= xsp %p\n",
+                            (void*)old_xsp, (void*)uregs.xsp);
+            break;
+         }
          if (sps) sps[i] = uregs.xsp;
          if (fps) fps[i] = uregs.xbp;
          ips[i++] = uregs.xip - 1; /* -1: refer to calling insn, not the RA */
@@ -646,6 +655,12 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
          if (0 == uregs.xip || 1 == uregs.xip) break;
          uregs.xsp = uregs.xbp + sizeof(Addr) /*saved %rbp*/ 
                                + sizeof(Addr) /*ra*/;
+         if (old_xsp >= uregs.xsp) {
+            if (debug)
+               VG_(printf) ("     FF end of stack old_xsp %p >= xsp %p\n",
+                            (void*)old_xsp, (void*)uregs.xsp);
+            break;
+         }
          uregs.xbp = (((UWord*)uregs.xbp)[0]);
          if (sps) sps[i] = uregs.xsp;
          if (fps) fps[i] = uregs.xbp;