Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 0/2,v2][AArch64]Handle additional brk instruction patterns
@ 2020-01-13 17:25 Luis Machado
  2020-01-13 17:25 ` [PATCH 1/2][AArch64] Recognize more program breakpoint patterns Luis Machado
  2020-01-13 17:30 ` [PATCH 2/2][AArch64] Test handling of additional brk instruction patterns Luis Machado
  0 siblings, 2 replies; 8+ messages in thread
From: Luis Machado @ 2020-01-13 17:25 UTC (permalink / raw)
  To: gdb-patches; +Cc: alan.hayward, tankut.baris.aktemur

This patch teaches GDB how to handle additional breakpoint instruction
patterns for aarch64.

Luis Machado (2):
  [AArch64] Recognize more program breakpoint patterns
  [AArch64] Test handling of additional brk instruction patterns

 gdb/aarch64-tdep.c                            | 39 +++++++++++
 gdb/arch-utils.c                              | 37 ++++++++++
 gdb/arch-utils.h                              |  4 ++
 gdb/breakpoint.c                              | 46 ++-----------
 gdb/breakpoint.h                              |  5 --
 gdb/gdbarch.c                                 | 23 +++++++
 gdb/gdbarch.h                                 |  7 ++
 gdb/gdbarch.sh                                |  4 ++
 gdb/infrun.c                                  |  4 +-
 gdb/testsuite/gdb.arch/aarch64-brk-patterns.c | 30 +++++++++
 .../gdb.arch/aarch64-brk-patterns.exp         | 67 +++++++++++++++++++
 11 files changed, 220 insertions(+), 46 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-brk-patterns.c
 create mode 100644 gdb/testsuite/gdb.arch/aarch64-brk-patterns.exp

-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH] [AArch64] Recognize more program breakpoint patterns
@ 2019-12-23 17:34 Luis Machado
  2020-01-13 17:03 ` [PATCH 1/2] " Luis Machado
  0 siblings, 1 reply; 8+ messages in thread
From: Luis Machado @ 2019-12-23 17:34 UTC (permalink / raw)
  To: gdb-patches

It was reported to me that program breakpoints (permanent ones inserted into
the code itself) other than the one GDB uses for AArch64 (0xd4200000) do not
generate visible stops when continuing, and GDB will continue spinning
infinitely.

This happens because GDB, upon hitting one of those program breakpoints, thinks
the SIGTRAP came from a delayed breakpoint hit...

(gdb) x/i $pc
=> 0x4005c0 <problem_function>: brk     #0x90f
(gdb) c
Continuing.
infrun: clear_proceed_status_thread (process 14198)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 14198
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 14198] at 0x4005c0
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun:   14198.14198.0 [process 14198],
infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: handle_inferior_event status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: stop_pc = 0x4005c0
infrun: delayed software breakpoint trap, ignoring
infrun: no stepping, continue
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 14198] at 0x4005c0
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun:   14198.14198.0 [process 14198],
infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: handle_inferior_event status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: stop_pc = 0x4005c0
infrun: delayed software breakpoint trap, ignoring
infrun: no stepping, continue
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 14198] at 0x4005c0
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun:   14198.14198.0 [process 14198],
infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: handle_inferior_event status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: stop_pc = 0x4005c0
infrun: delayed software breakpoint trap, ignoring
infrun: no stepping, continue
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 14198] at 0x4005c0
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun:   14198.14198.0 [process 14198],
infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: handle_inferior_event status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: stop_pc = 0x4005c0
infrun: delayed software breakpoint trap, ignoring
infrun: no stepping, continue
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 14198] at 0x4005c0
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun:   14198.14198.0 [process 14198],
infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
...

... which is not the case.

If the program breakpoint is one GDB recognizes, then it will stop when it
hits it.

(gdb) x/i $pc
=> 0x4005c0 <problem_function>: brk     #0x0
(gdb) c
Continuing.
infrun: clear_proceed_status_thread (process 14193)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 14193
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 14193] at 0x4005c0
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun:   14193.14193.0 [process 14193],
infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: handle_inferior_event status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: stop_pc = 0x4005c0
infrun: random signal (GDB_SIGNAL_TRAP)
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun:   process 14193 not executing
infrun: stop_all_threads, pass=1, iterations=1
infrun:   process 14193 not executing
infrun: stop_all_threads done

Program received signal SIGTRAP, Trace/breakpoint trap.
problem_function () at brk_0.c:7
7        asm("brk %0\n\t" ::"n"(0x0));
infrun: infrun_async(0)

Otherwise GDB will keep trying to resume the inferior and will keep
seeing the SIGTRAP's, without stopping.

To the user it appears GDB has gone into an infinite loop, interruptible only
by Ctrl-C.

Also, windbg seems to use a different variation of AArch64 breakpoint compared
to GDB. This causes problems when debugging Windows on ARM binaries, when
program breakpoints are being used.

The proposed patch creates a new gdbarch method (gdbarch_insn_is_breakpoint)
that tells GDB whether the underlying instruction is a breakpoint instruction
or not.

This is more general than only checking for the instruction GDB uses as
breakpoint.

The existing logic is still preserved for targets that do not implement this
new gdbarch method.

The end result is like so:

(gdb) x/i $pc
=> 0x4005c0 <problem_function>: brk     #0x90f
(gdb) c
Continuing.
infrun: clear_proceed_status_thread (process 16417)
infrun: proceed (addr=0xffffffffffffffff, signal=GDB_SIGNAL_DEFAULT)
infrun: proceed: resuming process 16417
infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 16417] at 0x4005c0
infrun: infrun_async(1)
infrun: prepare_to_wait
infrun: target_wait (-1.0.0, status) =
infrun:   16417.16417.0 [process 16417],
infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: handle_inferior_event status->kind = stopped, signal = GDB_SIGNAL_TRAP
infrun: stop_pc = 0x4005c0
infrun: random signal (GDB_SIGNAL_TRAP)
infrun: stop_waiting
infrun: stop_all_threads
infrun: stop_all_threads, pass=0, iterations=0
infrun:   process 16417 not executing
infrun: stop_all_threads, pass=1, iterations=1
infrun:   process 16417 not executing
infrun: stop_all_threads done

Program received signal SIGTRAP, Trace/breakpoint trap.
problem_function () at brk.c:7
7        asm("brk %0\n\t" ::"n"(0x900 + 0xf));
infrun: infrun_async(0)

Does this change look ok?

gdb/ChangeLog:

2019-12-23  Luis Machado  <luis.machado@linaro.org>

	* aarch64-tdep.c (BRK_INSN_MASK): Define to 0xd4200000.
	(aarch64_insn_is_breakpoint): New function.
	(aarch64_gdbarch_init): Set gdbarch_insn_is_breakpoint hook.
	* arch-utils.c (default_insn_is_breakpoint): New function.
	* arch-utils.h (default_insn_is_breakpoint): New prototype.
	* breakpoint.c (program_breakpoint_here): Updated to use
	  gdbarch_insn_is_breakpoint.
	Update documentation to clarify behavior.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* gdbarch.sh (gdbarch_insn_is_breakpoint): New method.

Change-Id: I96eb27151442f435560a58c87eac48b0f68432bc
---
 gdb/aarch64-tdep.c | 25 +++++++++++++++++++++++++
 gdb/arch-utils.c   |  7 +++++++
 gdb/arch-utils.h   |  3 +++
 gdb/breakpoint.c   | 19 ++++++++++++++-----
 gdb/gdbarch.c      | 23 +++++++++++++++++++++++
 gdb/gdbarch.h      |  7 +++++++
 gdb/gdbarch.sh     |  4 ++++
 7 files changed, 83 insertions(+), 5 deletions(-)

diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 1d5fb2001d..c69361d4ea 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1201,6 +1201,28 @@ aarch64_execute_dwarf_cfa_vendor_op (struct gdbarch *gdbarch, gdb_byte op,
   return false;
 }
 
+#define BRK_INSN_MASK 0xd4200000
+
+/* Implementation of gdbarch_insn_is_breakpoint for aarch64.  */
+
+static bool
+aarch64_insn_is_breakpoint (gdbarch *gdbarch,
+			    const gdb_byte *insn,
+			    unsigned int insn_size)
+{
+  gdb_assert (insn != nullptr);
+
+  uint32_t i;
+
+  i = (uint32_t) extract_unsigned_integer (insn, insn_size,
+					   gdbarch_byte_order (gdbarch));
+
+  /* Check if INSN is a BRK instruction pattern.  There are multiple choices
+     of such instructions with different immediate values.  Different OS' may
+     use a different variation, but they have the same outcome.  */
+  return (i & BRK_INSN_MASK) == BRK_INSN_MASK;
+}
+
 /* When arguments must be pushed onto the stack, they go on in reverse
    order.  The code below implements a FILO (stack) to do this.  */
 
@@ -3357,6 +3379,9 @@ aarch64_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_execute_dwarf_cfa_vendor_op (gdbarch,
 					   aarch64_execute_dwarf_cfa_vendor_op);
 
+  /* Permanent/Program breakpoint handling.  */
+  set_gdbarch_insn_is_breakpoint (gdbarch, aarch64_insn_is_breakpoint);
+
   /* Add some default predicates.  */
   frame_unwind_append_unwinder (gdbarch, &aarch64_stub_unwind);
   dwarf2_append_unwinders (gdbarch);
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index a1a003f91f..99c9f281be 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -876,6 +876,13 @@ int default_insn_is_jump (struct gdbarch *gdbarch, CORE_ADDR addr)
   return 0;
 }
 
+bool default_insn_is_breakpoint (struct gdbarch *gdbarch,
+				 const gdb_byte  *insn,
+				 unsigned int insn_size)
+{
+  return false;
+}
+
 void
 default_skip_permanent_breakpoint (struct regcache *regcache)
 {
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 48ff3bb9a1..77ffe8190c 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -227,6 +227,9 @@ extern int default_return_in_first_hidden_param_p (struct gdbarch *,
 extern int default_insn_is_call (struct gdbarch *, CORE_ADDR);
 extern int default_insn_is_ret (struct gdbarch *, CORE_ADDR);
 extern int default_insn_is_jump (struct gdbarch *, CORE_ADDR);
+extern bool default_insn_is_breakpoint (struct gdbarch *gdbarch,
+					const gdb_byte *insn,
+					unsigned int insn_size);
 
 /* Do-nothing version of vsyscall_range.  Returns false.  */
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 904abda8db..ffb7f7f8be 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8599,14 +8599,23 @@ program_breakpoint_here_p (struct gdbarch *gdbarch, CORE_ADDR address)
   target_mem = (gdb_byte *) alloca (len);
 
   /* Enable the automatic memory restoration from breakpoints while
-     we read the memory.  Otherwise we could say about our temporary
-     breakpoints they are permanent.  */
+     we read the memory.  Otherwise we may find temporary breakpoints, ones
+     inserted by GDB, and flag them as permanent breakpoints.  */
   scoped_restore restore_memory
     = make_scoped_restore_show_memory_breakpoints (0);
 
-  if (target_read_memory (address, target_mem, len) == 0
-      && memcmp (target_mem, bpoint, len) == 0)
-    return 1;
+  if (target_read_memory (address, target_mem, len) == 0)
+    {
+      /* Check if this is a breakpoint instruction for this architecture,
+	 including ones used by GDB. 
+
+	 Some architectures have more than one possible breakpoint
+	 instruction, but GDB does not use all of them.  We should detect those
+	 as well.  */
+      if (gdbarch_insn_is_breakpoint (gdbarch, target_mem, len)
+	  ||  memcmp (target_mem, bpoint, len) == 0)
+	return 1;
+    }
 
   return 0;
 }
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 59c97da985..9e21ba7eca 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -345,6 +345,7 @@ struct gdbarch
   gdbarch_insn_is_call_ftype *insn_is_call;
   gdbarch_insn_is_ret_ftype *insn_is_ret;
   gdbarch_insn_is_jump_ftype *insn_is_jump;
+  gdbarch_insn_is_breakpoint_ftype *insn_is_breakpoint;
   gdbarch_auxv_parse_ftype *auxv_parse;
   gdbarch_print_auxv_entry_ftype *print_auxv_entry;
   gdbarch_vsyscall_range_ftype *vsyscall_range;
@@ -464,6 +465,7 @@ gdbarch_alloc (const struct gdbarch_info *info,
   gdbarch->insn_is_call = default_insn_is_call;
   gdbarch->insn_is_ret = default_insn_is_ret;
   gdbarch->insn_is_jump = default_insn_is_jump;
+  gdbarch->insn_is_breakpoint = default_insn_is_breakpoint;
   gdbarch->print_auxv_entry = default_print_auxv_entry;
   gdbarch->vsyscall_range = default_vsyscall_range;
   gdbarch->infcall_mmap = default_infcall_mmap;
@@ -708,6 +710,7 @@ verify_gdbarch (struct gdbarch *gdbarch)
   /* Skip verify of insn_is_call, invalid_p == 0 */
   /* Skip verify of insn_is_ret, invalid_p == 0 */
   /* Skip verify of insn_is_jump, invalid_p == 0 */
+  /* Skip verify of insn_is_breakpoint, invalid_p == 0 */
   /* Skip verify of auxv_parse, has predicate.  */
   /* Skip verify of print_auxv_entry, invalid_p == 0 */
   /* Skip verify of vsyscall_range, invalid_p == 0 */
@@ -1137,6 +1140,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   fprintf_unfiltered (file,
                       "gdbarch_dump: inner_than = <%s>\n",
                       host_address_to_string (gdbarch->inner_than));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: insn_is_breakpoint = <%s>\n",
+                      host_address_to_string (gdbarch->insn_is_breakpoint));
   fprintf_unfiltered (file,
                       "gdbarch_dump: insn_is_call = <%s>\n",
                       host_address_to_string (gdbarch->insn_is_call));
@@ -4928,6 +4934,23 @@ set_gdbarch_insn_is_jump (struct gdbarch *gdbarch,
   gdbarch->insn_is_jump = insn_is_jump;
 }
 
+bool
+gdbarch_insn_is_breakpoint (struct gdbarch *gdbarch, const gdb_byte *insn, unsigned int insn_size)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->insn_is_breakpoint != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_insn_is_breakpoint called\n");
+  return gdbarch->insn_is_breakpoint (gdbarch, insn, insn_size);
+}
+
+void
+set_gdbarch_insn_is_breakpoint (struct gdbarch *gdbarch,
+                                gdbarch_insn_is_breakpoint_ftype insn_is_breakpoint)
+{
+  gdbarch->insn_is_breakpoint = insn_is_breakpoint;
+}
+
 int
 gdbarch_auxv_parse_p (struct gdbarch *gdbarch)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 78e05ecdcb..d94950b8f2 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1545,6 +1545,13 @@ typedef int (gdbarch_insn_is_jump_ftype) (struct gdbarch *gdbarch, CORE_ADDR add
 extern int gdbarch_insn_is_jump (struct gdbarch *gdbarch, CORE_ADDR addr);
 extern void set_gdbarch_insn_is_jump (struct gdbarch *gdbarch, gdbarch_insn_is_jump_ftype *insn_is_jump);
 
+/* Return true if INSN of size INSN_SIZE acts like a permanent breakpoint and
+   false otherwise. */
+
+typedef bool (gdbarch_insn_is_breakpoint_ftype) (struct gdbarch *gdbarch, const gdb_byte *insn, unsigned int insn_size);
+extern bool gdbarch_insn_is_breakpoint (struct gdbarch *gdbarch, const gdb_byte *insn, unsigned int insn_size);
+extern void set_gdbarch_insn_is_breakpoint (struct gdbarch *gdbarch, gdbarch_insn_is_breakpoint_ftype *insn_is_breakpoint);
+
 /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
    Return 0 if *READPTR is already at the end of the buffer.
    Return -1 if there is insufficient buffer for a whole entry.
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 331eb39278..dbf31ee8ae 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1152,6 +1152,10 @@ m;int;insn_is_ret;CORE_ADDR addr;addr;;default_insn_is_ret;;0
 # Return non-zero if the instruction at ADDR is a jump; zero otherwise.
 m;int;insn_is_jump;CORE_ADDR addr;addr;;default_insn_is_jump;;0
 
+# Return true if INSN of size INSN_SIZE acts like a permanent breakpoint and
+# false otherwise.
+m;bool;insn_is_breakpoint;const gdb_byte *insn, unsigned int insn_size;insn, insn_size;;default_insn_is_breakpoint;;0
+
 # Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
 # Return 0 if *READPTR is already at the end of the buffer.
 # Return -1 if there is insufficient buffer for a whole entry.
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-14 15:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-13 17:25 [PATCH 0/2,v2][AArch64]Handle additional brk instruction patterns Luis Machado
2020-01-13 17:25 ` [PATCH 1/2][AArch64] Recognize more program breakpoint patterns Luis Machado
2020-01-13 17:30 ` [PATCH 2/2][AArch64] Test handling of additional brk instruction patterns Luis Machado
2020-01-14  5:03   ` Simon Marchi
2020-01-14 13:45     ` Luis Machado
2020-01-14 15:37       ` Simon Marchi
     [not found]     ` <BYAPR11MB3030A0EBC8F50F7B7B0F2081C4340@BYAPR11MB3030.namprd11.prod.outlook.com>
2020-01-14 13:59       ` Luis Machado
  -- strict thread matches above, loose matches on Subject: below --
2019-12-23 17:34 [PATCH] [AArch64] Recognize more program breakpoint patterns Luis Machado
2020-01-13 17:03 ` [PATCH 1/2] " Luis Machado
2020-01-13 17:06   ` [PATCH 2/2] [AArch64] Test handling of additional brk instruction patterns Luis Machado

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox