* [PATCH] GDB (s12z): Improve reliability of the stack unwinder.
@ 2019-04-25 13:45 John Darrington
2019-05-09 4:59 ` [PING] " John Darrington
0 siblings, 1 reply; 5+ messages in thread
From: John Darrington @ 2019-04-25 13:45 UTC (permalink / raw)
To: gdb-patches; +Cc: John Darrington
Previously, the stack unwinder searched through consecutive bytes for values
which it thought might be the start of a stack mutating operation.
This was error prone, because such bytes could also be the operands of other
instructions. This change uses the opcodes api to interpret the code in each
frame.
gdb/ChangeLog:
* s12z-tdep.c (push_pull_get_stack_adjustment): New function.
(s12z_frame_cache): Use opcodes API to interpret stack frame code.
---
gdb/s12z-tdep.c | 227 ++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 164 insertions(+), 63 deletions(-)
diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
index cef92d8774..b80509f33e 100644
--- a/gdb/s12z-tdep.c
+++ b/gdb/s12z-tdep.c
@@ -30,6 +30,7 @@
#include "opcode/s12z.h"
#include "trad-frame.h"
#include "remote.h"
+#include "opcodes/s12z-opc.h"
/* Two of the registers included in S12Z_N_REGISTERS are
the CCH and CCL "registers" which are just views into
@@ -162,6 +163,97 @@ s12z_disassemble_info (struct gdbarch *gdbarch)
return di;
}
+
+struct mem_read_abstraction
+{
+ struct mem_read_abstraction_base base;
+ bfd_vma memaddr;
+ struct disassemble_info* info;
+};
+
+static void
+advance (struct mem_read_abstraction_base *b)
+{
+ struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
+ mra->memaddr++;
+}
+
+static bfd_vma
+posn (struct mem_read_abstraction_base *b)
+{
+ struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
+ return mra->memaddr;
+}
+
+static int
+abstract_read_memory (struct mem_read_abstraction_base *b,
+ int offset,
+ size_t n, bfd_byte *bytes)
+{
+ struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
+
+ int status =
+ (*mra->info->read_memory_func) (mra->memaddr + offset,
+ bytes, n, mra->info);
+
+ if (status != 0)
+ {
+ (*mra->info->memory_error_func) (status, mra->memaddr, mra->info);
+ return -1;
+ }
+
+ return 0;
+}
+
+
+/* Return the stack adjustment caused by
+ a push or pull instruction. */
+static int
+push_pull_get_stack_adjustment (int n_operands,
+ struct operand *const *operands)
+{
+ int stack_adjustment = 0;
+ gdb_assert (n_operands > 0);
+ if (operands[0]->cl == OPND_CL_REGISTER_ALL)
+ stack_adjustment = 26; /* All the regs */
+ else if (operands[0]->cl == OPND_CL_REGISTER_ALL16)
+ stack_adjustment = 4 * 2; /* All 4 16 bit regs */
+ else
+ for (int i = 0; i < n_operands; ++i)
+ {
+ if (operands[i]->cl != OPND_CL_REGISTER)
+ continue; /* I don't think this can ever happen. */
+ const struct register_operand *op
+ = (const struct register_operand *) operands[i];
+ switch (op->reg)
+ {
+ case REG_X:
+ case REG_Y:
+ stack_adjustment += 3;
+ break;
+ case REG_D7:
+ case REG_D6:
+ stack_adjustment += 4;
+ break;
+ case REG_D2:
+ case REG_D3:
+ case REG_D4:
+ case REG_D5:
+ stack_adjustment += 2;
+ break;
+ case REG_D0:
+ case REG_D1:
+ case REG_CCL:
+ case REG_CCH:
+ stack_adjustment += 1;
+ break;
+ default:
+ gdb_assert (0); /* This cannot happen. */
+ }
+ }
+ return stack_adjustment;
+}
+
/* Initialize a prologue cache. */
static struct trad_frame_cache *
@@ -234,73 +326,82 @@ s12z_frame_cache (struct frame_info *this_frame, void **prologue_cache)
CORE_ADDR addr = start_addr; /* Where we have got to? */
int frame_size = 0;
int saved_frame_size = 0;
- while (this_pc > addr)
- {
- struct disassemble_info di = s12z_disassemble_info (gdbarch);
-
- /* No instruction can be more than 11 bytes long, I think. */
- gdb_byte buf[11];
- int nb = print_insn_s12z (addr, &di);
- gdb_assert (nb <= 11);
+ struct disassemble_info di = s12z_disassemble_info (gdbarch);
- if (0 != target_read_code (addr, buf, nb))
- memory_error (TARGET_XFER_E_IO, addr);
- if (buf[0] == 0x05) /* RTS */
- {
- frame_size = saved_frame_size;
- }
- /* Conditional Branches. If any of these are encountered, then
- it is likely that a RTS will terminate it. So we need to save
- the frame size so it can be restored. */
- else if ( (buf[0] == 0x02) /* BRSET */
- || (buf[0] == 0x0B) /* DBcc / TBcc */
- || (buf[0] == 0x03)) /* BRCLR */
- {
- saved_frame_size = frame_size;
- }
- else if (buf[0] == 0x04) /* PUL/ PSH .. */
- {
- bool pull = buf[1] & 0x80;
- int stack_adjustment = 0;
- if (buf[1] & 0x40)
- {
- if (buf[1] & 0x01) stack_adjustment += 3; /* Y */
- if (buf[1] & 0x02) stack_adjustment += 3; /* X */
- if (buf[1] & 0x04) stack_adjustment += 4; /* D7 */
- if (buf[1] & 0x08) stack_adjustment += 4; /* D6 */
- if (buf[1] & 0x10) stack_adjustment += 2; /* D5 */
- if (buf[1] & 0x20) stack_adjustment += 2; /* D4 */
- }
- else
- {
- if (buf[1] & 0x01) stack_adjustment += 2; /* D3 */
- if (buf[1] & 0x02) stack_adjustment += 2; /* D2 */
- if (buf[1] & 0x04) stack_adjustment += 1; /* D1 */
- if (buf[1] & 0x08) stack_adjustment += 1; /* D0 */
- if (buf[1] & 0x10) stack_adjustment += 1; /* CCL */
- if (buf[1] & 0x20) stack_adjustment += 1; /* CCH */
- }
+ struct mem_read_abstraction mra;
+ mra.base.read = (int (*)(mem_read_abstraction_base*,
+ int, size_t, bfd_byte*)) abstract_read_memory;
+ mra.base.advance = advance ;
+ mra.base.posn = posn;
+ mra.info = &di;
- if (!pull)
- stack_adjustment = -stack_adjustment;
- frame_size -= stack_adjustment;
- }
- else if (buf[0] == 0x0a) /* LEA S, (xxx, S) */
- {
- if (0x06 == (buf[1] >> 4))
- {
- int simm = (signed char) (buf[1] & 0x0F);
- frame_size -= simm;
- }
- }
- else if (buf[0] == 0x1a) /* LEA S, (S, xxxx) */
- {
- int simm = (signed char) buf[1];
- frame_size -= simm;
- }
- addr += nb;
+ while (this_pc > addr)
+ {
+ enum optr optr = OP_INVALID;
+ short osize;
+ int n_operands = 0;
+ struct operand *operands[6];
+ mra.memaddr = addr;
+ int n_bytes =
+ decode_s12z (&optr, &osize, &n_operands, operands,
+ (mem_read_abstraction_base *) &mra);
+
+ switch (optr)
+ {
+ case OP_tbNE:
+ case OP_tbPL:
+ case OP_tbMI:
+ case OP_tbGT:
+ case OP_tbLE:
+ case OP_dbNE:
+ case OP_dbEQ:
+ case OP_dbPL:
+ case OP_dbMI:
+ case OP_dbGT:
+ case OP_dbLE:
+ /* Conditional Branches. If any of these are encountered, then
+ it is likely that a RTS will terminate it. So we need to save
+ the frame size so it can be restored. */
+ saved_frame_size = frame_size;
+ break;
+ case OP_rts:
+ /* Restore the frame size from a previously saved value. */
+ frame_size = saved_frame_size;
+ break;
+ case OP_push:
+ frame_size += push_pull_get_stack_adjustment (n_operands, operands);
+ break;
+ case OP_pull:
+ frame_size -= push_pull_get_stack_adjustment (n_operands, operands);
+ break;
+ case OP_lea:
+ if (operands[0]->cl == OPND_CL_REGISTER)
+ {
+ int reg = ((struct register_operand *) (operands[0]))->reg;
+ if ((reg == REG_S) && (operands[1]->cl == OPND_CL_MEMORY))
+ {
+ const struct memory_operand *mo
+ = (const struct memory_operand * ) operands[1];
+ if (mo->n_regs == 1 && !mo->indirect
+ && mo->regs[0] == REG_S
+ && mo->mutation == OPND_RM_NONE)
+ {
+ /* LEA S, (xxx, S) -- Decrement the stack. This is
+ almost certainly the start of a frame. */
+ int simm = (signed char) mo->base_offset;
+ frame_size -= simm;
+ }
+ }
+ }
+ break;
+ default:
+ break;
+ }
+ addr += n_bytes;
+ for (int o = 0; o < n_operands; ++o)
+ free (operands[o]);
}
/* If the PC has not actually got to this point, then the frame
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PING] Re: [PATCH] GDB (s12z): Improve reliability of the stack unwinder.
2019-04-25 13:45 [PATCH] GDB (s12z): Improve reliability of the stack unwinder John Darrington
@ 2019-05-09 4:59 ` John Darrington
2019-05-09 7:52 ` Andrew Burgess
0 siblings, 1 reply; 5+ messages in thread
From: John Darrington @ 2019-05-09 4:59 UTC (permalink / raw)
To: John Darrington; +Cc: gdb-patches
On Thu, Apr 25, 2019 at 03:45:15PM +0200, John Darrington suggested:
Previously, the stack unwinder searched through consecutive bytes for values
which it thought might be the start of a stack mutating operation.
This was error prone, because such bytes could also be the operands of other
instructions. This change uses the opcodes api to interpret the code in each
frame.
gdb/ChangeLog:
* s12z-tdep.c (push_pull_get_stack_adjustment): New function.
(s12z_frame_cache): Use opcodes API to interpret stack frame code.
---
gdb/s12z-tdep.c | 227 ++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 164 insertions(+), 63 deletions(-)
diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
index cef92d8774..b80509f33e 100644
--- a/gdb/s12z-tdep.c
+++ b/gdb/s12z-tdep.c
@@ -30,6 +30,7 @@
#include "opcode/s12z.h"
#include "trad-frame.h"
#include "remote.h"
+#include "opcodes/s12z-opc.h"
/* Two of the registers included in S12Z_N_REGISTERS are
the CCH and CCL "registers" which are just views into
@@ -162,6 +163,97 @@ s12z_disassemble_info (struct gdbarch *gdbarch)
return di;
}
+
+struct mem_read_abstraction
+{
+ struct mem_read_abstraction_base base;
+ bfd_vma memaddr;
+ struct disassemble_info* info;
+};
+
+static void
+advance (struct mem_read_abstraction_base *b)
+{
+ struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
+ mra->memaddr++;
+}
+
+static bfd_vma
+posn (struct mem_read_abstraction_base *b)
+{
+ struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
+ return mra->memaddr;
+}
+
+static int
+abstract_read_memory (struct mem_read_abstraction_base *b,
+ int offset,
+ size_t n, bfd_byte *bytes)
+{
+ struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
+
+ int status =
+ (*mra->info->read_memory_func) (mra->memaddr + offset,
+ bytes, n, mra->info);
+
+ if (status != 0)
+ {
+ (*mra->info->memory_error_func) (status, mra->memaddr, mra->info);
+ return -1;
+ }
+
+ return 0;
+}
+
+
+/* Return the stack adjustment caused by
+ a push or pull instruction. */
+static int
+push_pull_get_stack_adjustment (int n_operands,
+ struct operand *const *operands)
+{
+ int stack_adjustment = 0;
+ gdb_assert (n_operands > 0);
+ if (operands[0]->cl == OPND_CL_REGISTER_ALL)
+ stack_adjustment = 26; /* All the regs */
+ else if (operands[0]->cl == OPND_CL_REGISTER_ALL16)
+ stack_adjustment = 4 * 2; /* All 4 16 bit regs */
+ else
+ for (int i = 0; i < n_operands; ++i)
+ {
+ if (operands[i]->cl != OPND_CL_REGISTER)
+ continue; /* I don't think this can ever happen. */
+ const struct register_operand *op
+ = (const struct register_operand *) operands[i];
+ switch (op->reg)
+ {
+ case REG_X:
+ case REG_Y:
+ stack_adjustment += 3;
+ break;
+ case REG_D7:
+ case REG_D6:
+ stack_adjustment += 4;
+ break;
+ case REG_D2:
+ case REG_D3:
+ case REG_D4:
+ case REG_D5:
+ stack_adjustment += 2;
+ break;
+ case REG_D0:
+ case REG_D1:
+ case REG_CCL:
+ case REG_CCH:
+ stack_adjustment += 1;
+ break;
+ default:
+ gdb_assert (0); /* This cannot happen. */
+ }
+ }
+ return stack_adjustment;
+}
+
/* Initialize a prologue cache. */
static struct trad_frame_cache *
@@ -234,73 +326,82 @@ s12z_frame_cache (struct frame_info *this_frame, void **prologue_cache)
CORE_ADDR addr = start_addr; /* Where we have got to? */
int frame_size = 0;
int saved_frame_size = 0;
- while (this_pc > addr)
- {
- struct disassemble_info di = s12z_disassemble_info (gdbarch);
-
- /* No instruction can be more than 11 bytes long, I think. */
- gdb_byte buf[11];
- int nb = print_insn_s12z (addr, &di);
- gdb_assert (nb <= 11);
+ struct disassemble_info di = s12z_disassemble_info (gdbarch);
- if (0 != target_read_code (addr, buf, nb))
- memory_error (TARGET_XFER_E_IO, addr);
- if (buf[0] == 0x05) /* RTS */
- {
- frame_size = saved_frame_size;
- }
- /* Conditional Branches. If any of these are encountered, then
- it is likely that a RTS will terminate it. So we need to save
- the frame size so it can be restored. */
- else if ( (buf[0] == 0x02) /* BRSET */
- || (buf[0] == 0x0B) /* DBcc / TBcc */
- || (buf[0] == 0x03)) /* BRCLR */
- {
- saved_frame_size = frame_size;
- }
- else if (buf[0] == 0x04) /* PUL/ PSH .. */
- {
- bool pull = buf[1] & 0x80;
- int stack_adjustment = 0;
- if (buf[1] & 0x40)
- {
- if (buf[1] & 0x01) stack_adjustment += 3; /* Y */
- if (buf[1] & 0x02) stack_adjustment += 3; /* X */
- if (buf[1] & 0x04) stack_adjustment += 4; /* D7 */
- if (buf[1] & 0x08) stack_adjustment += 4; /* D6 */
- if (buf[1] & 0x10) stack_adjustment += 2; /* D5 */
- if (buf[1] & 0x20) stack_adjustment += 2; /* D4 */
- }
- else
- {
- if (buf[1] & 0x01) stack_adjustment += 2; /* D3 */
- if (buf[1] & 0x02) stack_adjustment += 2; /* D2 */
- if (buf[1] & 0x04) stack_adjustment += 1; /* D1 */
- if (buf[1] & 0x08) stack_adjustment += 1; /* D0 */
- if (buf[1] & 0x10) stack_adjustment += 1; /* CCL */
- if (buf[1] & 0x20) stack_adjustment += 1; /* CCH */
- }
+ struct mem_read_abstraction mra;
+ mra.base.read = (int (*)(mem_read_abstraction_base*,
+ int, size_t, bfd_byte*)) abstract_read_memory;
+ mra.base.advance = advance ;
+ mra.base.posn = posn;
+ mra.info = &di;
- if (!pull)
- stack_adjustment = -stack_adjustment;
- frame_size -= stack_adjustment;
- }
- else if (buf[0] == 0x0a) /* LEA S, (xxx, S) */
- {
- if (0x06 == (buf[1] >> 4))
- {
- int simm = (signed char) (buf[1] & 0x0F);
- frame_size -= simm;
- }
- }
- else if (buf[0] == 0x1a) /* LEA S, (S, xxxx) */
- {
- int simm = (signed char) buf[1];
- frame_size -= simm;
- }
- addr += nb;
+ while (this_pc > addr)
+ {
+ enum optr optr = OP_INVALID;
+ short osize;
+ int n_operands = 0;
+ struct operand *operands[6];
+ mra.memaddr = addr;
+ int n_bytes =
+ decode_s12z (&optr, &osize, &n_operands, operands,
+ (mem_read_abstraction_base *) &mra);
+
+ switch (optr)
+ {
+ case OP_tbNE:
+ case OP_tbPL:
+ case OP_tbMI:
+ case OP_tbGT:
+ case OP_tbLE:
+ case OP_dbNE:
+ case OP_dbEQ:
+ case OP_dbPL:
+ case OP_dbMI:
+ case OP_dbGT:
+ case OP_dbLE:
+ /* Conditional Branches. If any of these are encountered, then
+ it is likely that a RTS will terminate it. So we need to save
+ the frame size so it can be restored. */
+ saved_frame_size = frame_size;
+ break;
+ case OP_rts:
+ /* Restore the frame size from a previously saved value. */
+ frame_size = saved_frame_size;
+ break;
+ case OP_push:
+ frame_size += push_pull_get_stack_adjustment (n_operands, operands);
+ break;
+ case OP_pull:
+ frame_size -= push_pull_get_stack_adjustment (n_operands, operands);
+ break;
+ case OP_lea:
+ if (operands[0]->cl == OPND_CL_REGISTER)
+ {
+ int reg = ((struct register_operand *) (operands[0]))->reg;
+ if ((reg == REG_S) && (operands[1]->cl == OPND_CL_MEMORY))
+ {
+ const struct memory_operand *mo
+ = (const struct memory_operand * ) operands[1];
+ if (mo->n_regs == 1 && !mo->indirect
+ && mo->regs[0] == REG_S
+ && mo->mutation == OPND_RM_NONE)
+ {
+ /* LEA S, (xxx, S) -- Decrement the stack. This is
+ almost certainly the start of a frame. */
+ int simm = (signed char) mo->base_offset;
+ frame_size -= simm;
+ }
+ }
+ }
+ break;
+ default:
+ break;
+ }
+ addr += n_bytes;
+ for (int o = 0; o < n_operands; ++o)
+ free (operands[o]);
}
/* If the PC has not actually got to this point, then the frame
--
2.11.0
--
Avoid eavesdropping. Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3
fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PING] Re: [PATCH] GDB (s12z): Improve reliability of the stack unwinder.
2019-05-09 4:59 ` [PING] " John Darrington
@ 2019-05-09 7:52 ` Andrew Burgess
2019-05-09 10:22 ` John Darrington
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Burgess @ 2019-05-09 7:52 UTC (permalink / raw)
To: John Darrington; +Cc: gdb-patches
* John Darrington <john@darrington.wattle.id.au> [2019-05-09 06:59:19 +0200]:
> On Thu, Apr 25, 2019 at 03:45:15PM +0200, John Darrington suggested:
> Previously, the stack unwinder searched through consecutive bytes for values
> which it thought might be the start of a stack mutating operation.
> This was error prone, because such bytes could also be the operands of other
> instructions. This change uses the opcodes api to interpret the code in each
> frame.
This sounds like a good change, and the core of the patch looks
reasonable, there's a few style issues that need to be addressed
though.
>
> gdb/ChangeLog:
> * s12z-tdep.c (push_pull_get_stack_adjustment): New function.
> (s12z_frame_cache): Use opcodes API to interpret stack frame
> code.
It looks like most of the changes in this patch are missing from the
ChangeLog. You need to document each new struct and function.
> ---
> gdb/s12z-tdep.c | 227 ++++++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 164 insertions(+), 63 deletions(-)
>
> diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
> index cef92d8774..b80509f33e 100644
> --- a/gdb/s12z-tdep.c
> +++ b/gdb/s12z-tdep.c
> @@ -30,6 +30,7 @@
> #include "opcode/s12z.h"
> #include "trad-frame.h"
> #include "remote.h"
> +#include "opcodes/s12z-opc.h"
>
> /* Two of the registers included in S12Z_N_REGISTERS are
> the CCH and CCL "registers" which are just views into
> @@ -162,6 +163,97 @@ s12z_disassemble_info (struct gdbarch *gdbarch)
> return di;
> }
>
> +
> +struct mem_read_abstraction
> +{
> + struct mem_read_abstraction_base base;
> + bfd_vma memaddr;
> + struct disassemble_info* info;
> +};
The structure needs a descriptive comment at the top, and ideally each
field documented with a comment.
> +
> +static void
> +advance (struct mem_read_abstraction_base *b)
> +{
> + struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
> + mra->memaddr++;
> +}
All functions should have a header comment.
> +
> +static bfd_vma
> +posn (struct mem_read_abstraction_base *b)
> +{
> + struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
> + return mra->memaddr;
> +}
> +
> +static int
> +abstract_read_memory (struct mem_read_abstraction_base *b,
> + int offset,
> + size_t n, bfd_byte *bytes)
> +{
> + struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
> +
> + int status =
> + (*mra->info->read_memory_func) (mra->memaddr + offset,
> + bytes, n, mra->info);
> +
> + if (status != 0)
> + {
> + (*mra->info->memory_error_func) (status, mra->memaddr, mra->info);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> +/* Return the stack adjustment caused by
> + a push or pull instruction. */
> +static int
> +push_pull_get_stack_adjustment (int n_operands,
> + struct operand *const *operands)
> +{
> + int stack_adjustment = 0;
> + gdb_assert (n_operands > 0);
> + if (operands[0]->cl == OPND_CL_REGISTER_ALL)
> + stack_adjustment = 26; /* All the regs */
Missing a '.' at the end of this comment, and in a few other comments too.
> + else if (operands[0]->cl == OPND_CL_REGISTER_ALL16)
> + stack_adjustment = 4 * 2; /* All 4 16 bit regs */
> + else
> + for (int i = 0; i < n_operands; ++i)
> + {
> + if (operands[i]->cl != OPND_CL_REGISTER)
> + continue; /* I don't think this can ever happen. */
> + const struct register_operand *op
> + = (const struct register_operand *) operands[i];
> + switch (op->reg)
> + {
> + case REG_X:
> + case REG_Y:
> + stack_adjustment += 3;
> + break;
> + case REG_D7:
> + case REG_D6:
> + stack_adjustment += 4;
> + break;
> + case REG_D2:
> + case REG_D3:
> + case REG_D4:
> + case REG_D5:
> + stack_adjustment += 2;
> + break;
> + case REG_D0:
> + case REG_D1:
> + case REG_CCL:
> + case REG_CCH:
> + stack_adjustment += 1;
> + break;
> + default:
> + gdb_assert (0); /* This cannot happen. */
You can use gdb_assert_not_reached here.
> + }
> + }
> + return stack_adjustment;
> +}
> +
> /* Initialize a prologue cache. */
>
> static struct trad_frame_cache *
> @@ -234,73 +326,82 @@ s12z_frame_cache (struct frame_info *this_frame, void **prologue_cache)
> CORE_ADDR addr = start_addr; /* Where we have got to? */
> int frame_size = 0;
> int saved_frame_size = 0;
> - while (this_pc > addr)
> - {
> - struct disassemble_info di = s12z_disassemble_info (gdbarch);
> -
> - /* No instruction can be more than 11 bytes long, I think. */
> - gdb_byte buf[11];
>
> - int nb = print_insn_s12z (addr, &di);
> - gdb_assert (nb <= 11);
> + struct disassemble_info di = s12z_disassemble_info (gdbarch);
>
> - if (0 != target_read_code (addr, buf, nb))
> - memory_error (TARGET_XFER_E_IO, addr);
>
> - if (buf[0] == 0x05) /* RTS */
> - {
> - frame_size = saved_frame_size;
> - }
> - /* Conditional Branches. If any of these are encountered, then
> - it is likely that a RTS will terminate it. So we need to save
> - the frame size so it can be restored. */
> - else if ( (buf[0] == 0x02) /* BRSET */
> - || (buf[0] == 0x0B) /* DBcc / TBcc */
> - || (buf[0] == 0x03)) /* BRCLR */
> - {
> - saved_frame_size = frame_size;
> - }
> - else if (buf[0] == 0x04) /* PUL/ PSH .. */
> - {
> - bool pull = buf[1] & 0x80;
> - int stack_adjustment = 0;
> - if (buf[1] & 0x40)
> - {
> - if (buf[1] & 0x01) stack_adjustment += 3; /* Y */
> - if (buf[1] & 0x02) stack_adjustment += 3; /* X */
> - if (buf[1] & 0x04) stack_adjustment += 4; /* D7 */
> - if (buf[1] & 0x08) stack_adjustment += 4; /* D6 */
> - if (buf[1] & 0x10) stack_adjustment += 2; /* D5 */
> - if (buf[1] & 0x20) stack_adjustment += 2; /* D4 */
> - }
> - else
> - {
> - if (buf[1] & 0x01) stack_adjustment += 2; /* D3 */
> - if (buf[1] & 0x02) stack_adjustment += 2; /* D2 */
> - if (buf[1] & 0x04) stack_adjustment += 1; /* D1 */
> - if (buf[1] & 0x08) stack_adjustment += 1; /* D0 */
> - if (buf[1] & 0x10) stack_adjustment += 1; /* CCL */
> - if (buf[1] & 0x20) stack_adjustment += 1; /* CCH */
> - }
> + struct mem_read_abstraction mra;
> + mra.base.read = (int (*)(mem_read_abstraction_base*,
> + int, size_t, bfd_byte*)) abstract_read_memory;
> + mra.base.advance = advance ;
> + mra.base.posn = posn;
> + mra.info = &di;
>
> - if (!pull)
> - stack_adjustment = -stack_adjustment;
> - frame_size -= stack_adjustment;
> - }
> - else if (buf[0] == 0x0a) /* LEA S, (xxx, S) */
> - {
> - if (0x06 == (buf[1] >> 4))
> - {
> - int simm = (signed char) (buf[1] & 0x0F);
> - frame_size -= simm;
> - }
> - }
> - else if (buf[0] == 0x1a) /* LEA S, (S, xxxx) */
> - {
> - int simm = (signed char) buf[1];
> - frame_size -= simm;
> - }
> - addr += nb;
> + while (this_pc > addr)
> + {
> + enum optr optr = OP_INVALID;
> + short osize;
> + int n_operands = 0;
> + struct operand *operands[6];
> + mra.memaddr = addr;
> + int n_bytes =
> + decode_s12z (&optr, &osize, &n_operands, operands,
> + (mem_read_abstraction_base *) &mra);
> +
> + switch (optr)
> + {
> + case OP_tbNE:
> + case OP_tbPL:
> + case OP_tbMI:
> + case OP_tbGT:
> + case OP_tbLE:
> + case OP_dbNE:
> + case OP_dbEQ:
> + case OP_dbPL:
> + case OP_dbMI:
> + case OP_dbGT:
> + case OP_dbLE:
> + /* Conditional Branches. If any of these are encountered, then
> + it is likely that a RTS will terminate it. So we need to save
> + the frame size so it can be restored. */
> + saved_frame_size = frame_size;
> + break;
> + case OP_rts:
> + /* Restore the frame size from a previously saved value. */
> + frame_size = saved_frame_size;
> + break;
> + case OP_push:
> + frame_size += push_pull_get_stack_adjustment (n_operands, operands);
> + break;
> + case OP_pull:
> + frame_size -= push_pull_get_stack_adjustment (n_operands, operands);
> + break;
> + case OP_lea:
> + if (operands[0]->cl == OPND_CL_REGISTER)
> + {
> + int reg = ((struct register_operand *) (operands[0]))->reg;
> + if ((reg == REG_S) && (operands[1]->cl == OPND_CL_MEMORY))
> + {
> + const struct memory_operand *mo
> + = (const struct memory_operand * ) operands[1];
> + if (mo->n_regs == 1 && !mo->indirect
> + && mo->regs[0] == REG_S
> + && mo->mutation == OPND_RM_NONE)
> + {
> + /* LEA S, (xxx, S) -- Decrement the stack. This is
> + almost certainly the start of a frame. */
> + int simm = (signed char) mo->base_offset;
> + frame_size -= simm;
> + }
> + }
> + }
> + break;
> + default:
> + break;
> + }
> + addr += n_bytes;
> + for (int o = 0; o < n_operands; ++o)
> + free (operands[o]);
> }
>
> /* If the PC has not actually got to this point, then the frame
> --
> 2.11.0
Thanks,
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] GDB (s12z): Improve reliability of the stack unwinder.
2019-05-09 7:52 ` Andrew Burgess
@ 2019-05-09 10:22 ` John Darrington
2019-05-14 17:13 ` Andrew Burgess
0 siblings, 1 reply; 5+ messages in thread
From: John Darrington @ 2019-05-09 10:22 UTC (permalink / raw)
To: gdb-patches; +Cc: John Darrington
Previously, the stack unwinder searched through consecutive bytes for values
which it thought might be the start of a stack mutating operation.
This was error prone, because such bytes could also be the operands of other
instructions. This change uses the opcodes api to interpret the code in each
frame.
gdb/ChangeLog:
* s12z-tdep.c (push_pull_get_stack_adjustment): New function.
(advance, posn, abstract_read_memory): New functions.
[struct mem_read_abstraction]: New struct.
(s12z_frame_cache): Use opcodes API to interpret stack frame code.
---
gdb/s12z-tdep.c | 234 +++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 171 insertions(+), 63 deletions(-)
diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
index cef92d8774..b549862e84 100644
--- a/gdb/s12z-tdep.c
+++ b/gdb/s12z-tdep.c
@@ -30,6 +30,7 @@
#include "opcode/s12z.h"
#include "trad-frame.h"
#include "remote.h"
+#include "opcodes/s12z-opc.h"
/* Two of the registers included in S12Z_N_REGISTERS are
the CCH and CCL "registers" which are just views into
@@ -162,6 +163,104 @@ s12z_disassemble_info (struct gdbarch *gdbarch)
return di;
}
+
+/* A struct (based on mem_read_abstraction_base) to read memory
+ through the disassemble_info API. */
+struct mem_read_abstraction
+{
+ struct mem_read_abstraction_base base; /* The parent struct. */
+ bfd_vma memaddr; /* Where to read from. */
+ struct disassemble_info* info; /* The disassember to use for reading. */
+};
+
+/* Advance the reader by one byte. */
+static void
+advance (struct mem_read_abstraction_base *b)
+{
+ struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
+ mra->memaddr++;
+}
+
+/* Return the current position of the reader. */
+static bfd_vma
+posn (struct mem_read_abstraction_base *b)
+{
+ struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
+ return mra->memaddr;
+}
+
+/* Read the N bytes at OFFSET using B. The bytes read are stored in BYTES.
+ It is the caller's responsibility to ensure that this is of at least N
+ in size. */
+static int
+abstract_read_memory (struct mem_read_abstraction_base *b,
+ int offset,
+ size_t n, bfd_byte *bytes)
+{
+ struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
+
+ int status =
+ (*mra->info->read_memory_func) (mra->memaddr + offset,
+ bytes, n, mra->info);
+
+ if (status != 0)
+ {
+ (*mra->info->memory_error_func) (status, mra->memaddr, mra->info);
+ return -1;
+ }
+
+ return 0;
+}
+
+
+/* Return the stack adjustment caused by a push or pull instruction. */
+static int
+push_pull_get_stack_adjustment (int n_operands,
+ struct operand *const *operands)
+{
+ int stack_adjustment = 0;
+ gdb_assert (n_operands > 0);
+ if (operands[0]->cl == OPND_CL_REGISTER_ALL)
+ stack_adjustment = 26; /* All the regs are involved. */
+ else if (operands[0]->cl == OPND_CL_REGISTER_ALL16)
+ stack_adjustment = 4 * 2; /* All four 16 bit regs are involved. */
+ else
+ for (int i = 0; i < n_operands; ++i)
+ {
+ if (operands[i]->cl != OPND_CL_REGISTER)
+ continue; /* I don't think this can ever happen. */
+ const struct register_operand *op
+ = (const struct register_operand *) operands[i];
+ switch (op->reg)
+ {
+ case REG_X:
+ case REG_Y:
+ stack_adjustment += 3;
+ break;
+ case REG_D7:
+ case REG_D6:
+ stack_adjustment += 4;
+ break;
+ case REG_D2:
+ case REG_D3:
+ case REG_D4:
+ case REG_D5:
+ stack_adjustment += 2;
+ break;
+ case REG_D0:
+ case REG_D1:
+ case REG_CCL:
+ case REG_CCH:
+ stack_adjustment += 1;
+ break;
+ default:
+ gdb_assert_not_reached ("Invalid register in push/pull operation.");
+ break;
+ }
+ }
+ return stack_adjustment;
+}
+
/* Initialize a prologue cache. */
static struct trad_frame_cache *
@@ -234,73 +333,82 @@ s12z_frame_cache (struct frame_info *this_frame, void **prologue_cache)
CORE_ADDR addr = start_addr; /* Where we have got to? */
int frame_size = 0;
int saved_frame_size = 0;
- while (this_pc > addr)
- {
- struct disassemble_info di = s12z_disassemble_info (gdbarch);
-
- /* No instruction can be more than 11 bytes long, I think. */
- gdb_byte buf[11];
- int nb = print_insn_s12z (addr, &di);
- gdb_assert (nb <= 11);
+ struct disassemble_info di = s12z_disassemble_info (gdbarch);
- if (0 != target_read_code (addr, buf, nb))
- memory_error (TARGET_XFER_E_IO, addr);
- if (buf[0] == 0x05) /* RTS */
- {
- frame_size = saved_frame_size;
- }
- /* Conditional Branches. If any of these are encountered, then
- it is likely that a RTS will terminate it. So we need to save
- the frame size so it can be restored. */
- else if ( (buf[0] == 0x02) /* BRSET */
- || (buf[0] == 0x0B) /* DBcc / TBcc */
- || (buf[0] == 0x03)) /* BRCLR */
- {
- saved_frame_size = frame_size;
- }
- else if (buf[0] == 0x04) /* PUL/ PSH .. */
- {
- bool pull = buf[1] & 0x80;
- int stack_adjustment = 0;
- if (buf[1] & 0x40)
- {
- if (buf[1] & 0x01) stack_adjustment += 3; /* Y */
- if (buf[1] & 0x02) stack_adjustment += 3; /* X */
- if (buf[1] & 0x04) stack_adjustment += 4; /* D7 */
- if (buf[1] & 0x08) stack_adjustment += 4; /* D6 */
- if (buf[1] & 0x10) stack_adjustment += 2; /* D5 */
- if (buf[1] & 0x20) stack_adjustment += 2; /* D4 */
- }
- else
- {
- if (buf[1] & 0x01) stack_adjustment += 2; /* D3 */
- if (buf[1] & 0x02) stack_adjustment += 2; /* D2 */
- if (buf[1] & 0x04) stack_adjustment += 1; /* D1 */
- if (buf[1] & 0x08) stack_adjustment += 1; /* D0 */
- if (buf[1] & 0x10) stack_adjustment += 1; /* CCL */
- if (buf[1] & 0x20) stack_adjustment += 1; /* CCH */
- }
+ struct mem_read_abstraction mra;
+ mra.base.read = (int (*)(mem_read_abstraction_base*,
+ int, size_t, bfd_byte*)) abstract_read_memory;
+ mra.base.advance = advance ;
+ mra.base.posn = posn;
+ mra.info = &di;
- if (!pull)
- stack_adjustment = -stack_adjustment;
- frame_size -= stack_adjustment;
- }
- else if (buf[0] == 0x0a) /* LEA S, (xxx, S) */
- {
- if (0x06 == (buf[1] >> 4))
- {
- int simm = (signed char) (buf[1] & 0x0F);
- frame_size -= simm;
- }
- }
- else if (buf[0] == 0x1a) /* LEA S, (S, xxxx) */
- {
- int simm = (signed char) buf[1];
- frame_size -= simm;
- }
- addr += nb;
+ while (this_pc > addr)
+ {
+ enum optr optr = OP_INVALID;
+ short osize;
+ int n_operands = 0;
+ struct operand *operands[6];
+ mra.memaddr = addr;
+ int n_bytes =
+ decode_s12z (&optr, &osize, &n_operands, operands,
+ (mem_read_abstraction_base *) &mra);
+
+ switch (optr)
+ {
+ case OP_tbNE:
+ case OP_tbPL:
+ case OP_tbMI:
+ case OP_tbGT:
+ case OP_tbLE:
+ case OP_dbNE:
+ case OP_dbEQ:
+ case OP_dbPL:
+ case OP_dbMI:
+ case OP_dbGT:
+ case OP_dbLE:
+ /* Conditional Branches. If any of these are encountered, then
+ it is likely that a RTS will terminate it. So we need to save
+ the frame size so it can be restored. */
+ saved_frame_size = frame_size;
+ break;
+ case OP_rts:
+ /* Restore the frame size from a previously saved value. */
+ frame_size = saved_frame_size;
+ break;
+ case OP_push:
+ frame_size += push_pull_get_stack_adjustment (n_operands, operands);
+ break;
+ case OP_pull:
+ frame_size -= push_pull_get_stack_adjustment (n_operands, operands);
+ break;
+ case OP_lea:
+ if (operands[0]->cl == OPND_CL_REGISTER)
+ {
+ int reg = ((struct register_operand *) (operands[0]))->reg;
+ if ((reg == REG_S) && (operands[1]->cl == OPND_CL_MEMORY))
+ {
+ const struct memory_operand *mo
+ = (const struct memory_operand * ) operands[1];
+ if (mo->n_regs == 1 && !mo->indirect
+ && mo->regs[0] == REG_S
+ && mo->mutation == OPND_RM_NONE)
+ {
+ /* LEA S, (xxx, S) -- Decrement the stack. This is
+ almost certainly the start of a frame. */
+ int simm = (signed char) mo->base_offset;
+ frame_size -= simm;
+ }
+ }
+ }
+ break;
+ default:
+ break;
+ }
+ addr += n_bytes;
+ for (int o = 0; o < n_operands; ++o)
+ free (operands[o]);
}
/* If the PC has not actually got to this point, then the frame
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] GDB (s12z): Improve reliability of the stack unwinder.
2019-05-09 10:22 ` John Darrington
@ 2019-05-14 17:13 ` Andrew Burgess
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Burgess @ 2019-05-14 17:13 UTC (permalink / raw)
To: John Darrington; +Cc: gdb-patches
* John Darrington <john@darrington.wattle.id.au> [2019-05-09 12:22:19 +0200]:
> Previously, the stack unwinder searched through consecutive bytes for values
> which it thought might be the start of a stack mutating operation.
> This was error prone, because such bytes could also be the operands of other
> instructions. This change uses the opcodes api to interpret the code in each
> frame.
>
> gdb/ChangeLog:
> * s12z-tdep.c (push_pull_get_stack_adjustment): New function.
> (advance, posn, abstract_read_memory): New functions.
> [struct mem_read_abstraction]: New struct.
> (s12z_frame_cache): Use opcodes API to interpret stack frame
> code.
Looks good to me.
Thanks,
Andrew
> ---
> gdb/s12z-tdep.c | 234 +++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 171 insertions(+), 63 deletions(-)
>
> diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
> index cef92d8774..b549862e84 100644
> --- a/gdb/s12z-tdep.c
> +++ b/gdb/s12z-tdep.c
> @@ -30,6 +30,7 @@
> #include "opcode/s12z.h"
> #include "trad-frame.h"
> #include "remote.h"
> +#include "opcodes/s12z-opc.h"
>
> /* Two of the registers included in S12Z_N_REGISTERS are
> the CCH and CCL "registers" which are just views into
> @@ -162,6 +163,104 @@ s12z_disassemble_info (struct gdbarch *gdbarch)
> return di;
> }
>
> +
> +/* A struct (based on mem_read_abstraction_base) to read memory
> + through the disassemble_info API. */
> +struct mem_read_abstraction
> +{
> + struct mem_read_abstraction_base base; /* The parent struct. */
> + bfd_vma memaddr; /* Where to read from. */
> + struct disassemble_info* info; /* The disassember to use for reading. */
> +};
> +
> +/* Advance the reader by one byte. */
> +static void
> +advance (struct mem_read_abstraction_base *b)
> +{
> + struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
> + mra->memaddr++;
> +}
> +
> +/* Return the current position of the reader. */
> +static bfd_vma
> +posn (struct mem_read_abstraction_base *b)
> +{
> + struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
> + return mra->memaddr;
> +}
> +
> +/* Read the N bytes at OFFSET using B. The bytes read are stored in BYTES.
> + It is the caller's responsibility to ensure that this is of at least N
> + in size. */
> +static int
> +abstract_read_memory (struct mem_read_abstraction_base *b,
> + int offset,
> + size_t n, bfd_byte *bytes)
> +{
> + struct mem_read_abstraction *mra = (struct mem_read_abstraction *) b;
> +
> + int status =
> + (*mra->info->read_memory_func) (mra->memaddr + offset,
> + bytes, n, mra->info);
> +
> + if (status != 0)
> + {
> + (*mra->info->memory_error_func) (status, mra->memaddr, mra->info);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> +/* Return the stack adjustment caused by a push or pull instruction. */
> +static int
> +push_pull_get_stack_adjustment (int n_operands,
> + struct operand *const *operands)
> +{
> + int stack_adjustment = 0;
> + gdb_assert (n_operands > 0);
> + if (operands[0]->cl == OPND_CL_REGISTER_ALL)
> + stack_adjustment = 26; /* All the regs are involved. */
> + else if (operands[0]->cl == OPND_CL_REGISTER_ALL16)
> + stack_adjustment = 4 * 2; /* All four 16 bit regs are involved. */
> + else
> + for (int i = 0; i < n_operands; ++i)
> + {
> + if (operands[i]->cl != OPND_CL_REGISTER)
> + continue; /* I don't think this can ever happen. */
> + const struct register_operand *op
> + = (const struct register_operand *) operands[i];
> + switch (op->reg)
> + {
> + case REG_X:
> + case REG_Y:
> + stack_adjustment += 3;
> + break;
> + case REG_D7:
> + case REG_D6:
> + stack_adjustment += 4;
> + break;
> + case REG_D2:
> + case REG_D3:
> + case REG_D4:
> + case REG_D5:
> + stack_adjustment += 2;
> + break;
> + case REG_D0:
> + case REG_D1:
> + case REG_CCL:
> + case REG_CCH:
> + stack_adjustment += 1;
> + break;
> + default:
> + gdb_assert_not_reached ("Invalid register in push/pull operation.");
> + break;
> + }
> + }
> + return stack_adjustment;
> +}
> +
> /* Initialize a prologue cache. */
>
> static struct trad_frame_cache *
> @@ -234,73 +333,82 @@ s12z_frame_cache (struct frame_info *this_frame, void **prologue_cache)
> CORE_ADDR addr = start_addr; /* Where we have got to? */
> int frame_size = 0;
> int saved_frame_size = 0;
> - while (this_pc > addr)
> - {
> - struct disassemble_info di = s12z_disassemble_info (gdbarch);
> -
> - /* No instruction can be more than 11 bytes long, I think. */
> - gdb_byte buf[11];
>
> - int nb = print_insn_s12z (addr, &di);
> - gdb_assert (nb <= 11);
> + struct disassemble_info di = s12z_disassemble_info (gdbarch);
>
> - if (0 != target_read_code (addr, buf, nb))
> - memory_error (TARGET_XFER_E_IO, addr);
>
> - if (buf[0] == 0x05) /* RTS */
> - {
> - frame_size = saved_frame_size;
> - }
> - /* Conditional Branches. If any of these are encountered, then
> - it is likely that a RTS will terminate it. So we need to save
> - the frame size so it can be restored. */
> - else if ( (buf[0] == 0x02) /* BRSET */
> - || (buf[0] == 0x0B) /* DBcc / TBcc */
> - || (buf[0] == 0x03)) /* BRCLR */
> - {
> - saved_frame_size = frame_size;
> - }
> - else if (buf[0] == 0x04) /* PUL/ PSH .. */
> - {
> - bool pull = buf[1] & 0x80;
> - int stack_adjustment = 0;
> - if (buf[1] & 0x40)
> - {
> - if (buf[1] & 0x01) stack_adjustment += 3; /* Y */
> - if (buf[1] & 0x02) stack_adjustment += 3; /* X */
> - if (buf[1] & 0x04) stack_adjustment += 4; /* D7 */
> - if (buf[1] & 0x08) stack_adjustment += 4; /* D6 */
> - if (buf[1] & 0x10) stack_adjustment += 2; /* D5 */
> - if (buf[1] & 0x20) stack_adjustment += 2; /* D4 */
> - }
> - else
> - {
> - if (buf[1] & 0x01) stack_adjustment += 2; /* D3 */
> - if (buf[1] & 0x02) stack_adjustment += 2; /* D2 */
> - if (buf[1] & 0x04) stack_adjustment += 1; /* D1 */
> - if (buf[1] & 0x08) stack_adjustment += 1; /* D0 */
> - if (buf[1] & 0x10) stack_adjustment += 1; /* CCL */
> - if (buf[1] & 0x20) stack_adjustment += 1; /* CCH */
> - }
> + struct mem_read_abstraction mra;
> + mra.base.read = (int (*)(mem_read_abstraction_base*,
> + int, size_t, bfd_byte*)) abstract_read_memory;
> + mra.base.advance = advance ;
> + mra.base.posn = posn;
> + mra.info = &di;
>
> - if (!pull)
> - stack_adjustment = -stack_adjustment;
> - frame_size -= stack_adjustment;
> - }
> - else if (buf[0] == 0x0a) /* LEA S, (xxx, S) */
> - {
> - if (0x06 == (buf[1] >> 4))
> - {
> - int simm = (signed char) (buf[1] & 0x0F);
> - frame_size -= simm;
> - }
> - }
> - else if (buf[0] == 0x1a) /* LEA S, (S, xxxx) */
> - {
> - int simm = (signed char) buf[1];
> - frame_size -= simm;
> - }
> - addr += nb;
> + while (this_pc > addr)
> + {
> + enum optr optr = OP_INVALID;
> + short osize;
> + int n_operands = 0;
> + struct operand *operands[6];
> + mra.memaddr = addr;
> + int n_bytes =
> + decode_s12z (&optr, &osize, &n_operands, operands,
> + (mem_read_abstraction_base *) &mra);
> +
> + switch (optr)
> + {
> + case OP_tbNE:
> + case OP_tbPL:
> + case OP_tbMI:
> + case OP_tbGT:
> + case OP_tbLE:
> + case OP_dbNE:
> + case OP_dbEQ:
> + case OP_dbPL:
> + case OP_dbMI:
> + case OP_dbGT:
> + case OP_dbLE:
> + /* Conditional Branches. If any of these are encountered, then
> + it is likely that a RTS will terminate it. So we need to save
> + the frame size so it can be restored. */
> + saved_frame_size = frame_size;
> + break;
> + case OP_rts:
> + /* Restore the frame size from a previously saved value. */
> + frame_size = saved_frame_size;
> + break;
> + case OP_push:
> + frame_size += push_pull_get_stack_adjustment (n_operands, operands);
> + break;
> + case OP_pull:
> + frame_size -= push_pull_get_stack_adjustment (n_operands, operands);
> + break;
> + case OP_lea:
> + if (operands[0]->cl == OPND_CL_REGISTER)
> + {
> + int reg = ((struct register_operand *) (operands[0]))->reg;
> + if ((reg == REG_S) && (operands[1]->cl == OPND_CL_MEMORY))
> + {
> + const struct memory_operand *mo
> + = (const struct memory_operand * ) operands[1];
> + if (mo->n_regs == 1 && !mo->indirect
> + && mo->regs[0] == REG_S
> + && mo->mutation == OPND_RM_NONE)
> + {
> + /* LEA S, (xxx, S) -- Decrement the stack. This is
> + almost certainly the start of a frame. */
> + int simm = (signed char) mo->base_offset;
> + frame_size -= simm;
> + }
> + }
> + }
> + break;
> + default:
> + break;
> + }
> + addr += n_bytes;
> + for (int o = 0; o < n_operands; ++o)
> + free (operands[o]);
> }
>
> /* If the PC has not actually got to this point, then the frame
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-14 17:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-25 13:45 [PATCH] GDB (s12z): Improve reliability of the stack unwinder John Darrington
2019-05-09 4:59 ` [PING] " John Darrington
2019-05-09 7:52 ` Andrew Burgess
2019-05-09 10:22 ` John Darrington
2019-05-14 17:13 ` Andrew Burgess
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox