[PATCH 3/6] x32: Avoid unsigned long when installing fast tracepoint jump pads

[PATCH 3/6] x32: Avoid unsigned long when installing fast tracepoint jump pads

[Pedro Alves]

We're casting through unsigned long to write a 64-bit immediate
operand of movabs (the comment said movl, but that was incorrect).
The problem is that unsigned long is 32-bit on x32, so we were writing
fewer bytes than necessary.

Fix this by using an 8 byte memcpy like in other similar places in the
function.

gdb/gdbserver/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* linux-x86-low.c (amd64_install_fast_tracepoint_jump_pad): Fix
	comment.  Use memcpy instead of casting through unsigned long.
---
 gdb/gdbserver/linux-x86-low.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index d6b67c1..1ba98ba 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -1092,10 +1092,10 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
   buf[i++] = 0x41; buf[i++] = 0x51; /* push %r9 */
   buf[i++] = 0x41; buf[i++] = 0x50; /* push %r8 */
   buf[i++] = 0x9c; /* pushfq */
-  buf[i++] = 0x48; /* movl <addr>,%rdi */
+  buf[i++] = 0x48; /* movabs <addr>,%rdi */
   buf[i++] = 0xbf;
-  *((unsigned long *)(buf + i)) = (unsigned long) tpaddr;
-  i += sizeof (unsigned long);
+  memcpy (buf + i, &tpaddr, 8);
+  i += 8;
   buf[i++] = 0x57; /* push %rdi */
   append_insns (&buildaddr, i, buf);
 
-- 
2.5.5

[PATCH 2/6] x32 Fast tracepoints: Customize jump pad address

[Pedro Alves]

MAP_32BIT is ignored on x32, meaning the jump pad can end up somewhere
between 2GB and 4GB, too far away from the executable for 5-byte
relative jumps (JMP rel32).  So on x32, try explicitly placing the
jump pad near the middle of the available address space.

gdb/gdbserver/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* linux-amd64-ipa.c (alloc_jump_pad_buffer) [__ILP32__]: Try
	allocating around 0x80000000.
---
 gdb/gdbserver/linux-amd64-ipa.c | 52 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 4 deletions(-)

diff --git a/gdb/gdbserver/linux-amd64-ipa.c b/gdb/gdbserver/linux-amd64-ipa.c
index 15d08ff..0625b85 100644
--- a/gdb/gdbserver/linux-amd64-ipa.c
+++ b/gdb/gdbserver/linux-amd64-ipa.c
@@ -206,14 +206,57 @@ get_ipa_tdesc (int idx)
 		  "unknown ipa tdesc index: %d", idx);
 }
 
