foss-for-synopsys-dwc-arc-processors / binutils-gdb

A mirror of the upstream binutils-gdb repository for ARC specific work
GNU General Public License v2.0
13 stars 10 forks source link

[GDB] The breakpoint "kind" should be meaningful #42

Open shahab-vahedi opened 4 years ago

shahab-vahedi commented 4 years ago

In GDB and GDBserver code, we have target hooks that need a breakpoint kind to figure what to do:

gdbserver/linux-arc-low.cc
arc_target::sw_breakpoint_from_kind (int kind, int *size)

gdb/arc-tdep.c
arc_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
arc_sw_breakpoint_from_kind (struct gdbarch *gdbarch, int kind, int *size)

gdb/arc-linux-tdep.c
arc_linux_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
arc_linux_sw_breakpoint_from_kind (struct gdbarch *gdbarch, int kind, int *size)

However, looking at the implementation of these codes, it becomes obvious that we are piggy backing the breakpoint size through the kind. This is at best confusing.

The correct way to handle this is to have an enum and a map to resolve the problem:

enum arc_breakpoint_types {
  ARC_TRAP_S,
  ARC_BRK_S,
  ...
}

std::map<arc_breakpoint_types, uint8_t> = {
  {ARC_TRAP_S, 2},
  {ARC_BRK_S, 2},
   ...
}
vineetgarc commented 4 years ago

I just noticed your break point enum: please note we also have SWI_S (ARCv2 on wards) which is semantically different from TRAP_S.

SWI_S doesn't commit before exception is taken (so PC remains unchanged and exception returns back to orig PC. OTOH TRAP_S comit sbefore exception so PC moves to next PC for return. Thus SWI_S is cleaner to implement breakpointing in general as we always have the orig context for say removing the bkpt after say single-step.

When working on Linux kprobes infrastructure, we were emulating the No-commit semantics using an unimplemented instruction opcode. Per my request this was formalized into ISA as a new SWI_S instruction but I never got around to propagating this in software. Perhaps we can start doing this for gdb if you feel it helps.

shahab-vahedi commented 4 years ago

It definitely helps and I plan to do that. I will create a separate issue for that.

It is interesting that you're mentioning this. On a different topic, this advancing/committing trip_s instruction raised some questions. You can find the question here (look for stop_pc) and my respond here.