[patch] Fix PR 13392 : check offset of JMP insn

[patch] Fix PR 13392 : check offset of JMP insn

[Yao Qi]

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

Hi,
Bug 13392 is caused by incorrect JMP instruction when offset exceeds
integer limit.  This patch is to check the offset first to see if it is
still within the integer range.  Return error if offset exceeds.

The changes to test cases is a little mechanical, simply remove kfail,
and check the error messages of downloading tracepoints.  The test
result changes like this,

-KFAIL: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 2 (PRMS:
gdb/13392)
-PASS: gdb.trace/change-loc.exp: 2 ftrace: tstart
-KFAIL: gdb.trace/change-loc.exp: 2 ftrace: continue to marker 1 (PRMS:
gdb/13392)
-PASS: gdb.trace/pending.exp: ftrace works: start trace experiment
-KFAIL: gdb.trace/pending.exp: ftrace works: continue to marker (PRMS:
gdb/13392)
-KFAIL: gdb.trace/pending.exp: ftrace resolved_in_trace: continue to
marker 2 (PRMS: gdb/13392)
-KFAIL: gdb.trace/pending.exp: ftrace action_resolved: continue to
marker 2 (PRMS: gdb/13392)
-KFAIL: gdb.trace/pending.exp: ftrace installed_in_trace: continue to
marker 2 (PRMS: gdb/13392)

Tested on x86_64-linux, OK to apply?

-- 
Yao (齐尧)

[-- Attachment #2: 0001-fix-pr13392-check-offset-of-jmp-insn.patch --]
[-- Type: text/x-patch, Size: 8483 bytes --]

2012-03-06  Yao Qi  <yao@codesourcery.com>

	Fix PR server/13392.
	* linux-x86-low.c (amd64_install_fast_tracepoint_jump_pad): Check
	offset of JMP insn.
	* tracepoint.c (install_fast_tracepoint): Fill in ERRBUF when
	gets error.

2012-03-06  Yao Qi  <yao@codesourcery.com>

	Fix PR server/13392.
	* gdb.trace/change-loc.exp (tracepoint_change_loc_1): Remove kfail.
	(tracepoint_change_loc_2): Remove kfail.  Return if failed to
	download tracepoints.
	* gdb.trace/pending.exp (pending_tracepoint_works): Likwise.
	(pending_tracepoint_resolved_during_trace): Likewise.
	(pending_tracepoint_installed_during_trace): Likewise.
	(pending_tracepoint_with_action_resolved): Likewise.
---
 gdb/gdbserver/linux-x86-low.c          |   25 +++++++++++++-
 gdb/gdbserver/tracepoint.c             |    5 ++-
 gdb/testsuite/gdb.trace/change-loc.exp |   38 +++++++++++++++-----
 gdb/testsuite/gdb.trace/pending.exp    |   57 ++++++++++++++++++++++++--------
 4 files changed, 98 insertions(+), 27 deletions(-)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 58aaf9a..4647f5a 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -20,6 +20,7 @@
 #include <stddef.h>
 #include <signal.h>
 #include <limits.h>
+#include <inttypes.h>
 #include "server.h"
 #include "linux-low.h"
 #include "i387-fp.h"
@@ -1200,6 +1201,8 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
 {
   unsigned char buf[40];
   int i, offset;
+  int64_t loffset;
+
   CORE_ADDR buildaddr = *jump_entry;
 
   /* Build the jump pad.  */
@@ -1323,7 +1326,16 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
   *adjusted_insn_addr_end = buildaddr;
 
   /* Finally, write a jump back to the program.  */
-  offset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
+
+  loffset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
+  if (loffset > INT_MAX || loffset < INT_MIN)
+    {
+      warning ("Cannot handle jump of offset 0x%" PRIx64 " > 4-byte\n",
+	       loffset);
+      return 1;
+    }
+
+  offset = (int) loffset;
   memcpy (buf, jump_insn, sizeof (jump_insn));
   memcpy (buf + 1, &offset, 4);
   append_insns (&buildaddr, sizeof (jump_insn), buf);
@@ -1332,7 +1344,16 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
      is always done last (by our caller actually), so that we can
      install fast tracepoints with threads running.  This relies on
      the agent's atomic write support.  */
-  offset = *jump_entry - (tpaddr + sizeof (jump_insn));
+  loffset = *jump_entry - (tpaddr + sizeof (jump_insn));
+  if (loffset > INT_MAX || loffset < INT_MIN)
+    {
+      warning ("Cannot handle jump of offset 0x%" PRIx64 " > 4-byte\n",
+	       loffset);
+      return 1;
+    }
+
+  offset = (int) loffset;
+
   memcpy (buf, jump_insn, sizeof (jump_insn));
   memcpy (buf + 1, &offset, 4);
   memcpy (jjump_pad_insn, buf, sizeof (jump_insn));
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 21e58ff..1f85f9a 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -2853,7 +2853,10 @@ install_fast_tracepoint (struct tracepoint *tpoint, char *errbuf)
 					  errbuf);
 
   if (err)
-    return 1;
+    {
+      xsnprintf (errbuf, 50, "E.Failed to install fast tracepoint jumppad");
+      return 1;
+    }
 
   /* Wire it in.  */
   tpoint->handle = set_fast_tracepoint_jump (tpoint->address, fjump,
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index d6062fc..7f2fd91 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -114,8 +114,16 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
 	    pass "continue to marker 2"
 	}
 	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
+
+	    # It is possible to unable to create a jumppad for a fast tracepoint
+	    # due to the limitation of instructions.  We simply return to skip
+	    # the rest of tests.
+	    if [string equal $trace_type "ftrace"] {
+		return
+	    } else {
+		fail "continue to marker 2"
+	    }
+
 	}
     }
     # tracepoint has three locations after shlib change-loc-2 is loaded.
@@ -198,19 +206,29 @@ proc tracepoint_change_loc_2 { trace_type } { with_test_prefix "2 $trace_type" {
 	"breakpoint on marker"
 
     # tracepoint with two locations will be downloaded and installed.
-    gdb_test_no_output "tstart"
-
-    gdb_test_multiple "continue" "continue to marker 1" {
-	-re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 1"
-	}
+    gdb_test_multiple "tstart" "tstart" {
+        -re "^tstart\r\n$gdb_prompt $" {
+	    if ![string match "" "tstart"] then {
+		pass "tstart"
+            }
+        }
 	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 1"
-	    return
+	    # It is possible to unable to create a jumppad for a fast tracepoint
+	    # due to the limitation of instructions.  We simply return to skip
+	    # the rest of tests.
+
+	    if [string equal $trace_type "ftrace"] {
+		return
+	    } else {
+		fail "tstart"
+	    }
 	}
     }
 
     gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
+	"continue to marker 1"
+
+    gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
 	"continue to marker 2"
 
     # tracepoint has three locations after shlib change-loc-2 is loaded.
diff --git a/gdb/testsuite/gdb.trace/pending.exp b/gdb/testsuite/gdb.trace/pending.exp
index 017aea9..1391d42 100644
--- a/gdb/testsuite/gdb.trace/pending.exp
+++ b/gdb/testsuite/gdb.trace/pending.exp
@@ -132,18 +132,28 @@ proc pending_tracepoint_works { trace_type } { with_test_prefix "$trace_type wor
     gdb_test "break marker" "Breakpoint.*at.* file .*$srcfile, line.*" \
 	"breakpoint on marker"
 
-    gdb_test_no_output "tstart" "start trace experiment"
-
-    gdb_test_multiple "continue" "continue to marker" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker"
-	}
+    gdb_test_multiple "tstart" "start trace experiment" {
+        -re "^tstart\r\n$gdb_prompt $" {
+	    if ![string match "" "tstart"] then {
+		pass "start trace experiment"
+            }
+        }
 	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker"
-	    return
+	    # It is possible to unable to create a jumppad for a fast tracepoint
+	    # due to the limitation of instructions.  We simply return to skip
+	    # the rest of tests.
+
+	    if [string equal $trace_type "ftrace"] {
+		return
+	    } else {
+		fail "start trace experiment"
+	    }
 	}
     }
 
+    gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*" \
+	"continue to marker"
+
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
 
     gdb_test "tfind start" "#0 .*" "tfind test frame 0"
@@ -194,8 +204,15 @@ proc pending_tracepoint_resolved_during_trace { trace_type } \
 	    pass "continue to marker 2"
 	}
 	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
+	    # It is possible to unable to create a jumppad for a fast tracepoint
+	    # due to the limitation of instructions.  We simply return to skip
+	    # the rest of tests.
+
+	    if [string equal $trace_type "ftrace"] {
+		return
+	    } else {
+		fail "continue to marker 2"
+	    }
 	}
     }
 
@@ -258,8 +275,14 @@ proc pending_tracepoint_installed_during_trace { trace_type } \
 	    pass "continue to marker 2"
 	}
 	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
+	    # It is possible to unable to create a jumppad for a fast tracepoint
+	    # due to the limitation of instructions.  We simply return to skip
+	    # the rest of tests.
+	    if [string equal $trace_type "ftrace"] {
+		return
+	    } else {
+		fail "continue to marker 2"
+	    }
 	}
     }
 
@@ -428,8 +451,14 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
 	    pass "continue to marker 2"
 	}
 	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
