Mark Wielaard 120af0
commit f250c4d3241c156f8e65398e2af76e3e2ee1ccb5
Mark Wielaard 120af0
Author: philippe <philippe@a5019735-40e9-0310-863c-91ae7b9d1cf9>
Mark Wielaard 120af0
Date:   Wed Nov 18 20:56:55 2015 +0000
Mark Wielaard 120af0
Mark Wielaard 120af0
    Fix incorrect (or infinite loop) unwind on RHEL7 x86 32 bits.
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    On RHEL7 x86 32 bits, Valgrind unwinder cannot properly unwind
Mark Wielaard 120af0
    the stack just after a thread creation : the unwinder always retrieves
Mark Wielaard 120af0
    the same pc/sp/bp.
Mark Wielaard 120af0
    See below for an example.
Mark Wielaard 120af0
    This has as consequences that some stack traces are bigger than
Mark Wielaard 120af0
    needed (i.e. they always fill up the ips array). If
Mark Wielaard 120af0
    --merge-recursive-frames is given, then the unwinder enters in an
Mark Wielaard 120af0
    infinite loop (as identical frames will be merged, and the ips array
Mark Wielaard 120af0
    will never be filled in).
Mark Wielaard 120af0
    Thi patch adds an additional exit condition : after unwinding
Mark Wielaard 120af0
    a frame, if the previous sp is >= new sp, then unwinding stops.
Mark Wielaard 120af0
    Patch has been tested on debian 8/x86, RHEL7/x86.
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    
Mark Wielaard 120af0
       0x0417db67 <+55>:    mov    0x18(%esp),%ebx
Mark Wielaard 120af0
       0x0417db6b <+59>:    mov    0x28(%esp),%edi
Mark Wielaard 120af0
       0x0417db6f <+63>:    mov    $0x78,%eax
Mark Wielaard 120af0
       0x0417db74 <+68>:    mov    %ebx,(%ecx)
Mark Wielaard 120af0
       0x0417db76 <+70>:    int    $0x80
Mark Wielaard 120af0
    => 0x0417db78 <+72>:    pop    %edi
Mark Wielaard 120af0
       0x0417db79 <+73>:    pop    %esi
Mark Wielaard 120af0
       0x0417db7a <+74>:    pop    %ebx
Mark Wielaard 120af0
       0x0417db7b <+75>:    test   %eax,%eax
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    Valgrind stacktrace gives:
Mark Wielaard 120af0
    ==21261==    at 0x417DB78: clone (clone.S:110)
Mark Wielaard 120af0
    ==21261==    by 0x424702F: ???
Mark Wielaard 120af0
    ==21261==    by 0x424702F: ???
Mark Wielaard 120af0
    ==21261==    by 0x424702F: ???
Mark Wielaard 120af0
    ==21261==    by 0x424702F: ???
Mark Wielaard 120af0
    ==21261==    by 0x424702F: ???
Mark Wielaard 120af0
    ==21261==    by 0x424702F: ???
Mark Wielaard 120af0
    ==21261==    by 0x424702F: ???
Mark Wielaard 120af0
    ...
Mark Wielaard 120af0
    (till the array of ips is full)
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    while gdb stacktrace gives:
Mark Wielaard 120af0
    (gdb) bt
Mark Wielaard 120af0
    #0  clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:110
Mark Wielaard 120af0
    #1  0x00000000 in ?? ()
Mark Wielaard 120af0
    (gdb) p $pc
Mark Wielaard 120af0
    $2 = (void (*)()) 0x417db78 <clone+72>
Mark Wielaard 120af0
    (gdb)
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    With the fix, valgrind gives:
Mark Wielaard 120af0
    ==21261==    at 0x417DB78: clone (clone.S:110)
Mark Wielaard 120af0
    ==21261==    by 0x424702F: ???
Mark Wielaard 120af0
    which looks more reasonable.
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    
Mark Wielaard 120af0
    git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15729 a5019735-40e9-0310-863c-91ae7b9d1cf9
