Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Antoine Tremblay <antoine.tremblay@ericsson.com>
To: <gdb-patches@sourceware.org>
Cc: Antoine Tremblay <antoine.tremblay@ericsson.com>
Subject: [PATCH 2/5] Make breakpoint and breakpoint_len local variables in GDBServer.
Date: Fri, 18 Sep 2015 12:02:00 -0000	[thread overview]
Message-ID: <1442577749-6650-3-git-send-email-antoine.tremblay@ericsson.com> (raw)
In-Reply-To: <1442577749-6650-1-git-send-email-antoine.tremblay@ericsson.com>

This patch is in preparation for software breakpoints on ARM linux.

It refactors the breakpoint and breakpoint_len global variables to be local
so that multiple types of breakpoints can be used for an arch.

One important implementation detail is the introduction of the pcfull field
in struct raw_breakpoint.

In order to be able to reinsert a breakpoint we need to remember what were the
flags encoded in the PC, however since functions that compare the program pc
to the breakpoint pc expect an unencoded memory address, we can't put the
encoded value directly in the pc field.

So this patch introduces the pcfull field that contains the flags encoded
in the pc so that we can properly reinsert a breakpoint that has its type
information encoded in the pc.  pcfull shall only be used when
inserting/removing/reinserting a breakpoint, all other breakpoint->pc
references can remain the same.

Note this is for software breakpoints only, when using hardware breakpoints
then pcfull is not set or used.

No regressions on Ubuntu 14.04 on ARMv7 and x86.
With gdbserver-{native,extended} / { -marm -mthumb }

gdbserver/ChangeLog:
	* linux-low.c (initialize_low): Remove
        breakpoint_data initialization.
	* mem-break.c (struct raw_breakpoint): Add pcfull.
	(insert_memory_breakpoint): Call breakpoint_from_pc.
	(remove_memory_breakpoint): Likewise.
	(set_raw_breakpoint_at): Likewise.
	(set_breakpoint_at): Set default breakpoint size to 0.
	(set_breakpoint_data): Remove.
	(validate_inserted_breakpoint): Call breakpoint_from_pc.
	(check_mem_read): Call breakpoint_from_pc.
	(check_mem_write): Call breakpoint_from_pc.
	(clone_one_breakpoint): Copy pcfull field.
---
 gdb/gdbserver/linux-low.c |  6 ----
 gdb/gdbserver/mem-break.c | 76 ++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 58 insertions(+), 24 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index bb08761..06387a0 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -7069,16 +7069,10 @@ void
 initialize_low (void)
 {
   struct sigaction sigchld_action;
-  int breakpoint_len = 0;
-  const unsigned char *breakpoint = NULL;
 
   memset (&sigchld_action, 0, sizeof (sigchld_action));
   set_target_ops (&linux_target_ops);
 
-  breakpoint = the_target->breakpoint_from_pc (NULL, &breakpoint_len);
-
-  set_breakpoint_data (breakpoint,
-		       breakpoint_len);
   linux_init_signals ();
   linux_ptrace_init_warnings ();
 
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 9356741..68e14c3 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -21,8 +21,6 @@
 #include "server.h"
 #include "regcache.h"
 #include "ax.h"
-const unsigned char *breakpoint_data;
-int breakpoint_len;
 
 #define MAX_BREAKPOINT_LEN 8
 
@@ -100,6 +98,10 @@ struct raw_breakpoint
      breakpoint for a given PC.  */
   CORE_ADDR pc;
 
+  /* The breakpoint's insertion address, possibly with flags encoded in the pc
+     (e.g. the instruction mode on ARM).  */
+  CORE_ADDR pcfull;
+
   /* The breakpoint's size.  */
   int size;
 
@@ -300,6 +302,12 @@ insert_memory_breakpoint (struct raw_breakpoint *bp)
 {
   unsigned char buf[MAX_BREAKPOINT_LEN];
   int err;
+  const unsigned char *breakpoint_data;
+  int breakpoint_len;
+  CORE_ADDR pc;
+
+  pc = bp->pcfull;
+  breakpoint_data = the_target->breakpoint_from_pc (&pc, &breakpoint_len);
 
   if (breakpoint_data == NULL)
     return 1;
@@ -349,6 +357,11 @@ remove_memory_breakpoint (struct raw_breakpoint *bp)
 {
   unsigned char buf[MAX_BREAKPOINT_LEN];
   int err;
+  int breakpoint_len;
+  CORE_ADDR pc;
+
+  pc = bp->pcfull;
+  the_target->breakpoint_from_pc (&pc, &breakpoint_len);
 
   /* Since there can be trap breakpoints inserted in the same address
      range, we use `write_inferior_memory', which takes care of
@@ -375,15 +388,27 @@ remove_memory_breakpoint (struct raw_breakpoint *bp)
    returns NULL and writes the error code to *ERR.  */
 
 static struct raw_breakpoint *
-set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int size,
+set_raw_breakpoint_at (enum raw_bkpt_type type, const CORE_ADDR where, int size,
 		       int *err)
 {
   struct process_info *proc = current_process ();
   struct raw_breakpoint *bp;
+  CORE_ADDR pc;
+  int breakpoint_len;
+
+  /* pc could be modified by breakpoint_from_pc, use the modified
+     version to find breakpoints and use the full where pc for
+     insert_point so that arch specific data can be passed.  */
+  pc = where;
+
+  the_target->breakpoint_from_pc (&pc, &breakpoint_len);
+
+  if (size == 0)
+    size = breakpoint_len;
 
   if (type == raw_bkpt_type_sw || type == raw_bkpt_type_hw)
     {
-      bp = find_enabled_raw_code_breakpoint_at (where, type);
+      bp = find_enabled_raw_code_breakpoint_at (pc, type);
       if (bp != NULL && bp->size != size)
 	{
 	  /* A different size than previously seen.  The previous
@@ -396,7 +421,7 @@ set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int size,
 	}
     }
   else
-    bp = find_raw_breakpoint_at (where, type, size);
+    bp = find_raw_breakpoint_at (pc, type, size);
 
   if (bp != NULL)
     {
@@ -405,7 +430,8 @@ set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int size,
     }
 
   bp = XCNEW (struct raw_breakpoint);
-  bp->pc = where;
+  bp->pc = pc;
+  bp->pcfull = where;
   bp->size = size;
   bp->refcount = 1;
   bp->raw_type = type;
@@ -774,8 +800,9 @@ set_breakpoint_at (CORE_ADDR where, int (*handler) (CORE_ADDR))
 {
   int err_ignored;
 
+  /* default breakpoint_len will be initialized downstream.  */
   return set_breakpoint (other_breakpoint, raw_bkpt_type_sw,
-			 where, breakpoint_len, handler,
+			 where, 0, handler,
 			 &err_ignored);
 }
 
@@ -1588,13 +1615,6 @@ check_breakpoints (CORE_ADDR stop_pc)
     }
 }
 
-void
-set_breakpoint_data (const unsigned char *bp_data, int bp_len)
-{
-  breakpoint_data = bp_data;
-  breakpoint_len = bp_len;
-}
-
 int
 breakpoint_here (CORE_ADDR addr)
 {
@@ -1665,6 +1685,13 @@ validate_inserted_breakpoint (struct raw_breakpoint *bp)
 {
   unsigned char *buf;
   int err;
+  const unsigned char *breakpoint_data;
+  int breakpoint_len;
+  CORE_ADDR raw_pc;
+
+  raw_pc = bp->pcfull;
+
+  breakpoint_data = the_target->breakpoint_from_pc (&raw_pc, &breakpoint_len);
 
   gdb_assert (bp->inserted);
   gdb_assert (bp->raw_type == raw_bkpt_type_sw);
@@ -1762,10 +1789,15 @@ check_mem_read (CORE_ADDR mem_addr, unsigned char *buf, int mem_len)
 
   for (; bp != NULL; bp = bp->next)
     {
-      CORE_ADDR bp_end = bp->pc + breakpoint_len;
-      CORE_ADDR start, end;
+      int breakpoint_len;
+      CORE_ADDR raw_pc;
+      CORE_ADDR bp_end, start, end;
       int copy_offset, copy_len, buf_offset;
 
+      raw_pc = bp->pcfull;
+      the_target->breakpoint_from_pc (&raw_pc, &breakpoint_len);
+      bp_end = bp->pc + breakpoint_len;
+
       if (bp->raw_type != raw_bkpt_type_sw)
 	continue;
 
@@ -1851,10 +1883,17 @@ check_mem_write (CORE_ADDR mem_addr, unsigned char *buf,
 
   for (; bp != NULL; bp = bp->next)
     {
-      CORE_ADDR bp_end = bp->pc + breakpoint_len;
-      CORE_ADDR start, end;
+      int breakpoint_len;
+      const unsigned char *breakpoint_data;
+      CORE_ADDR raw_pc;
+      CORE_ADDR bp_end, start, end;
       int copy_offset, copy_len, buf_offset;
 
+      raw_pc = bp->pcfull;
+      breakpoint_data =
+	the_target->breakpoint_from_pc (&raw_pc, &breakpoint_len);
+      bp_end = bp->pc + breakpoint_len;
+
       if (bp->raw_type != raw_bkpt_type_sw)
 	continue;
 
@@ -1963,6 +2002,7 @@ clone_one_breakpoint (const struct breakpoint *src)
   dest_raw->raw_type = src->raw->raw_type;
   dest_raw->refcount = src->raw->refcount;
   dest_raw->pc = src->raw->pc;
+  dest_raw->pcfull = src->raw->pcfull;
   dest_raw->size = src->raw->size;
   memcpy (dest_raw->old_data, src->raw->old_data, MAX_BREAKPOINT_LEN);
   dest_raw->inserted = src->raw->inserted;
-- 
1.9.1


  reply	other threads:[~2015-09-18 12:02 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-18 12:02 [PATCH 0/5] Software breakpoints support for ARM linux Antoine Tremblay
2015-09-18 12:02 ` Antoine Tremblay [this message]
2015-09-28  9:55   ` [PATCH 2/5] Make breakpoint and breakpoint_len local variables in GDBServer Yao Qi
2015-09-28 10:05     ` Eli Zaretskii
2015-09-28 21:01       ` Antoine Tremblay
2015-09-28 20:59     ` Antoine Tremblay
2015-09-18 12:03 ` [PATCH 4/5] Handle breakpoint kinds for software breakpoints " Antoine Tremblay
2015-09-28 10:33   ` Yao Qi
2015-09-29 11:55     ` Antoine Tremblay
2015-09-18 12:03 ` [PATCH 3/5] Add support for ARM breakpoint types " Antoine Tremblay
2015-09-28 10:29   ` Yao Qi
2015-09-28 21:26     ` Antoine Tremblay
2015-09-29  8:32       ` Yao Qi
2015-09-29 11:38         ` Antoine Tremblay
2015-09-29 11:43           ` Yao Qi
2015-09-18 12:03 ` [PATCH 1/5] Support multiple breakpoint types per target " Antoine Tremblay
2015-09-23 10:51   ` Yao Qi
2015-09-23 12:37     ` Antoine Tremblay
2015-09-23 14:46       ` Yao Qi
2015-09-23 14:56         ` Antoine Tremblay
2015-09-18 12:03 ` [PATCH 5/5] Support software breakpoints for ARM linux " Antoine Tremblay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1442577749-6650-3-git-send-email-antoine.tremblay@ericsson.com \
    --to=antoine.tremblay@ericsson.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox