This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[binutils-gdb] x32: gdb: Fix 'call' insn relocation with qRelocInsn


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f077e978deccac00fea013c4f120122bf6726834

commit f077e978deccac00fea013c4f120122bf6726834
Author: Pedro Alves <palves@redhat.com>
Date:   Fri Aug 19 12:07:46 2016 +0100

    x32: gdb: Fix 'call' insn relocation with qRelocInsn
    
    Running the fast tracepoints tests against x32 gdbserver exposes a
    latent bug.  E.g.,:
    
     (gdb)
     continue
     Continuing.
     Reading /media/sf_host-pedro/gdb/mygit/build-ubuntu-x32/gdb/testsuite/outputs/gdb.trace/change-loc/change-loc-2.sl from remote target...
    
     Thread 1 "change-loc" received signal SIGSEGV, Segmentation fault.
     func4 () at /home/pedro/gdb/src/gdb/testsuite/gdb.trace/change-loc.h:24
     24      }
     (gdb) FAIL: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 2
    
    The test sets a fast tracepoint on a shared library.  On x32, shared
    libraries end up loaded somewhere in the upper 2GB of the 4GB address
    space x32 has access to.  When gdbserver needs to copy an instruction
    to execute it in the jump pad, it asks gdb to relocate/adjust it, with
    the qRelocInsn packet.  gdb converts "call" instructions into a "push
    $<2GB-4GB addr> + jmp" sequence, however, the "pushq" instruction sign
    extends its operand, so later when the called function returns, it
    returns to an incorrectly sign-extended address.  E.g.,
    0xfffffffffabc0000 instead of 0xfabc0000, resulting in the
    segmentation fault.
    
    Fix this by converting calls at such addresses to "sub + mov + jmp"
    sequences instead.
    
    gdb/ChangeLog:
    2016-08-19  Pedro Alves  <palves@redhat.com>
    
    	* amd64-tdep.c (amd64_relocate_instruction) <callq>: Handle return
    	addresses over 0x7fffffff.

Diff:
---
 gdb/ChangeLog    |  5 +++++
 gdb/amd64-tdep.c | 42 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4a70890..b706e71 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-08-19  Pedro Alves  <palves@redhat.com>
+
+	* amd64-tdep.c (amd64_relocate_instruction) <callq>: Handle return
+	addresses over 0x7fffffff.
+
 2016-08-18  Carl Love  <cel@us.ibm.com>
 
 	* MAINTANERS Write After Approval): Add "Carl Love".
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 6289d21..41b9783 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -1764,15 +1764,47 @@ amd64_relocate_instruction (struct gdbarch *gdbarch,
      the user program would return to.  */
   if (insn[0] == 0xe8)
     {
-      gdb_byte push_buf[16];
-      unsigned int ret_addr;
+      gdb_byte push_buf[32];
+      CORE_ADDR ret_addr;
+      int i = 0;
 
       /* Where "ret" in the original code will return to.  */
       ret_addr = oldloc + insn_length;
-      push_buf[0] = 0x68; /* pushq $...  */
-      store_unsigned_integer (&push_buf[1], 4, byte_order, ret_addr);
+
+      /* If pushing an address higher than or equal to 0x80000000,
+	 avoid 'pushq', as that sign extends its 32-bit operand, which
+	 would be incorrect.  */
+      if (ret_addr <= 0x7fffffff)
+	{
+	  push_buf[0] = 0x68; /* pushq $...  */
+	  store_unsigned_integer (&push_buf[1], 4, byte_order, ret_addr);
+	  i = 5;
+	}
+      else
+	{
+	  push_buf[i++] = 0x48; /* sub    $0x8,%rsp */
+	  push_buf[i++] = 0x83;
+	  push_buf[i++] = 0xec;
+	  push_buf[i++] = 0x08;
+
+	  push_buf[i++] = 0xc7; /* movl    $imm,(%rsp) */
+	  push_buf[i++] = 0x04;
+	  push_buf[i++] = 0x24;
+	  store_unsigned_integer (&push_buf[i], 4, byte_order,
+				  ret_addr & 0xffffffff);
+	  i += 4;
+
+	  push_buf[i++] = 0xc7; /* movl    $imm,4(%rsp) */
+	  push_buf[i++] = 0x44;
+	  push_buf[i++] = 0x24;
+	  push_buf[i++] = 0x04;
+	  store_unsigned_integer (&push_buf[i], 4, byte_order,
+				  ret_addr >> 32);
+	  i += 4;
+	}
+      gdb_assert (i <= sizeof (push_buf));
       /* Push the push.  */
-      append_insns (to, 5, push_buf);
+      append_insns (to, i, push_buf);
 
       /* Convert the relative call to a relative jump.  */
       insn[0] = 0xe9;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]