Mark Wielaard 120af0
Mark Wielaard 120af0
diff --git a/coregrind/m_stacktrace.c b/coregrind/m_stacktrace.c
Mark Wielaard 120af0
index 8c1e9a4..137e780 100644
Mark Wielaard 120af0
--- a/coregrind/m_stacktrace.c
Mark Wielaard 120af0
+++ b/coregrind/m_stacktrace.c
Mark Wielaard 120af0
@@ -350,6 +350,8 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
Mark Wielaard 120af0
           uregs.xbp <= fp_max - 1 * sizeof(UWord)/*see comment below*/ &&
Mark Wielaard 120af0
           VG_IS_4_ALIGNED(uregs.xbp))
Mark Wielaard 120af0
       {
Mark Wielaard 120af0
+         Addr old_xsp;
Mark Wielaard 120af0
+
Mark Wielaard 120af0
          /* fp looks sane, so use it. */
Mark Wielaard 120af0
          uregs.xip = (((UWord*)uregs.xbp)[1]);
Mark Wielaard 120af0
          // We stop if we hit a zero (the traditional end-of-stack
Mark Wielaard 120af0
@@ -382,6 +384,7 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
Mark Wielaard 120af0
             }
Mark Wielaard 120af0
          }
Mark Wielaard 120af0
 
Mark Wielaard 120af0
+         old_xsp = uregs.xsp;
Mark Wielaard 120af0
          uregs.xsp = uregs.xbp + sizeof(Addr) /*saved %ebp*/ 
Mark Wielaard 120af0
                                + sizeof(Addr) /*ra*/;
Mark Wielaard 120af0
          uregs.xbp = (((UWord*)uregs.xbp)[0]);
Mark Wielaard 120af0
@@ -393,6 +396,12 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
Mark Wielaard 120af0
                if (debug) VG_(printf)("     cache FPUNWIND >2\n");
Mark Wielaard 120af0
                if (debug) unwind_case = "FO";
Mark Wielaard 120af0
                if (do_stats) stats.FO++;
Mark Wielaard 120af0
+               if (old_xsp >= uregs.xsp) {
Mark Wielaard 120af0
+                  if (debug)
Mark Wielaard 120af0
+                    VG_(printf) ("     FO end of stack old_xsp %p >= xsp %p\n",
Mark Wielaard 120af0
+                                 (void*)old_xsp, (void*)uregs.xsp);
Mark Wielaard 120af0
+                  break;
Mark Wielaard 120af0
+               }
Mark Wielaard 120af0
             } else {
Mark Wielaard 120af0
                fp_CF_verif_cache [hash] = xip_verified ^ CFUNWIND;
Mark Wielaard 120af0
                if (debug) VG_(printf)("     cache CFUNWIND >2\n");
Mark Wielaard 120af0
@@ -406,6 +415,12 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
Mark Wielaard 120af0
          } else {
Mark Wielaard 120af0
             if (debug) unwind_case = "FF";
Mark Wielaard 120af0
             if (do_stats) stats.FF++;
Mark Wielaard 120af0
+            if (old_xsp >= uregs.xsp) {
Mark Wielaard 120af0
+               if (debug)
Mark Wielaard 120af0
+                  VG_(printf) ("     FF end of stack old_xsp %p >= xsp %p\n",
Mark Wielaard 120af0
+                               (void*)old_xsp, (void*)uregs.xsp);
Mark Wielaard 120af0
+               break;
Mark Wielaard 120af0
+            }
Mark Wielaard 120af0
          }
Mark Wielaard 120af0
          goto unwind_done;
