File gdb-x86_64-i386-syscall-restart-master.patch of Package gdb

Overview Repositories Revisions Requests Users Attributes Meta

File gdb-x86_64-i386-syscall-restart-master.patch of Package gdb (Revision e53ef9283c5edf7a18a9502157753ce2)

Currently displaying revision e53ef9283c5edf7a18a9502157753ce2 , Show latest

commit 3f52fdbcb599f76b4838020721ca6c9f1cc28f84
Author: Kevin Buettner <kevinb@redhat.com>
Date:   Sat Mar 16 12:40:01 2019 -0700

Fix amd64->i386 linux syscall restart problem
    
    This commit fixes some failures in gdb.base/interrupt.exp
    when debugging a 32-bit i386 linux inferior from an amd64 host.
    
    When running the following test...
    
      make check RUNTESTFLAGS="--target_board unix/-m32 interrupt.exp"
    
    ... without this commit, I see the following output:
    
    FAIL: gdb.base/interrupt.exp: continue (the program exited)
    FAIL: gdb.base/interrupt.exp: echo data
    FAIL: gdb.base/interrupt.exp: Send Control-C, second time
    FAIL: gdb.base/interrupt.exp: signal SIGINT (the program is no longer running)
    ERROR: Undefined command "".
    ERROR: GDB process no longer exists
    
                    === gdb Summary ===
    
    When the test is run with this commit in place, we see 12 passes
    instead.  This is the desired behavior.
    
    Analysis:
    
    On Linux, when a syscall is interrupted by a signal, the syscall
    may return -ERESTARTSYS when a signal occurs.  Doing so indicates that
    the syscall is restartable.  Then, depending on settings associated
    with the signal handler, and after the signal handler is called, the
    kernel can then either return -EINTR or can cause the syscall to be
    restarted.  In this discussion, we are concerned with the latter
    case.
    
    On i386, the kernel returns this status via the EAX register.
    
    When debugging a 32-bit (i386) process from a 64-bit (amd64)
    GDB, the debugger fetches 64-bit registers even though the
    process being debugged is 32-bit.  Since we're debugging a 32-bit
    target, only 32 bits are being saved in the register cache.
    Now, ideally, GDB would save all 64-bits in the regcache and
    then would be able to restore those same values when it comes
    time to continue the target.  I've looked into doing this, but
    it's not easy and I don't see many benefits to doing so.  One
    benefit, however, would be that EAX would appear as a negative
    value for doing syscall restarts.
    
    At the moment, GDB is setting the high 32 bits of RAX (and other
    registers too) to 0.  So, when GDB restores EAX just prior to
    a syscall restart, the high 32 bits of RAX are zeroed, thus making
    it look like a positive value.  For this particular purpose, we
    need to sign extend EAX so that RAX will appear as a negative
    value when EAX is set to -ERESTARTSYS.  This in turn will cause
    the signal handling code in the kernel to recognize -ERESTARTSYS
    which will in turn cause the syscall to be restarted.
    
    This commit is based on work by Jan Kratochvil from 2009:
    
    https://sourceware.org/ml/gdb-patches/2009-11/msg00592.html
    
    Jan's patch had the sign extension code in amd64-nat.c.  Several
    other native targets make use of this code, so it seemed better
    to move the sign extension code to a linux specific file.  I
    also added similar code to gdbserver.
    
    Another approach is to fix the problem in the kernel.  Hui Zhu
    tried to get a fix into the kernel back in 2014, but it was not
    accepted.  Discussion regarding this approach may be found here:
    
    https://lore.kernel.org/patchwork/patch/457841/
    
    Even if a fix were to be put into the kernel, we'd still need
    some kind of fix in GDB in order to support older kernels.
    
    Finally, I'll note that Fedora has been carrying a similar patch for
    at least nine years.  Other distributions, including RHEL and CentOS
    have picked up this change and have been using it too.
    
    gdb/ChangeLog:
    
            * amd64-linux-nat.c (amd64_linux_collect_native_gregset): New
            function.
            (fill_gregset): Call amd64_linux_collect_native_gregset instead
            of amd64_collect_native_gregset.
            (amd64_linux_nat_target::store_registers): Likewise.
    
    gdb/gdbserver/ChangeLog:
    
            * linux-x86-low.c (x86_fill_gregset): Sign extend EAX value
            when using a 64-bit gdbserver.

diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
index a0bb105f5a..8d0e8eb35c 100644
--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -92,6 +92,71 @@ static int amd64_linux_gregset32_reg_offset[] =
 /* Transfering the general-purpose registers between GDB, inferiors
    and core files.  */
 