-/* Allocate buffer for the jump pads.  Since we're using 32-bit jumps
-   to reach them, and the executable is at low addresses, MAP_32BIT
-   works just fine.  Shared libraries, being allocated at the top,
-   are unfortunately out of luck.  */
+/* Allocate buffer for the jump pads.  The branch instruction has a
+   reach of +/- 31-bit, and the executable is loaded at low addresses.
+
+   64-bit: Use MAP_32BIT to allocate in the first 2GB.  Shared
+   libraries, being allocated at the top, are unfortunately out of
+   luck.
+
+   x32: Since MAP_32BIT is 64-bit only, do the placement manually.
+   Try allocating at '0x80000000 - SIZE' initially, decreasing until
+   we hit a free area.  This ensures the executable is fully covered,
+   and is as close as possible to the shared libraries, which are
+   usually mapped at the top of the first 4GB of the address space.
+*/
 
 void *
 alloc_jump_pad_buffer (size_t size)
 {
+#if __ILP32__
+  uintptr_t addr;
+  int pagesize;
+
+  pagesize = sysconf (_SC_PAGE_SIZE);
+  if (pagesize == -1)
+    perror_with_name ("sysconf");
+
+  addr = 0x80000000 - size;
+
+  /* size should already be page-aligned, but this can't hurt.  */
+  addr &= ~(pagesize - 1);
+
+  /* Search for a free area.  If we hit 0, we're out of luck.  */
+  for (; addr; addr -= pagesize)
+    {
+      void *res;
+
+      /* No MAP_FIXED - we don't want to zap someone's mapping.  */
+      res = mmap ((void *) addr, size,
+		  PROT_READ | PROT_WRITE | PROT_EXEC,
+		  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+
+      /* If we got what we wanted, return.  */
+      if ((uintptr_t) res == addr)
+	return res;
+
+      /* If we got a mapping, but at a wrong address, undo it.  */
+      if (res != MAP_FAILED)
+	munmap (res, size);
+    }
+
+  return NULL;
+#else
   void *res = mmap (NULL, size, PROT_READ | PROT_WRITE | PROT_EXEC,
 		    MAP_PRIVATE | MAP_ANONYMOUS | MAP_32BIT, -1, 0);
 
@@ -221,6 +264,7 @@ alloc_jump_pad_buffer (size_t size)
     return NULL;
 
   return res;
+#endif
 }
 
 void
-- 
2.5.5

[PATCH 1/6] x32 Fast tracepoints: IPA target descriptions

[Pedro Alves]

Building GDB for x32 fails building the IPA, with:

   .../src/gdb/gdbserver/linux-amd64-ipa.c: In function ‘const target_desc* get_ipa_tdesc(int)’:
   .../src/gdb/gdbserver/linux-amd64-ipa.c:182:14: error: ‘tdesc_amd64_avx_linux’ was not declared in this scope
	  return tdesc_amd64_avx_linux;
		 ^
   .../src/gdb/gdbserver/linux-amd64-ipa.c:184:14: error: ‘tdesc_amd64_mpx_linux’ was not declared in this scope
	  return tdesc_amd64_mpx_linux;
		 ^
   .../src/gdb/gdbserver/linux-amd64-ipa.c:186:14: error: ‘tdesc_amd64_avx_mpx_linux’ was not declared in this scope
	  return tdesc_amd64_avx_mpx_linux;
		 ^
  [...]

The problem is that the IPA is trying to use the 64-bit descriptions,
when it should be using the x32 ones.

gdb/gdbserver/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* Makefile.in (x32-linux-ipa.o, x32-avx-linux-ipa.o)
	(x32-avx512-linux-ipa.o): New rules.
	* configure.ac (x86_64-*-linux*): New x32 check.
	* configure.srv (ipa_x32_linux_regobj): New.
	(x86_64-*-linux*): Use $ipa_x32_linux_regobj if building for x32.
	* linux-amd64-ipa.c (get_ipa_tdesc) [__ILP32__]: Return x32
	descriptions.
	(initialize_low_tracepoint) [__ILP32__]: Initialize x32
	descriptions.
	* configure: Regenerate.
---
 gdb/gdbserver/Makefile.in       |  9 +++++++++
 gdb/gdbserver/configure         | 31 +++++++++++++++++++++++++++++++
 gdb/gdbserver/configure.ac      | 14 ++++++++++++++
 gdb/gdbserver/configure.srv     |  8 +++++++-
 gdb/gdbserver/linux-amd64-ipa.c | 27 +++++++++++++++++++++++----
 5 files changed, 84 insertions(+), 5 deletions(-)

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 2be61ef..dd96213 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -647,6 +647,15 @@ rsp-low-ipa.o: ../common/rsp-low.c
 errors-ipa.o: ../common/errors.c
 	$(IPAGENT_COMPILE) $<
 	$(POSTCOMPILE)
+x32-linux-ipa.o: x32-linux.c
+	$(IPAGENT_COMPILE) $<
+	$(POSTCOMPILE)
+x32-avx-linux-ipa.o: x32-avx-linux.c
+	$(IPAGENT_COMPILE) $<
+	$(POSTCOMPILE)
+x32-avx512-linux-ipa.o: x32-avx512-linux.c
+	$(IPAGENT_COMPILE) $<
+	$(POSTCOMPILE)
 
 ax.o: ax.c
 	$(COMPILE) $(WARN_CFLAGS_NO_FORMAT) $<
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 2926deb..3dd5a32 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -6795,6 +6795,37 @@ fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $gdb_cv_i386_is_x86_64" >&5
 $as_echo "$gdb_cv_i386_is_x86_64" >&6; }
     ;;
