This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/5] Make breakpoint and breakpoint_len local variables in GDBServer.

On 09/28/2015 06:05 AM, Eli Zaretskii wrote:
From: Yao Qi <>
Cc: <>
Date: Mon, 28 Sep 2015 10:54:48 +0100

After thinking about it a bit more, I think this reveals some design
issues in GDBserver brekapoint, nowadays, GDBserver inserts its own
breakpoints and breakpoints requested by GDB.  After this patch series,
GDBserver should be able to:

   1) choose the right breakpoint instruction for its own breakpoints,
   according to the breakpoint address, status register flag, etc,
   through API set_breakpoint_at,
   2) choose the right breakpoint instruction for breakpoints requested
   by GDB, according to the information in Z packets, through API

there should be two paths for them, and each path needs different target
hook to choose breakpoint instructions.  breakpoint_from_pc is needed for
#1, and breakpoint_from_length is needed for #2.  In your current patch
set (in patch 4/5), two things are mixed together, which doesn't look
good to me.  The current functions calls in GDBserver to create
breakpoint is like

      `--> set_breakpoint
                     `--> the_target->insert_point

we are handling the breakpoint length at the lowest level, in
insert_memory_breakpoint, and we use breakpoint_from_pc and
breakpoint_from_length together there, which looks unclean.  Ideally, we
can move these code handling breakpoint length (breakpoint_from_pc and
breakpoint_from_length) to upper levels, like this,

   set_breakpoint_at (call breakpoint_from_pc)
   set_gdb_breakpoint_1 (call breakpoint_from_length)
      `--> set_breakpoint
                     `--> the_target->insert_point

all needed information is saved in struct breakpoint or struct
raw_breakpoint, and function set_breakpoint and it callees can use
breakpoint or raw_breakpoint directly.  It'll be cleaner in this way,
let me know what do you think?

Sometimes only the target layer knows how to choose the length
correctly.  Are we sure this isn't one of those cases, and can never

breakpoint_from_pc and breakpoint_from_length are target ops so indeed only the target knows but it's ok we can query the target properly.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]