+/* See amd64_collect_native_gregset.  This linux specific version handles
+   issues with negative EAX values not being restored correctly upon syscall
+   return when debugging 32-bit targets.  It has no effect on 64-bit
+   targets.  */
+
+static void
+amd64_linux_collect_native_gregset (const struct regcache *regcache,
+			            void *gregs, int regnum)
+{
+  amd64_collect_native_gregset (regcache, gregs, regnum);
+
+  struct gdbarch *gdbarch = regcache->arch ();
+  if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word == 32)
+    {
+      /* Sign extend EAX value to avoid potential syscall restart
+	 problems.  
+
+	 On Linux, when a syscall is interrupted by a signal, the
+	 (kernel function implementing the) syscall may return
+	 -ERESTARTSYS when a signal occurs.  Doing so indicates that
+	 the syscall is restartable.  Then, depending on settings
+	 associated with the signal handler, and after the signal
+	 handler is called, the kernel can then either return -EINTR
+	 or it can cause the syscall to be restarted.  We are
+	 concerned with the latter case here.
+	 
+	 On (32-bit) i386, the status (-ERESTARTSYS) is placed in the
+	 EAX register.  When debugging a 32-bit process from a 64-bit
+	 (amd64) GDB, the debugger fetches 64-bit registers even
+	 though the process being debugged is only 32-bit.  The
+	 register cache is only 32 bits wide though; GDB discards the
+	 high 32 bits when placing 64-bit values in the 32-bit
+	 regcache.  Normally, this is not a problem since the 32-bit
+	 process should only care about the lower 32-bit portions of
+	 these registers.  That said, it can happen that the 64-bit
+	 value being restored will be different from the 64-bit value
+	 that was originally retrieved from the kernel.  The one place
+	 (that we know of) where it does matter is in the kernel's
+	 syscall restart code.  The kernel's code for restarting a
+	 syscall after a signal expects to see a negative value
+	 (specifically -ERESTARTSYS) in the 64-bit RAX register in
+	 order to correctly cause a syscall to be restarted.
+	 
+	 The call to amd64_collect_native_gregset, above, is setting
+	 the high 32 bits of RAX (and other registers too) to 0.  For
+	 syscall restart, we need to sign extend EAX so that RAX will
+	 appear as a negative value when EAX is set to -ERESTARTSYS. 
+	 This in turn will cause the signal handling code in the
+	 kernel to recognize -ERESTARTSYS which will in turn cause the
+	 syscall to be restarted.
+
+	 The test case gdb.base/interrupt.exp tests for this problem.
+	 Without this sign extension code in place, it'll show
+	 a number of failures when testing against unix/-m32.  */
+
+      if (regnum == -1 || regnum == I386_EAX_REGNUM)
+	{
+	  void *ptr = ((gdb_byte *) gregs 
+		       + amd64_linux_gregset32_reg_offset[I386_EAX_REGNUM]);
+
+	  *(int64_t *) ptr = *(int32_t *) ptr;
+	}
+    }
+}
+
 /* Fill GDB's register cache with the general-purpose register values
    in *GREGSETP.  */
 
@@ -109,7 +174,7 @@ void
 fill_gregset (const struct regcache *regcache,
 	      elf_gregset_t *gregsetp, int regnum)
 {
-  amd64_collect_native_gregset (regcache, gregsetp, regnum);
+  amd64_linux_collect_native_gregset (regcache, gregsetp, regnum);
 }
 
 /* Transfering floating-point registers between GDB, inferiors and cores.  */
@@ -237,7 +302,7 @@ amd64_linux_nat_target::store_registers (struct regcache *regcache, int regnum)
       if (ptrace (PTRACE_GETREGS, tid, 0, (long) &regs) < 0)
 	perror_with_name (_("Couldn't get registers"));
 
-      amd64_collect_native_gregset (regcache, &regs, regnum);
+      amd64_linux_collect_native_gregset (regcache, &regs, regnum);
 
       if (ptrace (PTRACE_SETREGS, tid, 0, (long) &regs) < 0)
 	perror_with_name (_("Couldn't write registers"));
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 029796e361..dd7673126e 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -338,6 +338,19 @@ x86_fill_gregset (struct regcache *regcache, void *buf)
 
   collect_register_by_name (regcache, "orig_eax",
 			    ((char *) buf) + ORIG_EAX * REGSIZE);
+
+  /* Sign extend EAX value to avoid potential syscall restart
+     problems. 
+
+     See amd64_linux_collect_native_gregset() in gdb/amd64-linux-nat.c
+     for a detailed explanation.  */
+  if (register_size (regcache->tdesc, 0) == 4)
+    {
+      void *ptr = ((gdb_byte *) buf
+                   + i386_regmap[find_regno (regcache->tdesc, "eax")]);
+
+      *(int64_t *) ptr = *(int32_t *) ptr;
+    }
 }
 
 static void

Places

File gdb-x86_64-i386-syscall-restart-master.patch of Package gdb (Revision e53ef9283c5edf7a18a9502157753ce2)

Places