+	    # It is possible to unable to create a jumppad for a fast tracepoint
+	    # due to the limitation of instructions.  We simply return to skip
+	    # the rest of tests.
+	    if [string equal $trace_type "ftrace"] {
+		return
+	    } else {
+		fail "continue to marker 2"
+	    }
 	}
     }
 
-- 
1.7.0.4

Re: [patch] Fix PR 13392 : check offset of JMP insn

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 11069 bytes --]

On 03/06/2012 01:38 PM, Yao Qi wrote:

> Hi,
> Bug 13392 is caused by incorrect JMP instruction when offset exceeds
> integer limit.  This patch is to check the offset first to see if it is
> still within the integer range.  Return error if offset exceeds.
> 
> The changes to test cases is a little mechanical, simply remove kfail,
> and check the error messages of downloading tracepoints.  The test
> result changes like this,
> 
> -KFAIL: gdb.trace/change-loc.exp: 1 ftrace: continue to marker 2 (PRMS:
> gdb/13392)
> -PASS: gdb.trace/change-loc.exp: 2 ftrace: tstart
> -KFAIL: gdb.trace/change-loc.exp: 2 ftrace: continue to marker 1 (PRMS:
> gdb/13392)
> -PASS: gdb.trace/pending.exp: ftrace works: start trace experiment
> -KFAIL: gdb.trace/pending.exp: ftrace works: continue to marker (PRMS:
> gdb/13392)
> -KFAIL: gdb.trace/pending.exp: ftrace resolved_in_trace: continue to
> marker 2 (PRMS: gdb/13392)
> -KFAIL: gdb.trace/pending.exp: ftrace action_resolved: continue to
> marker 2 (PRMS: gdb/13392)
> -KFAIL: gdb.trace/pending.exp: ftrace installed_in_trace: continue to
> marker 2 (PRMS: gdb/13392)
> 
> Tested on x86_64-linux, OK to apply?
> 
> -- Yao (齐尧)
> 
> 
> 0001-fix-pr13392-check-offset-of-jmp-insn.patch
> 
> 
> 2012-03-06  Yao Qi  <yao@codesourcery.com>
> 
> 	Fix PR server/13392.
> 	* linux-x86-low.c (amd64_install_fast_tracepoint_jump_pad): Check
> 	offset of JMP insn.
> 	* tracepoint.c (install_fast_tracepoint): Fill in ERRBUF when
> 	gets error.
> 
> 2012-03-06  Yao Qi  <yao@codesourcery.com>
> 
> 	Fix PR server/13392.
> 	* gdb.trace/change-loc.exp (tracepoint_change_loc_1): Remove kfail.
> 	(tracepoint_change_loc_2): Remove kfail.  Return if failed to
> 	download tracepoints.
> 	* gdb.trace/pending.exp (pending_tracepoint_works): Likwise.
> 	(pending_tracepoint_resolved_during_trace): Likewise.
> 	(pending_tracepoint_installed_during_trace): Likewise.
> 	(pending_tracepoint_with_action_resolved): Likewise.
> ---
>  gdb/gdbserver/linux-x86-low.c          |   25 +++++++++++++-
>  gdb/gdbserver/tracepoint.c             |    5 ++-
>  gdb/testsuite/gdb.trace/change-loc.exp |   38 +++++++++++++++-----
>  gdb/testsuite/gdb.trace/pending.exp    |   57 ++++++++++++++++++++++++--------
>  4 files changed, 98 insertions(+), 27 deletions(-)
> 
> diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
> index 58aaf9a..4647f5a 100644
> --- a/gdb/gdbserver/linux-x86-low.c
> +++ b/gdb/gdbserver/linux-x86-low.c
> @@ -20,6 +20,7 @@
>  #include <stddef.h>
>  #include <signal.h>
>  #include <limits.h>
> +#include <inttypes.h>
>  #include "server.h"
>  #include "linux-low.h"
>  #include "i387-fp.h"
> @@ -1200,6 +1201,8 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
>  {
>    unsigned char buf[40];
>    int i, offset;
> +  int64_t loffset;
> +
>    CORE_ADDR buildaddr = *jump_entry;
>  
>    /* Build the jump pad.  */
> @@ -1323,7 +1326,16 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
>    *adjusted_insn_addr_end = buildaddr;
>  
>    /* Finally, write a jump back to the program.  */
> -  offset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
> +
> +  loffset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
> +  if (loffset > INT_MAX || loffset < INT_MIN)
> +    {
> +      warning ("Cannot handle jump of offset 0x%" PRIx64 " > 4-byte\n",
> +	       loffset);


We should send an error back to GDB with "E." instead of printing something
to gdbserver's console, and leaving the user with a generic and
unhelful error.  "4-byte" isn't strictly correct, as this is a signed
offset, and I think we can be a bit more clear.  So we end up with:

      sprintf (err,
	       "E.Jump back from jump pad too far from tracepoint "
	       "(offset 0x%" PRIx64 " > int32).", loffset);

> +      return 1;
> +    }
> +
> +  offset = (int) loffset;
>    memcpy (buf, jump_insn, sizeof (jump_insn));
>    memcpy (buf + 1, &offset, 4);
>    append_insns (&buildaddr, sizeof (jump_insn), buf);
> @@ -1332,7 +1344,16 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
>       is always done last (by our caller actually), so that we can
>       install fast tracepoints with threads running.  This relies on
>       the agent's atomic write support.  */
> -  offset = *jump_entry - (tpaddr + sizeof (jump_insn));
> +  loffset = *jump_entry - (tpaddr + sizeof (jump_insn));
> +  if (loffset > INT_MAX || loffset < INT_MIN)
> +    {
> +      warning ("Cannot handle jump of offset 0x%" PRIx64 " > 4-byte\n",
> +	       loffset);


      sprintf (err,
	       "E.Jump pad too far from tracepoint "
	       "(offset 0x%" PRIx64 " > int32).", loffset);

> +      return 1;
> +    }
> +
> +  offset = (int) loffset;
> +
>    memcpy (buf, jump_insn, sizeof (jump_insn));
>    memcpy (buf + 1, &offset, 4);
>    memcpy (jjump_pad_insn, buf, sizeof (jump_insn));
> diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
> index 21e58ff..1f85f9a 100644
> --- a/gdb/gdbserver/tracepoint.c
> +++ b/gdb/gdbserver/tracepoint.c
> @@ -2853,7 +2853,10 @@ install_fast_tracepoint (struct tracepoint *tpoint, char *errbuf)
>  					  errbuf);
>  
>    if (err)
> -    return 1;
> +    {
> +      xsnprintf (errbuf, 50, "E.Failed to install fast tracepoint jumppad");


This would overwrite any error the install_fast_tracepoint target hook wrote to
errbuf (can be seen quoted just above).

> +      return 1;
> +    }
>  
>    /* Wire it in.  */
>    tpoint->handle = set_fast_tracepoint_jump (tpoint->address, fjump,
> diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
> index d6062fc..7f2fd91 100644
> --- a/gdb/testsuite/gdb.trace/change-loc.exp
> +++ b/gdb/testsuite/gdb.trace/change-loc.exp
> @@ -114,8 +114,16 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
>  	    pass "continue to marker 2"
>  	}
>  	-re ".*$gdb_prompt $" {
> -	    kfail "gdb/13392" "continue to marker 2"
> -	    return
> +
> +	    # It is possible to unable to create a jumppad for a fast tracepoint
> +	    # due to the limitation of instructions.  We simply return to skip
> +	    # the rest of tests.


I understand what you mean, but this needs copy/editing.  "possible to unable to create"
doesn't parse.  But, I think we can do better when the target sends back the real error
to GDB.  Since that essentially means every hunk in your patch needs redoing, I went
ahead and did the changes.  See patches attached.

> +	    if [string equal $trace_type "ftrace"] {
> +		return
> +	    } else {
> +		fail "continue to marker 2"
> +	    }
> +
>  	}
>      }
>      # tracepoint has three locations after shlib change-loc-2 is loaded.
> @@ -198,19 +206,29 @@ proc tracepoint_change_loc_2 { trace_type } { with_test_prefix "2 $trace_type" {
>  	"breakpoint on marker"
>  
>      # tracepoint with two locations will be downloaded and installed.
> -    gdb_test_no_output "tstart"
> -
> -    gdb_test_multiple "continue" "continue to marker 1" {
> -	-re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
> -	    pass "continue to marker 1"
> -	}
> +    gdb_test_multiple "tstart" "tstart" {
> +        -re "^tstart\r\n$gdb_prompt $" {
> +	    if ![string match "" "tstart"] then {


???  When can [string match "" "tstart"] ever return something other than 0 ?

> +		pass "tstart"
> +            }
> +        }
>  	-re ".*$gdb_prompt $" {
> -	    kfail "gdb/13392" "continue to marker 1"
> -	    return
> +	    # It is possible to unable to create a jumppad for a fast tracepoint
> +	    # due to the limitation of instructions.  We simply return to skip
> +	    # the rest of tests.
> +
> +	    if [string equal $trace_type "ftrace"] {
> +		return
> +	    } else {
> +		fail "tstart"
> +	    }


Hmm, what does that error on "continue" mean?  With just your patch, I see in gdb.log:

continue
Continuing.
Target returns error code '01'.

That's another generic error that could be made to return "E." instead.

So I enabled remote debug, and we see:

continue
Continuing.
Sending packet: $QPassSignals:#f3...Packet received: OK
Sending packet: $vCont;s:p3e0d.3e0d#e8...Packet received: T0506:80d6ffffff7f0000;07:58d6ffffff7f0000;10:6a07400000000000;thread:p3e0d.3e0d;core:3;
Sending packet: $qTStatus#49...Packet received: T1;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:001331049107617717;stoptime:00000
0000;username::;notes::
Sending packet: $qTMinFTPILen#3b...Packet received: 5
Sending packet: $m7ffff7bf05c6,1#96...Packet received: e8
Sending packet: $m7ffff7bf05c7,4#9a...Packet received: f1ffffff
Sending packet: $QTDP:4:00007ffff7bf05c6:E:0:0:F5#75...Packet received: E.duplicate
Target returns error code '01'.

So we're failing in QTDP handling, as expected.  But where exactly?  It's not when
trying to install the fast tracepoint to target memory.  It's here:

--- c/gdb/gdbserver/tracepoint.c
+++ w/gdb/gdbserver/tracepoint.c
@@ -2307,7 +2307,8 @@ cmd_qtdp (char *own_buf)
          trace_debug ("Tracepoint error: tracepoint %d"
                       " at 0x%s already exists",
                       (int) num, paddress (addr));
-         write_enn (own_buf);
+         sprintf (own_buf, "E.duplicate");
+         //write_enn (own_buf);
          return;
        }

continue
Continuing.
Sending packet: $QPassSignals:#f3...Packet received: OK
Sending packet: $vCont;s:p3e0d.3e0d#e8...Packet received: T0506:80d6ffffff7f0000;07:58d6ffffff7f0000;10:6a07400000000000;thread:p3e0d.3e0d;core:3;
Sending packet: $qTStatus#49...Packet received: T1;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:001331049107617717;stoptime:00000
0000;username::;notes::
Sending packet: $qTMinFTPILen#3b...Packet received: 5
Sending packet: $m7ffff7bf05c6,1#96...Packet received: e8
Sending packet: $m7ffff7bf05c7,4#9a...Packet received: f1ffffff
Sending packet: $QTDP:4:00007ffff7bf05c6:E:0:0:F5#75...Packet received: E.duplicate
Target returns error code '.duplicate'.

So something doesn't look exactly right.  The target knows about this fast
tracepoint already, but it isn't really installed yet (so we have 3 levels of "inserted",
not just 2: never downloaded; downloaded; downloaded and installed).  Maybe GDB
should have disabled the tracepoint on "tstart", or deleted it from the target, or...
Anyway, that'll need some further thinking.  In the mean time, I've made gdbserver
send a clearer error back to GDB, and we end up with:

Target returns error code '.Tracepoint 2 at 0x7ffff67c2579 already exists'.

>      # tracepoint has three locations after shlib change-loc-2 is loaded.
> diff --git a/gdb/testsuite/gdb.trace/pending.exp b/gdb/testsuite/gdb.trace/pending.exp
> index 017aea9..1391d42 100644
> --- a/gdb/testsuite/gdb.trace/pending.exp
> +++ b/gdb/testsuite/gdb.trace/pending.exp


... more of the same.

Please find attached the interdiff (my changes on top of yours), and then
the final patch (both patches merged).

WDYT?

-- 
Pedro Alves

[-- Attachment #2: interdiff.diff --]
[-- Type: text/x-patch, Size: 10478 bytes --]

commit a7e67466d25e5f5172c08d8023290d0bb64fe4ef
Author: Pedro Alves <palves@redhat.com>
Date:   Tue Mar 6 14:51:03 2012 +0000

    a

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 4647f5a..ed1f8a8 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -1330,8 +1330,9 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
   loffset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
   if (loffset > INT_MAX || loffset < INT_MIN)
     {
-      warning ("Cannot handle jump of offset 0x%" PRIx64 " > 4-byte\n",
-	       loffset);
+      sprintf (err,
+	       "E.Jump back from jump pad too far from tracepoint "
+	       "(offset 0x%" PRIx64 " > int32).", loffset);
       return 1;
     }
 
@@ -1347,8 +1348,9 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
   loffset = *jump_entry - (tpaddr + sizeof (jump_insn));
   if (loffset > INT_MAX || loffset < INT_MIN)
     {
-      warning ("Cannot handle jump of offset 0x%" PRIx64 " > 4-byte\n",
-	       loffset);
+      sprintf (err,
+	       "E.Jump pad too far from tracepoint "
+	       "(offset 0x%" PRIx64 " > int32).", loffset);
       return 1;
     }
 
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 1f85f9a..3a10daf 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -2304,10 +2304,8 @@ cmd_qtdp (char *own_buf)
       /* Duplicate tracepoints are never allowed.  */
       if (tpoint)
 	{
-	  trace_debug ("Tracepoint error: tracepoint %d"
-		       " at 0x%s already exists",
+	  sprintf (own_buf, "E.Tracepoint %d at 0x%s already exists",
 		       (int) num, paddress (addr));
-	  write_enn (own_buf);
 	  return;
 	}
 
@@ -2853,10 +2851,7 @@ install_fast_tracepoint (struct tracepoint *tpoint, char *errbuf)
 					  errbuf);
 
   if (err)
-    {
-      xsnprintf (errbuf, 50, "E.Failed to install fast tracepoint jumppad");
-      return 1;
-    }
+    return 1;
 
   /* Wire it in.  */
   tpoint->handle = set_fast_tracepoint_jump (tpoint->address, fjump,
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index dc9b8a9..a9bbbbc 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -98,7 +98,21 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
     gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
 	"continue to marker 1"
     # Set a tracepoint during tracing.
-    gdb_test "${trace_type} set_tracepoint" ".*" "set tracepoint on set_tracepoint"
+    set test "set tracepoint on set_tracepoint"
+    gdb_test_multiple "${trace_type} set_tracepoint" $test {
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+	    } else {
+		fail $test
+	    }
+	}
+	-re "\r\n$gdb_prompt $" {
+	    pass $test
+	}
+    }
 
     gdb_trace_setactions "set action for tracepoint" "" \
 	"collect \$$pcreg" "^$"
@@ -109,21 +123,22 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
 \[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*4\.1.* in func4.*4\.2.* in func4.*" \
 	"tracepoint with two locations"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
-	}
-	-re ".*$gdb_prompt $" {
-
-	    # It is possible to unable to create a jumppad for a fast tracepoint
-	    # due to the limitation of instructions.  We simply return to skip
-	    # the rest of tests.
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .*Tracepoint 4 at .* already exists.*$gdb_prompt $" {
 	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint) earlier.
+		pass "$test (tracepoint already exists)"
+		# Skip the rest of the tests.
 		return
 	    } else {
-		fail "continue to marker 2"
+		fail $test
 	    }
-
+	}
+	-re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+	    pass "continue to marker 2"
 	}
     }
     # tracepoint has three locations after shlib change-loc-2 is loaded.
@@ -206,21 +221,20 @@ proc tracepoint_change_loc_2 { trace_type } { with_test_prefix "2 $trace_type" {
 	"breakpoint on marker"
 
     # tracepoint with two locations will be downloaded and installed.
-    gdb_test_multiple "tstart" "tstart" {
+    set test "tstart"
+    gdb_test_multiple "tstart" $test {
         -re "^tstart\r\n$gdb_prompt $" {
-	    if ![string match "" "tstart"] then {
-		pass "tstart"
-            }
+	    pass $test
         }
-	-re ".*$gdb_prompt $" {
-	    # It is possible to unable to create a jumppad for a fast tracepoint
-	    # due to the limitation of instructions.  We simply return to skip
-	    # the rest of tests.
-
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
 	    if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
 		return
 	    } else {
-		fail "tstart"
+		fail $test
 	    }
 	}
     }
diff --git a/gdb/testsuite/gdb.trace/pending.exp b/gdb/testsuite/gdb.trace/pending.exp
index 88b294a..f2147cc 100644
--- a/gdb/testsuite/gdb.trace/pending.exp
+++ b/gdb/testsuite/gdb.trace/pending.exp
@@ -132,21 +132,20 @@ proc pending_tracepoint_works { trace_type } { with_test_prefix "$trace_type wor
     gdb_test "break marker" "Breakpoint.*at.* file .*$srcfile, line.*" \
 	"breakpoint on marker"
 
-    gdb_test_multiple "tstart" "start trace experiment" {
+    set test "start trace experiment"
+    gdb_test_multiple "tstart" $test {
         -re "^tstart\r\n$gdb_prompt $" {
-	    if ![string match "" "tstart"] then {
-		pass "start trace experiment"
-            }
+	    pass $test
         }
-	-re ".*$gdb_prompt $" {
-	    # It is possible to unable to create a jumppad for a fast tracepoint
-	    # due to the limitation of instructions.  We simply return to skip
-	    # the rest of tests.
-
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
 	    if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
 		return
 	    } else {
-		fail "start trace experiment"
+		pass $test
 	    }
 	}
     }
@@ -199,21 +198,23 @@ proc pending_tracepoint_resolved_during_trace { trace_type } \
     gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*pending.c.*" \
 	"continue to marker 1"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
-	}
-	-re ".*$gdb_prompt $" {
-	    # It is possible to unable to create a jumppad for a fast tracepoint
-	    # due to the limitation of instructions.  We simply return to skip
-	    # the rest of tests.
-
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .*Tracepoint .* at .* already exists.*$gdb_prompt $" {
 	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint) earlier.
+		pass "$test (tracepoint already exists)"
+		# Skip the rest of the tests.
 		return
 	    } else {
-		fail "continue to marker 2"
+		fail $test
 	    }
 	}
+	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+	    pass $test
+	}
     }
 
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
@@ -270,20 +271,23 @@ proc pending_tracepoint_installed_during_trace { trace_type } \
 \[0-9\]+\[\t \]+\(fast |\)tracepoint\[ \t\]+keep y.*PENDING.*set_point2.*" \
 	"single pending tracepoint on set_point2"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
-	}
-	-re ".*$gdb_prompt $" {
-	    # It is possible to unable to create a jumppad for a fast tracepoint
-	    # due to the limitation of instructions.  We simply return to skip
-	    # the rest of tests.
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .*Tracepoint .* at .* already exists.*$gdb_prompt $" {
 	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint) earlier.
+		pass "$test (tracepoint already exists)"
+		# Skip the rest of the tests.
 		return
 	    } else {
-		fail "continue to marker 2"
+		fail $test
 	    }
 	}
+	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+	    pass "continue to marker 2"
+	}
     }
 
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
@@ -446,20 +450,23 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
     gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*pending.c.*" \
 	"continue to marker 1"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
-	}
-	-re ".*$gdb_prompt $" {
-	    # It is possible to unable to create a jumppad for a fast tracepoint
-	    # due to the limitation of instructions.  We simply return to skip
-	    # the rest of tests.
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .*Tracepoint .* at .* already exists.*$gdb_prompt $" {
 	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint) earlier.
+		pass "$test (tracepoint already exists)"
+		# Skip the rest of the tests.
 		return
 	    } else {
-		fail "continue to marker 2"
+		fail $test
 	    }
 	}
+	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+	    pass $test
+	}
     }
 
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"

[-- Attachment #3: pr13392.diff --]
[-- Type: text/x-patch, Size: 11253 bytes --]

2012-03-06  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	PR server/13392

	* linux-x86-low.c: Include inttypes.h.
	(amd64_install_fast_tracepoint_jump_pad): Check
	offset of JMP insn.
	* tracepoint.c (cmd_qtdp): Return an E. style error when a
	duplicate tracepoint is detected.

2012-03-06  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	PR server/13392

	* gdb.trace/change-loc.exp (tracepoint_change_loc_1): Remove
	kfail.  Expect target errors related to fast tracepoints being too
	far from the jump pad.
	(tracepoint_change_loc_2): Likewise.
	* gdb.trace/pending.exp (pending_tracepoint_works): Likewise.
	(pending_tracepoint_resolved_during_trace): Likewise.
	(pending_tracepoint_installed_during_trace): Likewise.
	(pending_tracepoint_with_action_resolved): Likewise.

diff --git c/gdb/gdbserver/linux-x86-low.c w/gdb/gdbserver/linux-x86-low.c
index 58aaf9a..ed1f8a8 100644
--- c/gdb/gdbserver/linux-x86-low.c
+++ w/gdb/gdbserver/linux-x86-low.c
@@ -20,6 +20,7 @@
 #include <stddef.h>
 #include <signal.h>
 #include <limits.h>
+#include <inttypes.h>
 #include "server.h"
 #include "linux-low.h"
 #include "i387-fp.h"
@@ -1200,6 +1201,8 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
 {
   unsigned char buf[40];
   int i, offset;
+  int64_t loffset;
+
   CORE_ADDR buildaddr = *jump_entry;
 
   /* Build the jump pad.  */
@@ -1323,7 +1326,17 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
   *adjusted_insn_addr_end = buildaddr;
 
   /* Finally, write a jump back to the program.  */
-  offset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
+
+  loffset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
+  if (loffset > INT_MAX || loffset < INT_MIN)
+    {
+      sprintf (err,
+	       "E.Jump back from jump pad too far from tracepoint "
+	       "(offset 0x%" PRIx64 " > int32).", loffset);
+      return 1;
+    }
+
+  offset = (int) loffset;
   memcpy (buf, jump_insn, sizeof (jump_insn));
   memcpy (buf + 1, &offset, 4);
   append_insns (&buildaddr, sizeof (jump_insn), buf);
@@ -1332,7 +1345,17 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
      is always done last (by our caller actually), so that we can
      install fast tracepoints with threads running.  This relies on
      the agent's atomic write support.  */
-  offset = *jump_entry - (tpaddr + sizeof (jump_insn));
+  loffset = *jump_entry - (tpaddr + sizeof (jump_insn));
+  if (loffset > INT_MAX || loffset < INT_MIN)
+    {
+      sprintf (err,
+	       "E.Jump pad too far from tracepoint "
+	       "(offset 0x%" PRIx64 " > int32).", loffset);
+      return 1;
+    }
+
+  offset = (int) loffset;
+
   memcpy (buf, jump_insn, sizeof (jump_insn));
   memcpy (buf + 1, &offset, 4);
   memcpy (jjump_pad_insn, buf, sizeof (jump_insn));
diff --git c/gdb/gdbserver/tracepoint.c w/gdb/gdbserver/tracepoint.c
index 21e58ff..3a10daf 100644
--- c/gdb/gdbserver/tracepoint.c
+++ w/gdb/gdbserver/tracepoint.c
@@ -2304,10 +2304,8 @@ cmd_qtdp (char *own_buf)
       /* Duplicate tracepoints are never allowed.  */
       if (tpoint)
 	{
-	  trace_debug ("Tracepoint error: tracepoint %d"
-		       " at 0x%s already exists",
+	  sprintf (own_buf, "E.Tracepoint %d at 0x%s already exists",
 		       (int) num, paddress (addr));
-	  write_enn (own_buf);
 	  return;
 	}
 
diff --git c/gdb/testsuite/gdb.trace/change-loc.exp w/gdb/testsuite/gdb.trace/change-loc.exp
index d6062fc..a9bbbbc 100644
--- c/gdb/testsuite/gdb.trace/change-loc.exp
+++ w/gdb/testsuite/gdb.trace/change-loc.exp
@@ -98,7 +98,21 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
     gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
 	"continue to marker 1"
     # Set a tracepoint during tracing.
-    gdb_test "${trace_type} set_tracepoint" ".*" "set tracepoint on set_tracepoint"
+    set test "set tracepoint on set_tracepoint"
+    gdb_test_multiple "${trace_type} set_tracepoint" $test {
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+	    } else {
+		fail $test
+	    }
+	}
+	-re "\r\n$gdb_prompt $" {
+	    pass $test
+	}
+    }
 
     gdb_trace_setactions "set action for tracepoint" "" \
 	"collect \$$pcreg" "^$"
@@ -109,14 +123,23 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
 \[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*4\.1.* in func4.*4\.2.* in func4.*" \
 	"tracepoint with two locations"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .*Tracepoint 4 at .* already exists.*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint) earlier.
+		pass "$test (tracepoint already exists)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
+	}
 	-re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
 	    pass "continue to marker 2"
 	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
-	}
     }
     # tracepoint has three locations after shlib change-loc-2 is loaded.
     gdb_test "info trace" \
@@ -198,19 +221,28 @@ proc tracepoint_change_loc_2 { trace_type } { with_test_prefix "2 $trace_type" {
 	"breakpoint on marker"
 
     # tracepoint with two locations will be downloaded and installed.
-    gdb_test_no_output "tstart"
-
-    gdb_test_multiple "continue" "continue to marker 1" {
-	-re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 1"
-	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 1"
-	    return
+    set test "tstart"
+    gdb_test_multiple "tstart" $test {
+        -re "^tstart\r\n$gdb_prompt $" {
+	    pass $test
+        }
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
 	}
     }
 
     gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
+	"continue to marker 1"
+
+    gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
 	"continue to marker 2"
 
     # tracepoint has three locations after shlib change-loc-2 is loaded.
diff --git c/gdb/testsuite/gdb.trace/pending.exp w/gdb/testsuite/gdb.trace/pending.exp
index 017aea9..f2147cc 100644
--- c/gdb/testsuite/gdb.trace/pending.exp
+++ w/gdb/testsuite/gdb.trace/pending.exp
@@ -132,18 +132,27 @@ proc pending_tracepoint_works { trace_type } { with_test_prefix "$trace_type wor
     gdb_test "break marker" "Breakpoint.*at.* file .*$srcfile, line.*" \
 	"breakpoint on marker"
 
-    gdb_test_no_output "tstart" "start trace experiment"
-
-    gdb_test_multiple "continue" "continue to marker" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker"
-	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker"
-	    return
+    set test "start trace experiment"
+    gdb_test_multiple "tstart" $test {
+        -re "^tstart\r\n$gdb_prompt $" {
+	    pass $test
+        }
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		pass $test
+	    }
 	}
     }
 
+    gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*" \
+	"continue to marker"
+
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
 
     gdb_test "tfind start" "#0 .*" "tfind test frame 0"
@@ -189,13 +198,22 @@ proc pending_tracepoint_resolved_during_trace { trace_type } \
     gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*pending.c.*" \
 	"continue to marker 1"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .*Tracepoint .* at .* already exists.*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint) earlier.
+		pass "$test (tracepoint already exists)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
 	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
+	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+	    pass $test
 	}
     }
 
@@ -253,14 +271,23 @@ proc pending_tracepoint_installed_during_trace { trace_type } \
 \[0-9\]+\[\t \]+\(fast |\)tracepoint\[ \t\]+keep y.*PENDING.*set_point2.*" \
 	"single pending tracepoint on set_point2"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .*Tracepoint .* at .* already exists.*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint) earlier.
+		pass "$test (tracepoint already exists)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
+	}
 	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
 	    pass "continue to marker 2"
 	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
-	}
     }
 
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
@@ -423,13 +450,22 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
     gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*pending.c.*" \
 	"continue to marker 1"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .*Tracepoint .* at .* already exists.*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint) earlier.
+		pass "$test (tracepoint already exists)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
 	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
+	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+	    pass $test
 	}
     }
 

Re: [patch] Fix PR 13392 : check offset of JMP insn

[Philippe Waroquiers]

On Tue, 2012-03-06 at 17:03 +0000, Pedro Alves wrote:
> We should send an error back to GDB with "E." instead of printing something
> to gdbserver's console, and leaving the user with a generic and
> unhelful error.  "4-byte" isn't strictly correct, as this is a signed
> offset, and I think we can be a bit more clear.  So we end up with:
> 
>       sprintf (err,
> 	       "E.Jump back from jump pad too far from tracepoint "
> 	       "(offset 0x%" PRIx64 " > int32).", loffset);
> 


In the gdb protocol documentation, all error replies but one
are described as E NN. Some of them specifies that NN are
hex digits.
The exception is the packet qTMinFTPILen:
the error reply is described as only an E
(this last sentence intentionnally not finished with a . :).

Is there somewhere a description of what an E. packet is,
and when this is allowed ?

Philippe

Re: [patch] Fix PR 13392 : check offset of JMP insn

[Stan Shebs]

On 3/6/12 12:14 PM, Philippe Waroquiers wrote:
> On Tue, 2012-03-06 at 17:03 +0000, Pedro Alves wrote:
>> We should send an error back to GDB with "E." instead of printing something
>> to gdbserver's console, and leaving the user with a generic and
>> unhelful error.  "4-byte" isn't strictly correct, as this is a signed
>> offset, and I think we can be a bit more clear.  So we end up with:
>>
>>        sprintf (err,
>> 	       "E.Jump back from jump pad too far from tracepoint "
>> 	       "(offset 0x%" PRIx64 ">  int32).", loffset);
>>
>
> In the gdb protocol documentation, all error replies but one
> are described as E NN. Some of them specifies that NN are
> hex digits.
> The exception is the packet qTMinFTPILen:
> the error reply is described as only an E
> (this last sentence intentionnally not finished with a . :).
>
> Is there somewhere a description of what an E. packet is,
> and when this is allowed ?
>

It doesn't look like it's officially described in the manual.  The 
theory is that ENN dates nearly to the 4-bit era :-) and is pretty much 
useless without an agreed-upon table of what the different NN values 
mean.  But since there are multiple generations of stubs out there that 
might or might not have assigned their own meanings to NN, those are 
pretty much off-limits now, and so E.<string> is a convenient way to 
extend error returns without disrupting backward compatibility too 
much.  The string is uninterpreted, GDB should just report that an error 
happened and print the string verbatim.

It's conceivable that one could do something with GDB that interprets 
the error reply (whether NN or a string) and does something differently, 
but in practice, if the target side is reporting errors, things are 
going straight downhill and the user needs to decide what to do about 
it.  Also, if a particular result is common enough that GDB has code to 
cope with it, then maybe it's just another element of the protocol, not 
really an error. :-)

Stan

Re: [patch] Fix PR 13392 : check offset of JMP insn

[Yao Qi]

[-- Attachment #1: Type: text/plain, Size: 3894 bytes --]

On 03/07/2012 01:03 AM, Pedro Alves wrote:
> Hmm, what does that error on "continue" mean?  With just your patch, I see in gdb.log:
> 
> continue
> Continuing.
> Target returns error code '01'.
> 
> That's another generic error that could be made to return "E." instead.

This problem is what I planed to fix in next step.

> 
> So I enabled remote debug, and we see:
> 
> continue
> Continuing.
> Sending packet: $QPassSignals:#f3...Packet received: OK
> Sending packet: $vCont;s:p3e0d.3e0d#e8...Packet received: T0506:80d6ffffff7f0000;07:58d6ffffff7f0000;10:6a07400000000000;thread:p3e0d.3e0d;core:3;
> Sending packet: $qTStatus#49...Packet received: T1;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:001331049107617717;stoptime:00000
> 0000;username::;notes::
> Sending packet: $qTMinFTPILen#3b...Packet received: 5
> Sending packet: $m7ffff7bf05c6,1#96...Packet received: e8
> Sending packet: $m7ffff7bf05c7,4#9a...Packet received: f1ffffff
> Sending packet: $QTDP:4:00007ffff7bf05c6:E:0:0:F5#75...Packet received: E.duplicate
> Target returns error code '01'.
> 
> So we're failing in QTDP handling, as expected.  But where exactly?  It's not when
> trying to install the fast tracepoint to target memory.  It's here:
> 
> --- c/gdb/gdbserver/tracepoint.c
> +++ w/gdb/gdbserver/tracepoint.c
> @@ -2307,7 +2307,8 @@ cmd_qtdp (char *own_buf)
>           trace_debug ("Tracepoint error: tracepoint %d"
>                        " at 0x%s already exists",
>                        (int) num, paddress (addr));
> -         write_enn (own_buf);
> +         sprintf (own_buf, "E.duplicate");
> +         //write_enn (own_buf);
>           return;
>         }
> 
> continue
> Continuing.
> Sending packet: $QPassSignals:#f3...Packet received: OK
> Sending packet: $vCont;s:p3e0d.3e0d#e8...Packet received: T0506:80d6ffffff7f0000;07:58d6ffffff7f0000;10:6a07400000000000;thread:p3e0d.3e0d;core:3;
> Sending packet: $qTStatus#49...Packet received: T1;tnotrun:0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:001331049107617717;stoptime:00000
> 0000;username::;notes::
> Sending packet: $qTMinFTPILen#3b...Packet received: 5
> Sending packet: $m7ffff7bf05c6,1#96...Packet received: e8
> Sending packet: $m7ffff7bf05c7,4#9a...Packet received: f1ffffff
> Sending packet: $QTDP:4:00007ffff7bf05c6:E:0:0:F5#75...Packet received: E.duplicate
> Target returns error code '.duplicate'.
> 
> So something doesn't look exactly right.  The target knows about this fast
> tracepoint already, but it isn't really installed yet (so we have 3 levels of "inserted",
> not just 2: never downloaded; downloaded; downloaded and installed).  Maybe GDB

Yes, there are 3 levels so far.  Do you think target should keep fast
tracepoints that "downloaded but not installed yet"?  I am inclined to
teach target to remove downloaded fast tracepoints if they are not
installed successfully.  Does it sounds good?  The QTDP is like a
transaction, returns OK when both download and installation is
successful.  If installation failed, roll back to cleanup downloaded
fast tracepoint.

> should have disabled the tracepoint on "tstart", or deleted it from the target, or...
> Anyway, that'll need some further thinking.  In the mean time, I've made gdbserver

Agreed.  GDB tracepoint/breakpoint module should exposes some interfaces
for other modules, such as target, to delete or disable breakpoint
locations when needed.

> send a clearer error back to GDB, and we end up with:
> 
> Target returns error code '.Tracepoint 2 at 0x7ffff67c2579 already exists'.

It is OK for me to let remote target to reports a duplicated tracepoint.

In this version, some changes compared with last one,

  - move `tracepoint already exist' patch out, which can be a separate one,
  - remove downloaded tracepoint in target when failed to install,

-- 
Yao (齐尧)

[-- Attachment #2: 0001-fix-pr13392-check-offset-of-jmp-insn.patch --]
[-- Type: text/x-patch, Size: 12421 bytes --]

2012-03-07  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	Fix PR server/13392.
	* linux-x86-low.c (amd64_install_fast_tracepoint_jump_pad): Check
	offset of JMP insn.
	* tracepoint.c (remove_tracepoint): New.
	(cmd_qtdp): Call remove_tracepoint when failed to install.

2012-03-07  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	Fix PR server/13392.
	* gdb.trace/change-loc.exp (tracepoint_change_loc_1): Remove kfail.
	(tracepoint_change_loc_2): Remove kfail.  Return if failed to
	download tracepoints.
	* gdb.trace/pending.exp (pending_tracepoint_works): Likewise.
	(pending_tracepoint_resolved_during_trace): Likewise.
	(pending_tracepoint_installed_during_trace): Likewise.
	(pending_tracepoint_with_action_resolved): Likewise.
---
 gdb/gdbserver/linux-x86-low.c          |   27 +++++++++-
 gdb/gdbserver/tracepoint.c             |   24 ++++++++
 gdb/testsuite/gdb.trace/change-loc.exp |   71 ++++++++++++++++++------
 gdb/testsuite/gdb.trace/pending.exp    |   94 ++++++++++++++++++++++----------
 4 files changed, 168 insertions(+), 48 deletions(-)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 58aaf9a..ed1f8a8 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -20,6 +20,7 @@
 #include <stddef.h>
 #include <signal.h>
 #include <limits.h>
+#include <inttypes.h>
 #include "server.h"
 #include "linux-low.h"
 #include "i387-fp.h"
@@ -1200,6 +1201,8 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
 {
   unsigned char buf[40];
   int i, offset;
+  int64_t loffset;
+
   CORE_ADDR buildaddr = *jump_entry;
 
   /* Build the jump pad.  */
@@ -1323,7 +1326,17 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
   *adjusted_insn_addr_end = buildaddr;
 
   /* Finally, write a jump back to the program.  */
-  offset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
+
+  loffset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
+  if (loffset > INT_MAX || loffset < INT_MIN)
+    {
+      sprintf (err,
+	       "E.Jump back from jump pad too far from tracepoint "
+	       "(offset 0x%" PRIx64 " > int32).", loffset);
+      return 1;
+    }
+
+  offset = (int) loffset;
   memcpy (buf, jump_insn, sizeof (jump_insn));
   memcpy (buf + 1, &offset, 4);
   append_insns (&buildaddr, sizeof (jump_insn), buf);
@@ -1332,7 +1345,17 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
      is always done last (by our caller actually), so that we can
      install fast tracepoints with threads running.  This relies on
      the agent's atomic write support.  */
-  offset = *jump_entry - (tpaddr + sizeof (jump_insn));
+  loffset = *jump_entry - (tpaddr + sizeof (jump_insn));
+  if (loffset > INT_MAX || loffset < INT_MIN)
+    {
+      sprintf (err,
+	       "E.Jump pad too far from tracepoint "
+	       "(offset 0x%" PRIx64 " > int32).", loffset);
+      return 1;
+    }
+
+  offset = (int) loffset;
+
   memcpy (buf, jump_insn, sizeof (jump_insn));
   memcpy (buf + 1, &offset, 4);
   memcpy (jjump_pad_insn, buf, sizeof (jump_insn));
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 21e58ff..6ec322a 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -1684,6 +1684,28 @@ find_tracepoint (int id, CORE_ADDR addr)
   return NULL;
 }
 
+/* Remove TPOINT from global list.  */
+
+static void
+remove_tracepoint (struct tracepoint *tpoint)
+{
+  struct tracepoint *tp, *tp_prev;
+
+  for (tp = tracepoints, tp_prev = NULL; tp && tp != tpoint;
+       tp_prev = tp, tp = tp->next)
+    ;
+
+  if (tp)
+    {
+      if (tp_prev)
+	tp_prev->next = tp->next;
+      else
+	tracepoints->next = tp->next;
+
+      xfree (tp);
+    }
+}
+
 /* There may be several tracepoints with the same number (because they
    are "locations", in GDB parlance); return the next one after the
    given tracepoint, or search from the beginning of the list if the
@@ -2391,6 +2413,8 @@ cmd_qtdp (char *own_buf)
 
       download_tracepoint (tpoint);
       install_tracepoint (tpoint, own_buf);
+      if (strncmp (own_buf, "OK", 2) != 0)
+	remove_tracepoint (tpoint);
 
       unpause_all (1);
       return;
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index d6062fc..71890d7 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -98,7 +98,21 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
     gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
 	"continue to marker 1"
     # Set a tracepoint during tracing.
-    gdb_test "${trace_type} set_tracepoint" ".*" "set tracepoint on set_tracepoint"
+    set test "set tracepoint on set_tracepoint"
+    gdb_test_multiple "${trace_type} set_tracepoint" $test {
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+	    } else {
+		fail $test
+	    }
+	}
+	-re "\r\n$gdb_prompt $" {
+	    pass $test
+	}
+    }
 
     gdb_trace_setactions "set action for tracepoint" "" \
 	"collect \$$pcreg" "^$"
@@ -109,15 +123,27 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
 \[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*4\.1.* in func4.*4\.2.* in func4.*" \
 	"tracepoint with two locations"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
-	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
-	}
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+            if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+                return
+            } else {
+		fail "continue to marker 2"
+		fail $test
+            }
+
+       }
+       -re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+           pass "continue to marker 2"
+       }
     }
+
     # tracepoint has three locations after shlib change-loc-2 is loaded.
     gdb_test "info trace" \
 	"Num     Type\[ \]+Disp Enb Address\[ \]+What.*
@@ -198,19 +224,28 @@ proc tracepoint_change_loc_2 { trace_type } { with_test_prefix "2 $trace_type" {
 	"breakpoint on marker"
 
     # tracepoint with two locations will be downloaded and installed.
-    gdb_test_no_output "tstart"
-
-    gdb_test_multiple "continue" "continue to marker 1" {
-	-re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 1"
-	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 1"
-	    return
+    set test "tstart"
+    gdb_test_multiple "tstart" $test {
+        -re "^tstart\r\n$gdb_prompt $" {
+	    pass "tstart"
+        }
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+            if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+                return
+            } else {
+		fail $test
+            }
 	}
     }
 
     gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
+	"continue to marker 1"
+
+    gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
 	"continue to marker 2"
 
     # tracepoint has three locations after shlib change-loc-2 is loaded.
diff --git a/gdb/testsuite/gdb.trace/pending.exp b/gdb/testsuite/gdb.trace/pending.exp
index 017aea9..7a41b2f 100644
--- a/gdb/testsuite/gdb.trace/pending.exp
+++ b/gdb/testsuite/gdb.trace/pending.exp
@@ -132,18 +132,28 @@ proc pending_tracepoint_works { trace_type } { with_test_prefix "$trace_type wor
     gdb_test "break marker" "Breakpoint.*at.* file .*$srcfile, line.*" \
 	"breakpoint on marker"
 
-    gdb_test_no_output "tstart" "start trace experiment"
+    set test "start trace experiment"
+    gdb_test_multiple "tstart" $test {
+        -re "^tstart\r\n$gdb_prompt $" {
+	    pass $test
+        }
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+            if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+                return
+            } else {
+		fail $test
+            }
+        }
 
-    gdb_test_multiple "continue" "continue to marker" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker"
-	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker"
-	    return
-	}
     }
 
+    gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*" \
+	"continue to marker"
+
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
 
     gdb_test "tfind start" "#0 .*" "tfind test frame 0"
@@ -189,13 +199,22 @@ proc pending_tracepoint_resolved_during_trace { trace_type } \
     gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*pending.c.*" \
 	"continue to marker 1"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
 	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
+	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+	    pass $test
 	}
     }
 
@@ -253,14 +272,23 @@ proc pending_tracepoint_installed_during_trace { trace_type } \
 \[0-9\]+\[\t \]+\(fast |\)tracepoint\[ \t\]+keep y.*PENDING.*set_point2.*" \
 	"single pending tracepoint on set_point2"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
-	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
-	}
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	    -re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+               # Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+                return
+            } else {
+		fail $test
+            }
+        }
+       -re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+           pass $test
+       }
     }
 
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
@@ -423,14 +451,24 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
     gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*pending.c.*" \
 	"continue to marker 1"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	    -re "Target returns error code .* too far .*$gdb_prompt $" {
+            if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+                return
+            } else {
+		fail $test
+            }
+	}
 	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
 	    pass "continue to marker 2"
 	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
-	}
+
     }
 
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
-- 
1.7.0.4

Documenting E. packet. (was Re: [patch] Fix PR 13392 : check offset of JMP insn)

[Philippe Waroquiers]

On Tue, 2012-03-06 at 13:47 -0800, Stan Shebs wrote:

> > Is there somewhere a description of what an E. packet is,
> > and when this is allowed ?
> >
...
> It doesn't look like it's officially described in the manual.  The 
...
> 
> It's conceivable that one could do something with GDB that interprets 
> the error reply (whether NN or a string) and does something differently, 
> but in practice, if the target side is reporting errors, things are 
> going straight downhill and the user needs to decide what to do about 
> it.  Also, if a particular result is common enough that GDB has code to 
> cope with it, then maybe it's just another element of the protocol, not 
> really an error. :-)

Effectively, having GDB trying to do something sophisticated depending
on the E NN nr or E. string received from the stub looks difficult.

There is however already some kind of logic in GDB.
E.g. in remote.c, the function trace_error handles differently E.,
E1? and E2?.

There are other places where only E NN is considered as an error.
E. is not considered as an error.
Example: qRcmd.


When I did the Valgrind gdbserver, I had to return a readable error
msg to the user but did not find a way to do that in the doc.
I found the E. by reading the gdb sources and used it (believing
I was doing a nasty thing. But now, I see it is just an officially
accepted error packet, just not documented :).
=> it looks better to document this E. packet.

However, it is not clear to me what is exactly expected for
this documentation/behaviour.

It looks better if at all places where an E NN is accepted by GDB,
GDB would also accept an E. packet.

But that is not the current behaviour, so either remote.c is
changed to consistently accept E NN and E. everywhere,
or the protocol doc must match the current behaviour,
and indicate for each packet if an E NN and/or an E. error is
accepted.

Philippe

Re: Documenting E. packet. (was Re: [patch] Fix PR 13392 : check offset of JMP insn)

[Pedro Alves]

On 03/07/2012 08:41 PM, Philippe Waroquiers wrote:

> On Tue, 2012-03-06 at 13:47 -0800, Stan Shebs wrote:
> 
>>> Is there somewhere a description of what an E. packet is,
>>> and when this is allowed ?
>>>
> ...
>> It doesn't look like it's officially described in the manual.  The 
> ...
>>
>> It's conceivable that one could do something with GDB that interprets 
>> the error reply (whether NN or a string) and does something differently, 
>> but in practice, if the target side is reporting errors, things are 
>> going straight downhill and the user needs to decide what to do about 
>> it.  Also, if a particular result is common enough that GDB has code to 
>> cope with it, then maybe it's just another element of the protocol, not 
>> really an error. :-)
> 
> Effectively, having GDB trying to do something sophisticated depending
> on the E NN nr or E. string received from the stub looks difficult.
> 
> There is however already some kind of logic in GDB.
> E.g. in remote.c, the function trace_error handles differently E.,
> E1? and E2?.


Yeah, that's old legacy stuff.  :-(  We're probably safe in removing those
special E1 and E2 handlings (which all I know about is what's in the code)
by now.

See: packet_ok -> packet_check_result

      /* Always treat "E." as an error.  This will be used for
	 more verbose error messages, such as E.memtypes.  */
      if (buf[0] == 'E' && buf[1] == '.')
	return PACKET_ERROR;

So all places that use the modern packet_ok explicitly handle this.

It's very lame that we print the leading '.'.  We should fix that.

> 
> There are other places where only E NN is considered as an error.
> E. is not considered as an error.
> Example: qRcmd.


I think CS/Maciej had a patch for this.

> When I did the Valgrind gdbserver, I had to return a readable error
> msg to the user but did not find a way to do that in the doc.
> I found the E. by reading the gdb sources and used it (believing
> I was doing a nasty thing. But now, I see it is just an officially
> accepted error packet, just not documented :).
> => it looks better to document this E. packet.
> 
> However, it is not clear to me what is exactly expected for
> this documentation/behaviour.
> 
> It looks better if at all places where an E NN is accepted by GDB,
> GDB would also accept an E. packet.


Yeah, though we'd a careful audit.  We should be careful in avoiding the
case of with newer stubs sending "E.xxx" to GDBs that haven't been updated,
and those GDBs not understanding it as an error packet.

> But that is not the current behaviour, so either remote.c is
> changed to consistently accept E NN and E. everywhere,
> or the protocol doc must match the current behaviour,
> and indicate for each packet if an E NN and/or an E. error is
> accepted.


In any case, the documentation should be updated.

-- 
Pedro Alves

Re: Documenting E. packet. (was Re: [patch] Fix PR 13392 : check offset of JMP insn)

[Philippe Waroquiers]

On Wed, 2012-03-07 at 20:59 +0000, Pedro Alves wrote:
> > It looks better if at all places where an E NN is accepted by GDB,
> > GDB would also accept an E. packet.
> 
> 
> Yeah, though we'd a careful audit.  We should be careful in avoiding the
> case of with newer stubs sending "E.xxx" to GDBs that haven't been updated,
> and those GDBs not understanding it as an error packet.
Not clear to me how this can be achieved. 

E.g. if qRcmd now accepts an E. error packet in gdb 7.5,
and a stub is modified to send such an E., trying to use
this stub with gdb 7.4 or before will not give the expected behaviour
when E. is returned by the new stub to the old gdb.

Unless the new stub would have a way to know that the gdb does not
understand E. reply to qRcmd ?

> 
> > But that is not the current behaviour, so either remote.c is
> > changed to consistently accept E NN and E. everywhere,
> > or the protocol doc must match the current behaviour,
> > and indicate for each packet if an E NN and/or an E. error is
> > accepted.
> 
> 
> In any case, the documentation should be updated.
Yes, either both code and doc, or only doc.

Philippe

Re: [patch] Fix PR 13392 : check offset of JMP insn

[Pedro Alves]

On 03/07/2012 08:24 AM, Yao Qi wrote:

> On 03/07/2012 01:03 AM, Pedro Alves wrote:

>> > send a clearer error back to GDB, and we end up with:

>> > 
>> > Target returns error code '.Tracepoint 2 at 0x7ffff67c2579 already exists'.
> It is OK for me to let remote target to reports a duplicated tracepoint.


I don't know what you mean then.  It already does, but with a cryptic E01.

> In this version, some changes compared with last one,
> 
>   - move `tracepoint already exist' patch out, which can be a separate one,