+
+  x86_64-*-linux*)
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking if building for x32" >&5
+$as_echo_n "checking if building for x32... " >&6; }
+if test "${gdb_cv_x86_is_x32+set}" = set; then :
+  $as_echo_n "(cached) " >&6
+else
+  save_CPPFLAGS="$CPPFLAGS"
+		    CPPFLAGS="$CPPFLAGS $CFLAGS"
+		    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+#if __x86_64__ && __ILP32__
+got it
+#endif
+
+_ACEOF
+if (eval "$ac_cpp conftest.$ac_ext") 2>&5 |
+  $EGREP "got it" >/dev/null 2>&1; then :
+  gdb_cv_x86_is_x32=yes
+else
+  gdb_cv_x86_is_x32=no
+fi
+rm -f conftest*
+
+		    CPPFLAGS="$save_CPPFLAGS"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $gdb_cv_x86_is_x32" >&5
+$as_echo "$gdb_cv_x86_is_x32" >&6; }
+    ;;
+
   m68k-*-*)
     { $as_echo "$as_me:${as_lineno-$LINENO}: checking if building for Coldfire" >&5
 $as_echo_n "checking if building for Coldfire... " >&6; }
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index 627c590..01ec634 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -218,6 +218,20 @@ got it
                     [gdb_cv_i386_is_x86_64=no])
                     CPPFLAGS="$save_CPPFLAGS"])
     ;;