Mark Wielaard 120af0
       } else {
Mark Wielaard c283c1
commit 4520d562975820aced0fda6ed503379f337da66e
Mark Wielaard c283c1
Author: philippe <philippe@a5019735-40e9-0310-863c-91ae7b9d1cf9>
Mark Wielaard c283c1
Date:   Wed Feb 17 22:41:14 2016 +0000
Mark Wielaard c283c1
Mark Wielaard c283c1
    Fix incorrect (or infinite loop) unwind on RHEL7 amd64 64 bits.
Mark Wielaard c283c1
    
Mark Wielaard c283c1
    Same kind of problems as explained and fixed in revision 15720:
Mark Wielaard c283c1
    In some cases, unwinding always retrieves the same pc/sp/bp.
Mark Wielaard c283c1
    
Mark Wielaard c283c1
    Fix for 64 bits is similar: stop unwinding if the previous sp is >= new sp
Mark Wielaard c283c1
    
Mark Wielaard c283c1
    
Mark Wielaard c283c1
    
Mark Wielaard c283c1
    git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15794 a5019735-40e9-0310-863c-91ae7b9d1cf9
Mark Wielaard c283c1
Mark Wielaard c283c1
diff --git a/coregrind/m_stacktrace.c b/coregrind/m_stacktrace.c
Mark Wielaard c283c1
index 137e780..ef4984c 100644
Mark Wielaard c283c1
--- a/coregrind/m_stacktrace.c
Mark Wielaard c283c1
+++ b/coregrind/m_stacktrace.c
Mark Wielaard c283c1
@@ -607,16 +607,25 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
Mark Wielaard c283c1
     * next function which is completely wrong.
Mark Wielaard c283c1
     */
Mark Wielaard c283c1
    while (True) {
Mark Wielaard c283c1
+      Addr old_xsp;
Mark Wielaard c283c1
 
Mark Wielaard c283c1
       if (i >= max_n_ips)
Mark Wielaard c283c1
          break;
Mark Wielaard c283c1
 
Mark Wielaard c283c1
+      old_xsp = uregs.xsp;
Mark Wielaard c283c1
+
Mark Wielaard c283c1
       /* Try to derive a new (ip,sp,fp) triple from the current set. */
Mark Wielaard c283c1
 
Mark Wielaard c283c1
       /* First off, see if there is any CFI info to hand which can
Mark Wielaard c283c1
          be used. */
Mark Wielaard c283c1
       if ( VG_(use_CF_info)( &uregs, fp_min, fp_max ) ) {
Mark Wielaard c283c1
          if (0 == uregs.xip || 1 == uregs.xip) break;
Mark Wielaard c283c1
+         if (old_xsp >= uregs.xsp) {
Mark Wielaard c283c1
+            if (debug)
Mark Wielaard c283c1
+               VG_(printf) ("     CF end of stack old_xsp %p >= xsp %p\n",
Mark Wielaard c283c1
+                            (void*)old_xsp, (void*)uregs.xsp);
Mark Wielaard c283c1
+            break;
Mark Wielaard c283c1
+         }
Mark Wielaard c283c1
          if (sps) sps[i] = uregs.xsp;
Mark Wielaard c283c1
          if (fps) fps[i] = uregs.xbp;
Mark Wielaard c283c1
          ips[i++] = uregs.xip - 1; /* -1: refer to calling insn, not the RA */
Mark Wielaard c283c1
@@ -646,6 +655,12 @@ UInt VG_(get_StackTrace_wrk) ( ThreadId tid_if_known,
Mark Wielaard c283c1
          if (0 == uregs.xip || 1 == uregs.xip) break;
Mark Wielaard c283c1
          uregs.xsp = uregs.xbp + sizeof(Addr) /*saved %rbp*/ 
Mark Wielaard c283c1
                                + sizeof(Addr) /*ra*/;
Mark Wielaard c283c1
+         if (old_xsp >= uregs.xsp) {
Mark Wielaard c283c1
+            if (debug)
Mark Wielaard c283c1
+               VG_(printf) ("     FF end of stack old_xsp %p >= xsp %p\n",
Mark Wielaard c283c1
+                            (void*)old_xsp, (void*)uregs.xsp);
Mark Wielaard c283c1
+            break;
Mark Wielaard c283c1
+         }
Mark Wielaard c283c1
          uregs.xbp = (((UWord*)uregs.xbp)[0]);
Mark Wielaard c283c1
          if (sps) sps[i] = uregs.xsp;
Mark Wielaard c283c1
          if (fps) fps[i] = uregs.xbp;