>   - remove downloaded tracepoint in target when failed to install,

We should also check what happens when we do "enable TRACEPOINT_FOO", and
that tracepoints fails to be likewise inserted.  Following this direction,
we should make sure it stays disabled in the target, but _not_ deleted.

> +/* Remove TPOINT from global list.  */

> +
> +static void
> +remove_tracepoint (struct tracepoint *tpoint)
> +{
> +  struct tracepoint *tp, *tp_prev;
> +
> +  for (tp = tracepoints, tp_prev = NULL; tp && tp != tpoint;
> +       tp_prev = tp, tp = tp->next)
> +    ;
> +
> +  if (tp)
> +    {
> +      if (tp_prev)
> +	tp_prev->next = tp->next;
> +      else
> +	tracepoints->next = tp->next;


This doesn't look correct.  If there's no tp_prev, then TP must be
TRACEPOINTS, and so you need to do 'tracepoints = tp->next;'.

Or just rewrite this as:

static int
remove_tracepoint (struct tracepoint *todel)
{
  struct tracepoint *tp, **tp_link;

  tp = tracepoints;
  tp_link = &tracepoints;

  while (tp != NULL)
    {
      if (tp == todel)
	{
	  *tp_link = tp->next;

	  xfree (tp);
	  return;
	}

	tp_link = &tp->next;
	tp = *tp_link;
    }
}


>  /* There may be several tracepoints with the same number (because they
>     are "locations", in GDB parlance); return the next one after the
>     given tracepoint, or search from the beginning of the list if the
> @@ -2391,6 +2413,8 @@ cmd_qtdp (char *own_buf)
>  
>        download_tracepoint (tpoint);
>        install_tracepoint (tpoint, own_buf);
> +      if (strncmp (own_buf, "OK", 2) != 0)
> +	remove_tracepoint (tpoint);


own_buf is a \0 terminated string.  You can make that a straight strcmp(own_buf, "OK").
(Or change install_tracepoint's return type to int, to signal error.)

> +	-re "Target returns error code .* too far .*$gdb_prompt $" {
> +            if [string equal $trace_type "ftrace"] {
> +		# The target was unable to install the fast tracepoint
> +		# (e.g., jump pad too far from tracepoint).
> +		pass "$test (too far)"
> +		# Skip the rest of the tests.
> +                return


Can you check tab/spaces here, and in the similar places, please?  I may
have messed that up myself.

Otherwise okay.

-- 
Pedro Alves

Re: [patch] Fix PR 13392 : check offset of JMP insn

[Yao Qi]

[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]

On 03/09/2012 12:26 AM, Pedro Alves wrote:
>> >   - remove downloaded tracepoint in target when failed to install,
> We should also check what happens when we do "enable TRACEPOINT_FOO", and
> that tracepoints fails to be likewise inserted.  Following this direction,
> we should make sure it stays disabled in the target, but _not_ deleted.
> 

AFAIK, tracepoint is not inserted when "enable tracepoint", so we don't
have to worry about the fails of inserting tracepoints in "enable
tracepoint".  Tracepoints are downloaded and installed in QTDP or
QTStart.  For QTenable/QTDisable, it simply changes the flag `enable' in
tracepoint.  Of course, we may get fails during QTenable/QTDisable, but
fails are not caused by inserting/installing tracepoint.

>> > +/* Remove TPOINT from global list.  */
>> > +
>> > +static void
>> > +remove_tracepoint (struct tracepoint *tpoint)
>> > +{
>> > +  struct tracepoint *tp, *tp_prev;
>> > +
>> > +  for (tp = tracepoints, tp_prev = NULL; tp && tp != tpoint;
>> > +       tp_prev = tp, tp = tp->next)
>> > +    ;
>> > +
>> > +  if (tp)
>> > +    {
>> > +      if (tp_prev)
>> > +	tp_prev->next = tp->next;
>> > +      else
>> > +	tracepoints->next = tp->next;
> 
> This doesn't look correct.  If there's no tp_prev, then TP must be
> TRACEPOINTS, and so you need to do 'tracepoints = tp->next;'.
> 

Oops, sorry.  I should stop coding at mid-night.  Fixed.

Regression tested on x86_64-linux.  Committed.
http://sourceware.org/ml/gdb-cvs/2012-03/msg00146.html

-- 
Yao (齐尧)

[-- Attachment #2: 0001-fix-pr13392-check-offset-of-jmp-insn.patch --]
[-- Type: text/x-patch, Size: 12261 bytes --]


2012-03-08  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	Fix PR server/13392.
	* linux-x86-low.c (amd64_install_fast_tracepoint_jump_pad): Check
	offset of JMP insn.
	* tracepoint.c (remove_tracepoint): New.
	(cmd_qtdp): Call remove_tracepoint when failed to install.

2012-03-08  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	Fix PR server/13392.
	* gdb.trace/change-loc.exp (tracepoint_change_loc_1): Remove kfail.
	(tracepoint_change_loc_2): Remove kfail.  Return if failed to
	download tracepoints.
	* gdb.trace/pending.exp (pending_tracepoint_works): Likewise.
	(pending_tracepoint_resolved_during_trace): Likewise.
	(pending_tracepoint_installed_during_trace): Likewise.
	(pending_tracepoint_with_action_resolved): Likewise.
---
 gdb/gdbserver/linux-x86-low.c          |   27 +++++++++-
 gdb/gdbserver/tracepoint.c             |   24 +++++++++
 gdb/testsuite/gdb.trace/change-loc.exp |   71 +++++++++++++++++++------
 gdb/testsuite/gdb.trace/pending.exp    |   90 ++++++++++++++++++++++---------
 4 files changed, 166 insertions(+), 46 deletions(-)

diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 58aaf9a..ed1f8a8 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -20,6 +20,7 @@
 #include <stddef.h>
 #include <signal.h>
 #include <limits.h>
+#include <inttypes.h>
 #include "server.h"
 #include "linux-low.h"
 #include "i387-fp.h"
@@ -1200,6 +1201,8 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
 {
   unsigned char buf[40];
   int i, offset;
+  int64_t loffset;
+
   CORE_ADDR buildaddr = *jump_entry;
 
   /* Build the jump pad.  */
@@ -1323,7 +1326,17 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
   *adjusted_insn_addr_end = buildaddr;
 
   /* Finally, write a jump back to the program.  */
-  offset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
+
+  loffset = (tpaddr + orig_size) - (buildaddr + sizeof (jump_insn));
+  if (loffset > INT_MAX || loffset < INT_MIN)
+    {
+      sprintf (err,
+	       "E.Jump back from jump pad too far from tracepoint "
+	       "(offset 0x%" PRIx64 " > int32).", loffset);
+      return 1;
+    }
+
+  offset = (int) loffset;
   memcpy (buf, jump_insn, sizeof (jump_insn));
   memcpy (buf + 1, &offset, 4);
   append_insns (&buildaddr, sizeof (jump_insn), buf);
@@ -1332,7 +1345,17 @@ amd64_install_fast_tracepoint_jump_pad (CORE_ADDR tpoint, CORE_ADDR tpaddr,
      is always done last (by our caller actually), so that we can
      install fast tracepoints with threads running.  This relies on
      the agent's atomic write support.  */
-  offset = *jump_entry - (tpaddr + sizeof (jump_insn));
+  loffset = *jump_entry - (tpaddr + sizeof (jump_insn));
+  if (loffset > INT_MAX || loffset < INT_MIN)
+    {
+      sprintf (err,
+	       "E.Jump pad too far from tracepoint "
+	       "(offset 0x%" PRIx64 " > int32).", loffset);
+      return 1;
+    }
+
+  offset = (int) loffset;
+
   memcpy (buf, jump_insn, sizeof (jump_insn));
   memcpy (buf + 1, &offset, 4);
   memcpy (jjump_pad_insn, buf, sizeof (jump_insn));
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 21e58ff..7bf576b 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -1684,6 +1684,28 @@ find_tracepoint (int id, CORE_ADDR addr)
   return NULL;
 }
 
+/* Remove TPOINT from global list.  */
+
+static void
+remove_tracepoint (struct tracepoint *tpoint)
+{
+  struct tracepoint *tp, *tp_prev;
+
+  for (tp = tracepoints, tp_prev = NULL; tp && tp != tpoint;
+       tp_prev = tp, tp = tp->next)
+    ;
+
+  if (tp)
+    {
+      if (tp_prev)
+	tp_prev->next = tp->next;
+      else
+	tracepoints = tp->next;
+
+      xfree (tp);
+    }
+}
+
 /* There may be several tracepoints with the same number (because they
    are "locations", in GDB parlance); return the next one after the
    given tracepoint, or search from the beginning of the list if the
@@ -2391,6 +2413,8 @@ cmd_qtdp (char *own_buf)
 
       download_tracepoint (tpoint);
       install_tracepoint (tpoint, own_buf);
+      if (strcmp (own_buf, "OK") != 0)
+	remove_tracepoint (tpoint);
 
       unpause_all (1);
       return;
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index d6062fc..a5a982f 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -98,7 +98,21 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
     gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
 	"continue to marker 1"
     # Set a tracepoint during tracing.
-    gdb_test "${trace_type} set_tracepoint" ".*" "set tracepoint on set_tracepoint"
+    set test "set tracepoint on set_tracepoint"
+    gdb_test_multiple "${trace_type} set_tracepoint" $test {
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+	    } else {
+		fail $test
+	    }
+	}
+	-re "\r\n$gdb_prompt $" {
+	    pass $test
+	}
+    }
 
     gdb_trace_setactions "set action for tracepoint" "" \
 	"collect \$$pcreg" "^$"
@@ -109,15 +123,27 @@ proc tracepoint_change_loc_1 { trace_type } { with_test_prefix "1 $trace_type" {
 \[0-9\]+\[\t \]+\(|fast \)tracepoint\[ \]+keep y.*\<MULTIPLE\>.*4\.1.* in func4.*4\.2.* in func4.*" \
 	"tracepoint with two locations"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
-	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
-	}
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+            if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+                return
+            } else {
+		fail "continue to marker 2"
+		fail $test
+            }
+
+       }
+       -re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+           pass "continue to marker 2"
+       }
     }
+
     # tracepoint has three locations after shlib change-loc-2 is loaded.
     gdb_test "info trace" \
 	"Num     Type\[ \]+Disp Enb Address\[ \]+What.*
@@ -198,19 +224,28 @@ proc tracepoint_change_loc_2 { trace_type } { with_test_prefix "2 $trace_type" {
 	"breakpoint on marker"
 
     # tracepoint with two locations will be downloaded and installed.
-    gdb_test_no_output "tstart"
-
-    gdb_test_multiple "continue" "continue to marker 1" {
-	-re ".*Breakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 1"
-	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 1"
-	    return
+    set test "tstart"
+    gdb_test_multiple "tstart" $test {
+        -re "^tstart\r\n$gdb_prompt $" {
+	    pass "tstart"
+        }
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+            if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+		return
+            } else {
+		fail $test
+            }
 	}
     }
 
     gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
+	"continue to marker 1"
+
+    gdb_test "continue" ".*Breakpoint.*marker.*at.*$srcfile.*" \
 	"continue to marker 2"
 
     # tracepoint has three locations after shlib change-loc-2 is loaded.
diff --git a/gdb/testsuite/gdb.trace/pending.exp b/gdb/testsuite/gdb.trace/pending.exp
index 017aea9..4e7dc31 100644
--- a/gdb/testsuite/gdb.trace/pending.exp
+++ b/gdb/testsuite/gdb.trace/pending.exp
@@ -132,18 +132,28 @@ proc pending_tracepoint_works { trace_type } { with_test_prefix "$trace_type wor
     gdb_test "break marker" "Breakpoint.*at.* file .*$srcfile, line.*" \
 	"breakpoint on marker"
 
-    gdb_test_no_output "tstart" "start trace experiment"
-
-    gdb_test_multiple "continue" "continue to marker" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker"
+    set test "start trace experiment"
+    gdb_test_multiple "tstart" $test {
+	-re "^tstart\r\n$gdb_prompt $" {
+	    pass $test
 	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker"
-	    return
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# The target was unable to install the fast tracepoint
+		# (e.g., jump pad too far from tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
 	}
+
     }
 
+    gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*" \
+	"continue to marker"
+
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
 
     gdb_test "tfind start" "#0 .*" "tfind test frame 0"
@@ -189,13 +199,22 @@ proc pending_tracepoint_resolved_during_trace { trace_type } \
     gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*pending.c.*" \
 	"continue to marker 1"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
 	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
+	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+	    pass $test
 	}
     }
 
@@ -253,14 +272,23 @@ proc pending_tracepoint_installed_during_trace { trace_type } \
 \[0-9\]+\[\t \]+\(fast |\)tracepoint\[ \t\]+keep y.*PENDING.*set_point2.*" \
 	"single pending tracepoint on set_point2"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
-	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
-	    pass "continue to marker 2"
-	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	-re "Target returns error code .* too far .*$gdb_prompt $" {
+	    if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+		return
+	    } else {
+		fail $test
+	    }
 	}
+	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
+           pass $test
+       }
     }
 
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
@@ -423,14 +451,24 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
     gdb_test "continue" "Continuing.\r\n\r\nBreakpoint.*marker.*at.*pending.c.*" \
 	"continue to marker 1"
 
-    gdb_test_multiple "continue" "continue to marker 2" {
+    set test "continue to marker 2"
+    gdb_test_multiple "continue" $test {
+	    -re "Target returns error code .* too far .*$gdb_prompt $" {
+            if [string equal $trace_type "ftrace"] {
+		# Expected if the target was unable to install the
+		# fast tracepoint (e.g., jump pad too far from
+		# tracepoint).
+		pass "$test (too far)"
+		# Skip the rest of the tests.
+		return
+            } else {
+		fail $test
+            }
+	}
 	-re "Continuing.\r\n\r\nBreakpoint.*marker.*at.*$srcfile.*$gdb_prompt $" {
 	    pass "continue to marker 2"
 	}
-	-re ".*$gdb_prompt $" {
-	    kfail "gdb/13392" "continue to marker 2"
-	    return
-	}
+
     }
 
     gdb_test "tstop" "\[\r\n\]+" "stop trace experiment"
-- 
1.7.0.4