+
+  x86_64-*-linux*)
+    AC_CACHE_CHECK([if building for x32], [gdb_cv_x86_is_x32],
+		   [save_CPPFLAGS="$CPPFLAGS"
+		    CPPFLAGS="$CPPFLAGS $CFLAGS"
+		    AC_EGREP_CPP([got it], [
+#if __x86_64__ && __ILP32__
+got it
+#endif
+		 ], [gdb_cv_x86_is_x32=yes],
+		    [gdb_cv_x86_is_x32=no])
+		    CPPFLAGS="$save_CPPFLAGS"])
+    ;;
+
   m68k-*-*)
     AC_CACHE_CHECK([if building for Coldfire], [gdb_cv_m68k_is_coldfire],
       	           [save_CPPFLAGS="$CPPFLAGS"
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index fe53e03..39e8392 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -31,6 +31,7 @@ srv_amd64_linux_regobj="amd64-linux.o amd64-avx-linux.o amd64-avx512-linux.o amd
 
 ipa_i386_linux_regobj="i386-linux-ipa.o i386-avx-linux-ipa.o  i386-avx-mpx-linux-ipa.o i386-avx512-linux-ipa.o i386-mpx-linux-ipa.o i386-mmx-linux-ipa.o"
 ipa_amd64_linux_regobj="amd64-linux-ipa.o amd64-avx-linux-ipa.o amd64-avx-mpx-linux-ipa.o amd64-avx512-linux-ipa.o amd64-mpx-linux-ipa.o"
+ipa_x32_linux_regobj="x32-linux-ipa.o x32-avx-linux-ipa.o x32-avx512-linux-ipa.o"
 ipa_ppc_linux_regobj="powerpc-32l-ipa.o powerpc-altivec32l-ipa.o powerpc-cell32l-ipa.o powerpc-vsx32l-ipa.o powerpc-isa205-32l-ipa.o powerpc-isa205-altivec32l-ipa.o powerpc-isa205-vsx32l-ipa.o powerpc-e500l-ipa.o powerpc-64l-ipa.o powerpc-altivec64l-ipa.o powerpc-cell64l-ipa.o powerpc-vsx64l-ipa.o powerpc-isa205-64l-ipa.o powerpc-isa205-altivec64l-ipa.o powerpc-isa205-vsx64l-ipa.o"
 
 srv_i386_32bit_xmlfiles="i386/32bit-core.xml i386/32bit-sse.xml i386/32bit-avx.xml i386/32bit-avx512.xml i386/32bit-mpx.xml"
@@ -364,7 +365,12 @@ case "${target}" in
 			srv_linux_regsets=yes
 			srv_linux_thread_db=yes
 			srv_linux_btrace=yes
-			ipa_obj="${ipa_amd64_linux_regobj} linux-amd64-ipa.o"
+			if test "$gdb_cv_x86_is_x32" = yes ; then
+			    ipa_obj="${ipa_x32_linux_regobj}"
+			else
+			    ipa_obj="${ipa_amd64_linux_regobj}"
+			fi
+			ipa_obj="${ipa_obj} linux-amd64-ipa.o"
 			;;
   x86_64-*-mingw*)	srv_regobj="$srv_amd64_regobj"
 			srv_tgtobj="x86-low.o x86-dregs.o i387-fp.o win32-low.o win32-i386-low.o"
diff --git a/gdb/gdbserver/linux-amd64-ipa.c b/gdb/gdbserver/linux-amd64-ipa.c
index 4997d0e..15d08ff 100644
--- a/gdb/gdbserver/linux-amd64-ipa.c
+++ b/gdb/gdbserver/linux-amd64-ipa.c
@@ -174,6 +174,19 @@ supply_static_tracepoint_registers (struct regcache *regcache,
 const struct target_desc *
 get_ipa_tdesc (int idx)
 {
+#if defined __ILP32__
+  switch (idx)
+    {
+    case X86_TDESC_SSE:
+      return tdesc_x32_linux;
+    case X86_TDESC_AVX:
+      return tdesc_x32_avx_linux;
+    case X86_TDESC_AVX512:
+      return tdesc_x32_avx512_linux;
+    default:
+      break;
+    }
+#else
   switch (idx)
     {
     case X86_TDESC_SSE:
@@ -186,11 +199,11 @@ get_ipa_tdesc (int idx)
       return tdesc_amd64_avx_mpx_linux;
     case X86_TDESC_AVX512:
       return tdesc_amd64_avx512_linux;
-    default:
-      internal_error (__FILE__, __LINE__,
-		      "unknown ipa tdesc index: %d", idx);
-      return tdesc_amd64_linux;
     }
+#endif
+
+  internal_error (__FILE__, __LINE__,
+		  "unknown ipa tdesc index: %d", idx);
 }
 
 /* Allocate buffer for the jump pads.  Since we're using 32-bit jumps
@@ -213,9 +226,15 @@ alloc_jump_pad_buffer (size_t size)
 void
 initialize_low_tracepoint (void)
 {
+#if defined __ILP32__
+  init_registers_x32_linux ();
+  init_registers_x32_avx_linux ();
+  init_registers_x32_avx512_linux ();
+#else
   init_registers_amd64_linux ();
   init_registers_amd64_avx_linux ();
   init_registers_amd64_avx_mpx_linux ();
   init_registers_amd64_mpx_linux ();
   init_registers_amd64_avx512_linux ();
+#endif
 }
-- 
2.5.5

[PATCH 6/6] x32: Fix gdb.trace/mi-trace-frame-collected.exp

[Pedro Alves]

gdb.trace/mi-trace-frame-collected.exp has a couple failures on x32:

 FAIL: gdb.trace/mi-trace-frame-collected.exp: live: -trace-frame-collected (register)
 FAIL: gdb.trace/mi-trace-frame-collected.exp: tfile: -trace-frame-collected (register)

gdb.log:

 -trace-frame-collected
 ^done,explicit-variables=[{name="gdb_char_test",value="0 '\\000'"}],computed-expressions=[],registers=[{number="16",value="0x4004dc"},{number="204",value="0x4004dc"}],tvars
 =[],memory=[{address="0x00601060",length="1"}]
 (gdb)
 FAIL: gdb.trace/mi-trace-frame-collected.exp: live: -trace-frame-collected (register)
[...]
 -trace-frame-collected
 ^done,explicit-variables=[{name="gdb_char_test",value="0 '\\000'"}],computed-expressions=[],registers=[{number="16",value="0x4004dc"},{number="204",value="0x4004dc"}],tvars
 =[],memory=[{address="0x00601060",length="1"}]
 (gdb)
 FAIL: gdb.trace/mi-trace-frame-collected.exp: tfile: -trace-frame-collected (register)

This test only collects the PC, and thus expects to only see one
register in the output of -trace-frame-collected.  However, while on
the 64-bit ABI gdb only exposes 64-bit $pc/$rip (register 16 above),
on x32, GDB exposes 32-bit $eip as well, as a pseudo-register
(register 204 above).  Thus, collecting $pc/$rip automatically always
collects $eip as well.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.trace/mi-trace-frame-collected.exp
	(test_trace_frame_collected): On x32, expect two registers.
---
 gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp b/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
index a69c329..567954a 100644
--- a/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
+++ b/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
@@ -95,15 +95,25 @@ proc test_trace_frame_collected { data_source } {
 
 	# Test MI command '-trace-frame-collected' dumps only
 	# collected registers.
+
+	# While the tracepoint has no explicit action that causes
+	# collection of registers other than the PC, some
+	# architectures manage to collect or guess more than that.
 	if { [istarget "s390*-*-*"] } {
-	    # Only PC is collected, but on s390 PC is a pseudo-register -
-	    # collecting it also collects the underlying PSWA register.
+	    # On s390 PC is a pseudo-register - collecting it also
+	    # collects the underlying PSWA register.
 	    if { "$data_source" != "tfile" } {
 		set reg_pattern "$reg_pattern,$reg_pattern"
 	    } else {
 		# For tfile, PSWM and CC are also guessed.
 		set reg_pattern "$reg_pattern,$reg_pattern,$reg_pattern,$reg_pattern"
 	    }
+	} elseif {[is_amd64_regs_target] && [is_ilp32_target]} {
+	    # x32.  While on the 64-bit ABI gdb only exposes 64-bit
+	    # $pc/$rip, on x32, GDB exposes 32-bit $eip as well, as a
+	    # pseudo-register.  Thus, collecting $pc/$rip
+	    # automatically always collects $eip as well.
+	    set reg_pattern "$reg_pattern,$reg_pattern"
 	}
 
 	mi_gdb_test "-trace-frame-collected" \
-- 
2.5.5

[PATCH 0/6] x32: Fast tracepoints support

[Pedro Alves]

Yesterday I installed an Ubuntu 16.04 virtual machine here, and
installed the x32 runtime support (basically, run "apt-cache search
x32", and install all that shows).  That made it easy to fix a few
basic x32 problems, the result of which were yesterday's x32 patches.

There's still one remaining annoying x32 problem however.  Even though
gdb and gdbserver build fine on x32 now, the build still fails, when
building the fast tracepoints in-process agent DSO (IPA), with:

   .../src/gdb/gdbserver/linux-amd64-ipa.c: In function ‘const target_desc* get_ipa_tdesc(int)’:
   .../src/gdb/gdbserver/linux-amd64-ipa.c:182:14: error: ‘tdesc_amd64_avx_linux’ was not declared in this scope
	  return tdesc_amd64_avx_linux;
		 ^
  [...]

So I took a stab at fixing it.  And then I thought I'd run the fast
tracepoint tests against the result.  And that showed a few problems,
all fixed by this series...

A diff of testing with:
 RUNTESTFLAGS="--target_board=native-gdbserver" TESTS="gdb.trace/*.exp"
vs 
 RUNTESTFLAGS="--target_board=native-gdbserver/-mx32" TESTS="gdb.trace/*.exp"

Shows that x32 support is just as good as 64-bit.  Actually, it's
better, since fast tracepoints on shared libraries work.  E.g.:

 -PASS: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 2 (too far)
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 2
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tracepoint with three locations
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 3
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tracepoint with two locations - installed (unload)
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tracepoint with two locations - pending (unload)
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tstop
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tfind frame 0
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tfind

I didn't bother to save a few bytes by making the jump pad,
gdbserver's agent bytecode JIT etc. use/emit shorter instructions that
use 32-bit operands in the places where that would be possible on x32.
Not that it'd be complicated, it's just that I've already invested
more time in this at this point than I wanted, and that I'm assuming
that it wouldn't result in a significant optimization.  I'd be glad to
be proven wrong though, if someone wants to try that out.

Pedro Alves (6):
  x32 Fast tracepoints: IPA target descriptions
  x32 Fast tracepoints: Customize jump pad address
  x32: Avoid unsigned long when installing fast tracepoint jump pads
  x32: gdbserver's agent bytecode JIT: fix "call" emission
  x32: gdb: Fix 'call' insn relocation with qRelocInsn
  x32: Fix gdb.trace/mi-trace-frame-collected.exp

 gdb/amd64-tdep.c                                   | 42 ++++++++++--
 gdb/gdbserver/Makefile.in                          |  9 +++
 gdb/gdbserver/configure                            | 31 +++++++++
 gdb/gdbserver/configure.ac                         | 14 ++++
 gdb/gdbserver/configure.srv                        |  8 ++-
 gdb/gdbserver/linux-amd64-ipa.c                    | 79 +++++++++++++++++++---
 gdb/gdbserver/linux-x86-low.c                      |  8 ++-
 .../gdb.trace/mi-trace-frame-collected.exp         | 14 +++-
 8 files changed, 186 insertions(+), 19 deletions(-)

-- 
2.5.5

[PATCH 4/6] x32: gdbserver's agent bytecode JIT: fix "call" emission

[Pedro Alves]

Running fast tracepoint tests on x32 exposes a latent bug in the agent
bytecode jitting.  There's a code path that forgets to emit the call
opcode...  Whoops.  Fixes a bunch of gdb.trace/trace-condition.exp
FAILs, like:

  (gdb)
  continue
  Continuing.

  Thread 1 "trace-condition" received signal SIGSEGV, Segmentation fault.
  0x7ffec016 in ?? ()
  (gdb) FAIL: gdb.trace/trace-condition.exp: ftrace: $rip == *set_point: advance through tracing

gdb/gdbserver/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* linux-x86-low.c (amd64_emit_call): Emit missing call opcode.
---
 gdb/gdbserver/linux-x86-low.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 1ba98ba..d847b93 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -1842,6 +1842,8 @@ amd64_emit_call (CORE_ADDR fn)
   else
     {
       int offset32 = offset64; /* we know we can't overflow here.  */
+
+      buf[i++] = 0xe8; /* call <reladdr> */
       memcpy (buf + i, &offset32, 4);
       i += 4;
     }
-- 
2.5.5

[PATCH 5/6] x32: gdb: Fix 'call' insn relocation with qRelocInsn

[Pedro Alves]

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:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* amd64-tdep.c (amd64_relocate_instruction) <callq>: Handle return
	addresses over 0x7fffffff.
---
 gdb/amd64-tdep.c | 42 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

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;
-- 
2.5.5

RE: [PATCH 0/6] x32: Fast tracepoints support

[Tedeschi, Walfred]

Pedro,

Thanks for processing this!
1/6 2/6 and 3/6 looks good. 

Thanks again!

Fred


-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Pedro Alves
Sent: Thursday, July 28, 2016 2:01 AM
To: gdb-patches@sourceware.org
Subject: [PATCH 0/6] x32: Fast tracepoints support

Yesterday I installed an Ubuntu 16.04 virtual machine here, and installed the x32 runtime support (basically, run "apt-cache search x32", and install all that shows).  That made it easy to fix a few basic x32 problems, the result of which were yesterday's x32 patches.

There's still one remaining annoying x32 problem however.  Even though gdb and gdbserver build fine on x32 now, the build still fails, when building the fast tracepoints in-process agent DSO (IPA), with:

   .../src/gdb/gdbserver/linux-amd64-ipa.c: In function ‘const target_desc* get_ipa_tdesc(int)’:
   .../src/gdb/gdbserver/linux-amd64-ipa.c:182:14: error: ‘tdesc_amd64_avx_linux’ was not declared in this scope
	  return tdesc_amd64_avx_linux;
		 ^
  [...]

So I took a stab at fixing it.  And then I thought I'd run the fast tracepoint tests against the result.  And that showed a few problems, all fixed by this series...

A diff of testing with:
 RUNTESTFLAGS="--target_board=native-gdbserver" TESTS="gdb.trace/*.exp"
vs
 RUNTESTFLAGS="--target_board=native-gdbserver/-mx32" TESTS="gdb.trace/*.exp"

Shows that x32 support is just as good as 64-bit.  Actually, it's better, since fast tracepoints on shared libraries work.  E.g.:

 -PASS: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 2 (too far)
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 2
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tracepoint with three locations
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 3
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tracepoint with two locations - installed (unload)
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tracepoint with two locations - pending (unload)
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tstop
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tfind frame 0
 +PASS: gdb.trace/change-loc.exp: 1 ftrace: tfind

I didn't bother to save a few bytes by making the jump pad, gdbserver's agent bytecode JIT etc. use/emit shorter instructions that use 32-bit operands in the places where that would be possible on x32.
Not that it'd be complicated, it's just that I've already invested more time in this at this point than I wanted, and that I'm assuming that it wouldn't result in a significant optimization.  I'd be glad to be proven wrong though, if someone wants to try that out.

Pedro Alves (6):
  x32 Fast tracepoints: IPA target descriptions
  x32 Fast tracepoints: Customize jump pad address
  x32: Avoid unsigned long when installing fast tracepoint jump pads
  x32: gdbserver's agent bytecode JIT: fix "call" emission
  x32: gdb: Fix 'call' insn relocation with qRelocInsn
  x32: Fix gdb.trace/mi-trace-frame-collected.exp

 gdb/amd64-tdep.c                                   | 42 ++++++++++--
 gdb/gdbserver/Makefile.in                          |  9 +++
 gdb/gdbserver/configure                            | 31 +++++++++
 gdb/gdbserver/configure.ac                         | 14 ++++
 gdb/gdbserver/configure.srv                        |  8 ++-
 gdb/gdbserver/linux-amd64-ipa.c                    | 79 +++++++++++++++++++---
 gdb/gdbserver/linux-x86-low.c                      |  8 ++-
 .../gdb.trace/mi-trace-frame-collected.exp         | 14 +++-
 8 files changed, 186 insertions(+), 19 deletions(-)

--
2.5.5

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 3/6] x32: Avoid unsigned long when installing fast tracepoint jump pads

