[RFC] [PATCH] Let user set breakpoint with mouse.

[Hannes Domani]

 Am Freitag, 18. Juni 2021, 14:30:09 MESZ hat Daniel Levin via Gdb-patches <gdb-patches@sourceware.org> Folgendes geschrieben:

> Greetings all
>
> This is my first GDB patch.
>
> I implemented the feature proposed by #23632. The user can now click on a line in either the source or disasm windows in tui to toggle a breakpoint there.
>
> There is a good chance the large number of arguments passed to create_breakpoint is erroneous in an edge-case I have yet to find, which is why there is an RFC in the subject line.
>
> I look forward to feedback, comments and so on.

Just FYI, I also sent something similar to this some time ago:
https://sourceware.org/pipermail/gdb-patches/2021-March/176814.html

It worked slightly different, a left-click where a breakpoint already was,
would disable it, and only a middle-click would delete it.

But yours includes the disasm window, that's better.

I haven't tried yours yet, but with mine I had the problem that once I
created so many breakpoints that the command window shows the pagination
question, gdb would crash when trying to continue from there.
I'm wondering if this happens here as well.


Hannes


>
> Best,
>
> Daniel Levin
>
> ---
> gdb/breakpoint.c        | 16 +++++++++++++++-
> gdb/breakpoint.h        |  5 +++++
> gdb/tui/tui-winsource.c | 33 +++++++++++++++++++++++++++++++++
> gdb/tui/tui-winsource.h |  8 ++++++++
> 4 files changed, 61 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index fb011fc1e0f..43900573fe5 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -79,7 +79,6 @@
> #include <algorithm>
> #include "progspace-and-thread.h"
> #include "gdbsupport/array-view.h"
> -#include "gdbsupport/gdb_optional.h"
>
> /* Prototypes for local functions.  */
>
> @@ -4036,6 +4035,21 @@ breakpoint_init_inferior (enum inf_context context)
>   moribund_locations.clear ();
> }
>
> +gdb::optional<bp_location *>
> +get_breakpoint_here (const address_space *aspace, CORE_ADDR pc)
> +{
> +  for (bp_location *bl : all_bp_locations ())
> +    {
> +      if (bl->loc_type != bp_loc_software_breakpoint
> +      && bl->loc_type != bp_loc_hardware_breakpoint)
> +    continue;
> +
> +      if (breakpoint_location_address_match (bl, aspace, pc))
> +        return bl;
> +    }
> +  return {};
> +}
> +
> /* These functions concern about actual breakpoints inserted in the
>     target --- to e.g. check if we need to do decr_pc adjustment or if
>     we need to hop over the bkpt --- so we check for address space
> diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
> index e40504f14ed..533b0eae5e1 100644
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -30,6 +30,7 @@
> #include "gdbsupport/array-view.h"
> #include "gdbsupport/filtered-iterator.h"
> #include "gdbsupport/function-view.h"
> +#include "gdbsupport/gdb_optional.h"
> #include "gdbsupport/refcounted-object.h"
> #include "gdbsupport/safe-iterator.h"
> #include "cli/cli-script.h"
> @@ -1227,6 +1228,10 @@ enum breakpoint_here
>
> /* Prototypes for breakpoint-related functions.  */
>
> +/* Box possible breakpoint at the given location in an optional type */
> +extern gdb::optional<bp_location *>
> +get_breakpoint_here (const address_space *, CORE_ADDR);
> +
> extern enum breakpoint_here breakpoint_here_p (const address_space *,
>                           CORE_ADDR);
>
> diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
> index afd51e95980..a4754bfa5f8 100644
> --- a/gdb/tui/tui-winsource.c
> +++ b/gdb/tui/tui-winsource.c
> @@ -524,3 +524,36 @@ tui_source_window_base::update_exec_info ()
>     }
>   refresh_window ();
> }
> +
> +static CORE_ADDR
> +resolve_to_addr (tui_source_element *src_element)
> +{
> +  if (src_element->line_or_addr.loa == LOA_ADDRESS)
> +      return src_element->line_or_addr.u.addr;
> +
> +  CORE_ADDR loc_of_src_line;
> +  struct symtab_and_line cursal = get_current_source_symtab_and_line ();
> +  find_line_pc (cursal.symtab, src_element->line_or_addr.u.line_no, &loc_of_src_line);
> +  return loc_of_src_line;
> +}
> +
> +void
> +tui_source_window_base::toggle_bp_at_line (tui_source_element *src_element) {
> +  CORE_ADDR clicked_addr = resolve_to_addr (src_element);
> +  auto possible_prev_bp = get_breakpoint_here (current_program_space->aspace, clicked_addr);
> +  if (possible_prev_bp.has_value () && (*possible_prev_bp)->owner != nullptr)
> +    delete_breakpoint ((*possible_prev_bp)->owner);
> +  else
> +    {
> +      auto here = new_address_location (clicked_addr, nullptr, 0);
> +      create_breakpoint (m_gdbarch,
> +            here.get(), NULL, -1, NULL, false,
> +            0,
> +            0, bp_breakpoint,
> +            0,
> +            AUTO_BOOLEAN_TRUE,
> +            &bkpt_breakpoint_ops,
> +            0, 1,
> +            0, 0);
> +    }
> +}
> diff --git a/gdb/tui/tui-winsource.h b/gdb/tui/tui-winsource.h
> index d4528e351f4..6eee0c72b43 100644
> --- a/gdb/tui/tui-winsource.h
> +++ b/gdb/tui/tui-winsource.h
> @@ -124,6 +124,14 @@ struct tui_source_window_base : public tui_win_info
>   /* Redraw the complete line of a source or disassembly window.  */
>   void show_source_line (int lineno);
>
> +  void click (int mouse_x, int mouse_y, int mouse_button) override
> +  {
> +    if (mouse_button == 1 && mouse_y < m_content.size ())
> +      toggle_bp_at_line (&m_content[mouse_y]);
> +  }
> +
> +  void toggle_bp_at_line (tui_source_element *src_element);
> +
>   /* Used for horizontal scroll.  */
>   int m_horizontal_offset = 0;
>   struct tui_line_or_address m_start_line_or_addr;
> --
> 2.31.1