[PATCH 1/2] This patch fixes GDBServer's run control for single stepping
Antoine Tremblay
antoine.tremblay@ericsson.com
Mon Apr 3 13:18:00 GMT 2017
Yao Qi writes:
> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
>> I think maybe the best solution would be to abstract only that part of
>> get_next_pc in a function: the if block starting with if
>> (self->has_thumb2_breakpoint) around line 301.
>>
>> And have only that part return the next_pc + the breakpoint kind, this
>> would avoid breaking all the virtual get_next_pc functions just for that
>> case and allow the same code to be used in kind_from_current_state.
>>
>> We'll still redo the work but at least the code will be in one
>> place. WDYT ?
>
> That should be fine, although I am not exactly sure what are you
> going to do.
It looks like this, comments ?:
----
commit 9a8b46f9038e9d3805c8e6ec1bdf0dff1c95c065
Author: Antoine Tremblay <antoine.tremblay@ericsson.com>
Date: Mon Apr 3 09:13:20 2017 -0400
refactor get-next-pc
---
gdb/arch/arm-get-next-pcs.c | 199 ++++++++++++++++++++++----------------
gdb/arch/arm-get-next-pcs.h | 9 ++
gdb/gdbserver/linux-aarch32-low.c | 46 ++++++++-
gdb/gdbserver/linux-aarch32-low.h | 3 +
gdb/gdbserver/linux-arm-low.c | 21 ----
gdb/gdbserver/mem-break.c | 16 ++-
6 files changed, 188 insertions(+), 106 deletions(-)
diff --git a/gdb/arch/arm-get-next-pcs.c b/gdb/arch/arm-get-next-pcs.c
index 398dd59a..d967e16 100644
--- a/gdb/arch/arm-get-next-pcs.c
+++ b/gdb/arch/arm-get-next-pcs.c
@@ -22,6 +22,7 @@
#include "common-regcache.h"
#include "arm.h"
#include "arm-get-next-pcs.h"
+#include <vector>
/* See arm-get-next-pcs.h. */
@@ -258,6 +259,115 @@ arm_deal_with_atomic_sequence_raw (struct arm_get_next_pcs *self)
return next_pcs;
}
+/* See arm-get-next-pcs.h. */
+
+std::vector <std::pair<CORE_ADDR, arm_breakpoint_kinds>>
+thumb_get_next_pcs_raw_itblock
+(CORE_ADDR pc, unsigned short inst1,
+ ULONGEST status, ULONGEST itstate,
+ ULONGEST (*read_mem_uint) (CORE_ADDR memaddr, int len, int byte_order),
+ int byte_order_for_code)
+{
+ std::vector <std::pair <CORE_ADDR, arm_breakpoint_kinds>> next_pcs;
+
+ if ((inst1 & 0xff00) == 0xbf00 && (inst1 & 0x000f) != 0)
+ {
+ /* An IT instruction. Because this instruction does not
+ modify the flags, we can accurately predict the next
+ executed instruction. */
+ itstate = inst1 & 0x00ff;
+ pc += thumb_insn_size (inst1);
+
+ while (itstate != 0 && ! condition_true (itstate >> 4, status))
+ {
+ inst1 = read_mem_uint (pc, 2,byte_order_for_code);
+ pc += thumb_insn_size (inst1);
+ itstate = thumb_advance_itstate (itstate);
+ }
+ next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
+ (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
+ return next_pcs;
+ }
+ else if (itstate != 0)
+ {
+ /* We are in a conditional block. Check the condition. */
+ if (! condition_true (itstate >> 4, status))
+ {
+ /* Advance to the next executed instruction. */
+ pc += thumb_insn_size (inst1);
+ itstate = thumb_advance_itstate (itstate);
+
+ while (itstate != 0 && ! condition_true (itstate >> 4, status))
+ {
+ inst1 = read_mem_uint (pc, 2, byte_order_for_code);
+
+ pc += thumb_insn_size (inst1);
+ itstate = thumb_advance_itstate (itstate);
+ }
+
+ next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
+ (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
+ return next_pcs;
+ }
+ else if ((itstate & 0x0f) == 0x08)
+ {
+ /* This is the last instruction of the conditional
+ block, and it is executed. We can handle it normally
+ because the following instruction is not conditional,
+ and we must handle it normally because it is
+ permitted to branch. Fall through. */
+ }
+ else
+ {
+ int cond_negated;
+
+ /* There are conditional instructions after this one.
+ If this instruction modifies the flags, then we can
+ not predict what the next executed instruction will
+ be. Fortunately, this instruction is archi2tecturally
+ forbidden to branch; we know it will fall through.
+ Start by skipping past it. */
+ pc += thumb_insn_size (inst1);
+ itstate = thumb_advance_itstate (itstate);
+
+ /* Set a breakpoint on the following instruction. */
+ gdb_assert ((itstate & 0x0f) != 0);
+ next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
+ (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
+
+ cond_negated = (itstate >> 4) & 1;
+
+ /* Skip all following instructions with the same
+ condition. If there is a later instruction in the IT
+ block with the opposite condition, set the other
+ breakpoint there. If not, then set a breakpoint on
+ the instruction after the IT block. */
+ do
+ {
+ inst1 = read_mem_uint (pc, 2, byte_order_for_code);
+ pc += thumb_insn_size (inst1);
+ itstate = thumb_advance_itstate (itstate);
+ }
+ while (itstate != 0 && ((itstate >> 4) & 1) == cond_negated);
+
+ if (itstate != 0 && ((itstate >> 4) & 1) == cond_negated)
+ {
+ next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
+ (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB2));
+ }
+ else
+ {
+ next_pcs.push_back (std::pair<CORE_ADDR, arm_breakpoint_kinds>
+ (MAKE_THUMB_ADDR (pc), ARM_BP_KIND_THUMB));
+ }
+
+ return next_pcs;
+ }
+ }
+
+ return next_pcs;
+}
+
/* Find the next possible PCs for thumb mode. */
static VEC (CORE_ADDR) *
@@ -300,89 +410,14 @@ thumb_get_next_pcs_raw (struct arm_get_next_pcs *self)
if (self->has_thumb2_breakpoint)
{
- if ((inst1 & 0xff00) == 0xbf00 && (inst1 & 0x000f) != 0)
- {
- /* An IT instruction. Because this instruction does not
- modify the flags, we can accurately predict the next
- executed instruction. */
- itstate = inst1 & 0x00ff;
- pc += thumb_insn_size (inst1);
-
- while (itstate != 0 && ! condition_true (itstate >> 4, status))
- {
- inst1 = self->ops->read_mem_uint (pc, 2,byte_order_for_code);
- pc += thumb_insn_size (inst1);
- itstate = thumb_advance_itstate (itstate);
- }
-
- VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc));
- return next_pcs;
- }
- else if (itstate != 0)
- {
- /* We are in a conditional block. Check the condition. */
- if (! condition_true (itstate >> 4, status))
- {
- /* Advance to the next executed instruction. */
- pc += thumb_insn_size (inst1);
- itstate = thumb_advance_itstate (itstate);
+ std::vector <std::pair <CORE_ADDR, arm_breakpoint_kinds>> itblock_next_pcs
+ = thumb_get_next_pcs_raw_itblock
+ (pc, inst1, status, itstate, self->ops->read_mem_uint,
+ byte_order_for_code);
- while (itstate != 0 && ! condition_true (itstate >> 4, status))
- {
- inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
-
- pc += thumb_insn_size (inst1);
- itstate = thumb_advance_itstate (itstate);
- }
-
- VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc));
- return next_pcs;
- }
- else if ((itstate & 0x0f) == 0x08)
- {
- /* This is the last instruction of the conditional
- block, and it is executed. We can handle it normally
- because the following instruction is not conditional,
- and we must handle it normally because it is
- permitted to branch. Fall through. */
- }
- else
- {
- int cond_negated;
-
- /* There are conditional instructions after this one.
- If this instruction modifies the flags, then we can
- not predict what the next executed instruction will
- be. Fortunately, this instruction is architecturally
- forbidden to branch; we know it will fall through.
- Start by skipping past it. */
- pc += thumb_insn_size (inst1);
- itstate = thumb_advance_itstate (itstate);
-
- /* Set a breakpoint on the following instruction. */
- gdb_assert ((itstate & 0x0f) != 0);
- VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc));
-
- cond_negated = (itstate >> 4) & 1;
-
- /* Skip all following instructions with the same
- condition. If there is a later instruction in the IT
- block with the opposite condition, set the other
- breakpoint there. If not, then set a breakpoint on
- the instruction after the IT block. */
- do
- {
- inst1 = self->ops->read_mem_uint (pc, 2, byte_order_for_code);
- pc += thumb_insn_size (inst1);
- itstate = thumb_advance_itstate (itstate);
- }
- while (itstate != 0 && ((itstate >> 4) & 1) == cond_negated);
-
- VEC_safe_push (CORE_ADDR, next_pcs, MAKE_THUMB_ADDR (pc));
-
- return next_pcs;
- }
- }
+ for(auto const &nextpc : itblock_next_pcs) {
+ VEC_safe_push (CORE_ADDR, next_pcs, nextpc.first);
+ }
}
else if (itstate & 0x0f)
{
diff --git a/gdb/arch/arm-get-next-pcs.h b/gdb/arch/arm-get-next-pcs.h
index 2300ac1..830844b 100644
--- a/gdb/arch/arm-get-next-pcs.h
+++ b/gdb/arch/arm-get-next-pcs.h
@@ -20,6 +20,7 @@
#ifndef ARM_GET_NEXT_PCS_H
#define ARM_GET_NEXT_PCS_H 1
#include "gdb_vecs.h"
+#include <vector>
/* Forward declaration. */
struct arm_get_next_pcs;
@@ -63,4 +64,12 @@ void arm_get_next_pcs_ctor (struct arm_get_next_pcs *self,
/* Find the next possible PCs after the current instruction executes. */
VEC (CORE_ADDR) *arm_get_next_pcs (struct arm_get_next_pcs *self);
+/* Return the next possible PCs after PC if those are in a thumb2 it block. */
+std::vector <std::pair <CORE_ADDR, arm_breakpoint_kinds>>
+thumb_get_next_pcs_raw_itblock
+(CORE_ADDR pc, unsigned short inst1,
+ ULONGEST status, ULONGEST itstate,
+ ULONGEST (*read_mem_uint) (CORE_ADDR memaddr, int len, int byte_order),
+ int byte_order_for_code);
+
#endif /* ARM_GET_NEXT_PCS_H */
diff --git a/gdb/gdbserver/linux-aarch32-low.c b/gdb/gdbserver/linux-aarch32-low.c
index 2b710ba..ff4869f 100644
--- a/gdb/gdbserver/linux-aarch32-low.c
+++ b/gdb/gdbserver/linux-aarch32-low.c
@@ -18,6 +18,7 @@
#include "server.h"
#include "arch/arm.h"
#include "arch/arm-linux.h"
+#include "arch/arm-get-next-pcs.h"
#include "linux-low.h"
#include "linux-aarch32-low.h"
@@ -28,6 +29,8 @@
#include <elf.h>
#endif
+#include <vector>
+
/* Correct in either endianness. */
#define arm_abi_breakpoint 0xef9f0001UL
@@ -287,8 +290,31 @@ arm_breakpoint_kind_from_current_state (CORE_ADDR *pcptr)
{
if (arm_is_thumb_mode ())
{
- *pcptr = MAKE_THUMB_ADDR (*pcptr);
- return arm_breakpoint_kind_from_pc (pcptr);
+ struct regcache *regcache = get_thread_regcache (current_thread, 1);
+ CORE_ADDR pc = regcache_read_pc (regcache);
+ ULONGEST status, itstate;
+ unsigned short inst1;
+
+ *pcptr = UNMAKE_THUMB_ADDR (*pcptr);
+
+ inst1 = get_next_pcs_read_memory_unsigned_integer
+ (pc, 2, 0);
+ status = regcache_raw_get_unsigned (regcache, ARM_PS_REGNUM);
+ itstate = ((status >> 8) & 0xfc) | ((status >> 25) & 0x3);
+
+ std::vector <std::pair <CORE_ADDR, arm_breakpoint_kinds>> itblock_next_pcs
+ = thumb_get_next_pcs_raw_itblock
+ (pc, inst1, status, itstate,
+ get_next_pcs_read_memory_unsigned_integer,
+ 0);
+
+ /* If this PC is in an itblock let thumb_get_next_pcs_raw_itblock
+ decide the kind. */
+ for(auto const &nextpc : itblock_next_pcs) {
+ if (MAKE_THUMB_ADDR (*pcptr) == nextpc.first)
+ return nextpc.second;
+ }
+ return ARM_BP_KIND_THUMB;
}
else
{
@@ -296,6 +322,22 @@ arm_breakpoint_kind_from_current_state (CORE_ADDR *pcptr)
}
}
+/* Read memory from the inferiror.
+ BYTE_ORDER is ignored and there to keep compatiblity with GDB's
+ read_memory_unsigned_integer. */
+ULONGEST
+get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
+ int len,
+ int byte_order)
+{
+ ULONGEST res;
+
+ res = 0;
+ target_read_memory (memaddr, (unsigned char *) &res, len);
+
+ return res;
+}
+
void
initialize_low_arch_aarch32 (void)
{
diff --git a/gdb/gdbserver/linux-aarch32-low.h b/gdb/gdbserver/linux-aarch32-low.h
index fecfcbe..77fca32 100644
--- a/gdb/gdbserver/linux-aarch32-low.h
+++ b/gdb/gdbserver/linux-aarch32-low.h
@@ -27,6 +27,9 @@ int arm_breakpoint_kind_from_pc (CORE_ADDR *pcptr);
const gdb_byte *arm_sw_breakpoint_from_kind (int kind , int *size);
int arm_breakpoint_kind_from_current_state (CORE_ADDR *pcptr);
int arm_breakpoint_at (CORE_ADDR where);
+ULONGEST get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
+ int len,
+ int byte_order);
void initialize_low_arch_aarch32 (void);
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index fc2b348..fc6b5cc 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -139,11 +139,6 @@ static int arm_regmap[] = {
64
};
-/* Forward declarations needed for get_next_pcs ops. */
-static ULONGEST get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
- int len,
- int byte_order);
-
static CORE_ADDR get_next_pcs_addr_bits_remove (struct arm_get_next_pcs *self,
CORE_ADDR val);
@@ -252,22 +247,6 @@ get_next_pcs_is_thumb (struct arm_get_next_pcs *self)
return arm_is_thumb_mode ();
}
-/* Read memory from the inferiror.
- BYTE_ORDER is ignored and there to keep compatiblity with GDB's
- read_memory_unsigned_integer. */
-static ULONGEST
-get_next_pcs_read_memory_unsigned_integer (CORE_ADDR memaddr,
- int len,
- int byte_order)
-{
- ULONGEST res;
-
- res = 0;
- target_read_memory (memaddr, (unsigned char *) &res, len);
-
- return res;
-}
-
/* Fetch the thread-local storage pointer for libthread_db. */
ps_err_e
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 6e6926a..f3845cf 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -855,7 +855,21 @@ set_breakpoint_type_at (enum bkpt_type type, CORE_ADDR where,
{
int err_ignored;
CORE_ADDR placed_address = where;
- int breakpoint_kind = target_breakpoint_kind_from_pc (&placed_address);
+ int breakpoint_kind;
+
+ /* Get the kind of breakpoint to PLACED_ADDRESS except single-step
+ breakpoint. Get the kind of single-step breakpoint according to
+ the current register state. */
+ if (type == single_step_breakpoint)
+ {
+ breakpoint_kind
+ = target_breakpoint_kind_from_current_state (&placed_address);
+ }
+ else
+ {
+ breakpoint_kind
+ = target_breakpoint_kind_from_pc (&placed_address);
+ }
return set_breakpoint (type, raw_bkpt_type_sw,
placed_address, breakpoint_kind, handler,
More information about the Gdb-patches
mailing list