[Pedro Alves]

On 07/28/2016 04:07 AM, H.J. Lu wrote:

>> Fix this by using an 8 byte memcpy like in other similar places in the
>> function.

...

>> diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
>> index d6b67c1..1ba98ba 100644
>> --- a/gdb/gdbserver/linux-x86-low.c
>> +++ b/gdb/gdbserver/linux-x86-low.c
>> @@ -1092,10 +1092,10 @@ amd64_install_fast_tracepoint_jump_pad
> (CORE_ADDR tpoint, CORE_ADDR tpaddr,
>>    buf[i++] = 0x41; buf[i++] = 0x51; /* push %r9 */
>>    buf[i++] = 0x41; buf[i++] = 0x50; /* push %r8 */
>>    buf[i++] = 0x9c; /* pushfq */
>> -  buf[i++] = 0x48; /* movl <addr>,%rdi */
>> +  buf[i++] = 0x48; /* movabs <addr>,%rdi */
>>    buf[i++] = 0xbf;
>> -  *((unsigned long *)(buf + i)) = (unsigned long) tpaddr;
>> -  i += sizeof (unsigned long);
>> +  memcpy (buf + i, &tpaddr, 8);
>> +  i += 8;
>>    buf[i++] = 0x57; /* push %rdi */
>>    append_insns (&buildaddr, i, buf);

> 
> Why not use long long here?
> 

My initial fix used uint64_t, which would be even more explicit,
but then I noticed that the the rest of the function uses this
same exact memcpy pattern in other cases where it writes other 64-bit
immediates.  So this makes everything consistent.
Note also that memcpy instead of casting also avoids worrying
about strict aliasing violations.

Thanks,
Pedro Alves

Re: [PATCH 0/6] x32: Fast tracepoints support

[Pedro Alves]

I almost forgot about this.  I'll push it all to master in
a bit.

I think this is pretty safe to put in 7.12 as well though.
Any thoughts?  Here's my analysis of the patches in the series:

>   x32 Fast tracepoints: IPA target descriptions

This makes './configure && make' just work OOTB on x32.  The IPA
doesn't really work on x32 yet after this, but I think making it
easier for downstreams and users to build gdb trumps someone
trying out fast tracepoints on x32 and that not working.

>   x32 Fast tracepoints: Customize jump pad address

This is guarded under #ifdef so should be quite safe.

>   x32: Avoid unsigned long when installing fast tracepoint jump pads

Trivial and quite safe, IMO.

>   x32: gdbserver's agent bytecode JIT: fix "call" emission
>   x32: gdb: Fix 'call' insn relocation with qRelocInsn

These two fix issues that could trigger with 64-bit as well,
and only touch the code paths that are clearly broken, so safe
to backport, IMO.

>   x32: Fix gdb.trace/mi-trace-frame-collected.exp

This is just a small testsuite tweak that adds a code path
that only executes on x32.

Thanks,
Pedro Alves

On 07/28/2016 02:01 AM, Pedro Alves wrote:
> Yesterday I installed an Ubuntu 16.04 virtual machine here, and
> installed the x32 runtime support (basically, run "apt-cache search
> x32", and install all that shows).  That made it easy to fix a few
> basic x32 problems, the result of which were yesterday's x32 patches.
> 
> There's still one remaining annoying x32 problem however.  Even though
> gdb and gdbserver build fine on x32 now, the build still fails, when
> building the fast tracepoints in-process agent DSO (IPA), with:
> 
>    .../src/gdb/gdbserver/linux-amd64-ipa.c: In function ‘const target_desc* get_ipa_tdesc(int)’:
>    .../src/gdb/gdbserver/linux-amd64-ipa.c:182:14: error: ‘tdesc_amd64_avx_linux’ was not declared in this scope
> 	  return tdesc_amd64_avx_linux;
> 		 ^
>   [...]
> 
> So I took a stab at fixing it.  And then I thought I'd run the fast
> tracepoint tests against the result.  And that showed a few problems,
> all fixed by this series...
> 
> A diff of testing with:
>  RUNTESTFLAGS="--target_board=native-gdbserver" TESTS="gdb.trace/*.exp"
> vs 
>  RUNTESTFLAGS="--target_board=native-gdbserver/-mx32" TESTS="gdb.trace/*.exp"
> 
> Shows that x32 support is just as good as 64-bit.  Actually, it's
> better, since fast tracepoints on shared libraries work.  E.g.:
> 
>  -PASS: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 2 (too far)
>  +PASS: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 2
>  +PASS: gdb.trace/change-loc.exp: 1 ftrace: tracepoint with three locations
>  +PASS: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 3
>  +PASS: gdb.trace/change-loc.exp: 1 ftrace: tracepoint with two locations - installed (unload)
>  +PASS: gdb.trace/change-loc.exp: 1 ftrace: tracepoint with two locations - pending (unload)
>  +PASS: gdb.trace/change-loc.exp: 1 ftrace: tstop
>  +PASS: gdb.trace/change-loc.exp: 1 ftrace: tfind frame 0
>  +PASS: gdb.trace/change-loc.exp: 1 ftrace: tfind
> 
> I didn't bother to save a few bytes by making the jump pad,
> gdbserver's agent bytecode JIT etc. use/emit shorter instructions that
> use 32-bit operands in the places where that would be possible on x32.
> Not that it'd be complicated, it's just that I've already invested
> more time in this at this point than I wanted, and that I'm assuming
> that it wouldn't result in a significant optimization.  I'd be glad to
> be proven wrong though, if someone wants to try that out.
> 
> Pedro Alves (6):
>   x32 Fast tracepoints: IPA target descriptions
>   x32 Fast tracepoints: Customize jump pad address
>   x32: Avoid unsigned long when installing fast tracepoint jump pads
>   x32: gdbserver's agent bytecode JIT: fix "call" emission
>   x32: gdb: Fix 'call' insn relocation with qRelocInsn
>   x32: Fix gdb.trace/mi-trace-frame-collected.exp
> 
>  gdb/amd64-tdep.c                                   | 42 ++++++++++--
>  gdb/gdbserver/Makefile.in                          |  9 +++
>  gdb/gdbserver/configure                            | 31 +++++++++
>  gdb/gdbserver/configure.ac                         | 14 ++++
>  gdb/gdbserver/configure.srv                        |  8 ++-
>  gdb/gdbserver/linux-amd64-ipa.c                    | 79 +++++++++++++++++++---
>  gdb/gdbserver/linux-x86-low.c                      |  8 ++-
>  .../gdb.trace/mi-trace-frame-collected.exp         | 14 +++-
>  8 files changed, 186 insertions(+), 19 deletions(-)
> 

Re: [PATCH 0/6] x32: Fast tracepoints support

[Pedro Alves]

On 08/19/2016 12:02 PM, Pedro Alves wrote:
> I almost forgot about this.  I'll push it all to master in
> a bit.
> 
> I think this is pretty safe to put in 7.12 as well though.
> Any thoughts?  Here's my analysis of the patches in the series:

I've pushed it to 7.12 now.

Thanks,
Pedro Alves