Recognize more program breakpoint patterns

New in v3:

- Code cleanups based on reviews.

New in v2:

- Fixed misc problems based on reviews.
- Switched to using gdbarch_program_breakpoint_here_p as opposed to
gdbarch_insn_is_breakpoint.
- Fixed matching of brk instructions. Previously the mask was incorrect, which
was showing up as a few failures in the testsuite. Now it is clean.
- New testcase (separate patch).
- Moved program_breakpoint_here () to arch-utils.c and made it the default
implementation of gdbarch_program_breakpoint_here_p.

--

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_program_breakpoint_here_p)
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)

gdb/ChangeLog:

2020-01-29  Luis Machado  <luis.machado@linaro.org>

	* aarch64-tdep.c (BRK_INSN_MASK): Define to 0xffe0001f.
	(BRK_INSN_MASK): Define to 0xd4200000.
	(aarch64_program_breakpoint_here_p): New function.
	(aarch64_gdbarch_init): Set gdbarch_program_breakpoint_here_p hook.
	* arch-utils.c (default_program_breakpoint_here_p): Moved from
	breakpoint.c.
	* arch-utils.h (default_program_breakpoint_here_p): Moved from
	breakpoint.h
	* breakpoint.c (bp_loc_is_permanent): Changed return type to bool and
	call gdbarch_program_breakpoint_here_p.
	(program_breakpoint_here): Moved to arch-utils.c, renamed to
	default_program_breakpoint_here_p, changed return type to bool and
	simplified.
	* breakpoint.h (program_breakpoint_here): Moved prototype to
	arch-utils.h, renamed to default_program_breakpoint_here_p and changed
	return type to bool.
	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* gdbarch.sh (program_breakpoint_here_p): New method.
	* infrun.c (handle_signal_stop): Call
	gdbarch_program_breakpoint_here_p.

1 files changed, 7 insertions, 39 deletions

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a7a378131a9..5a9352c26fe 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8515,7 +8515,7 @@ mention (struct breakpoint *b)
 }
 
 
-static int bp_loc_is_permanent (struct bp_location *loc);
+static bool bp_loc_is_permanent (struct bp_location *loc);
 
 static struct bp_location *
 add_location_to_breakpoint (struct breakpoint *b,
@@ -8581,42 +8581,10 @@ add_location_to_breakpoint (struct breakpoint *b,
 }
 
 
-/* See breakpoint.h.  */
-
-int
-program_breakpoint_here_p (struct gdbarch *gdbarch, CORE_ADDR address)
-{
-  int len;
-  CORE_ADDR addr;
-  const gdb_byte *bpoint;
-  gdb_byte *target_mem;
-
-  addr = address;
-  bpoint = gdbarch_breakpoint_from_pc (gdbarch, &addr, &len);
-
-  /* Software breakpoints unsupported?  */
-  if (bpoint == NULL)
-    return 0;
-
-  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.  */
-  scoped_restore restore_memory
-    = make_scoped_restore_show_memory_breakpoints (0);
+/* Return true if LOC is pointing to a permanent breakpoint,
+   return false otherwise.  */
 
-  if (target_read_memory (address, target_mem, len) == 0
-      && memcmp (target_mem, bpoint, len) == 0)
-    return 1;
-
-  return 0;
-}
-
-/* Return 1 if LOC is pointing to a permanent breakpoint,
-   return 0 otherwise.  */
-
-static int
+static bool
 bp_loc_is_permanent (struct bp_location *loc)
 {
   gdb_assert (loc != NULL);
@@ -8624,14 +8592,14 @@ bp_loc_is_permanent (struct bp_location *loc)
   /* If we have a non-breakpoint-backed catchpoint or a software
      watchpoint, just return 0.  We should not attempt to read from
      the addresses the locations of these breakpoint types point to.
-     program_breakpoint_here_p, below, will attempt to read
+     gdbarch_program_breakpoint_here_p, below, will attempt to read
      memory.  */
   if (!bl_address_is_meaningful (loc))
-    return 0;
+    return false;
 
   scoped_restore_current_pspace_and_thread restore_pspace_thread;
   switch_to_program_space_and_thread (loc->pspace);
-  return program_breakpoint_here_p (loc->gdbarch, loc->address);
+  return gdbarch_program_breakpoint_here_p (loc->gdbarch, loc->address);
 }
 
 /* Build a command list for the dprintf corresponding to the current