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