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

[Yao Qi <yao@codesourcery.com>]

[-- 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