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 {