From patchwork Thu Apr 25 13:45:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Darrington X-Patchwork-Id: 32410 Received: (qmail 45193 invoked by alias); 25 Apr 2019 13:45:33 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 45144 invoked by uid 89); 25 Apr 2019 13:45:33 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-13.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy=H*F:U*john, opcodes, s12z, H*Ad:U*john X-HELO: jocasta.intra Received: from de.cellform.com (HELO jocasta.intra) (88.217.224.109) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 25 Apr 2019 13:45:30 +0000 Received: from jocasta.intra (localhost [127.0.0.1]) by jocasta.intra (8.15.2/8.15.2/Debian-8) with ESMTPS id x3PDjRCr008248 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 25 Apr 2019 15:45:27 +0200 Received: (from john@localhost) by jocasta.intra (8.15.2/8.15.2/Submit) id x3PDjRhq008247; Thu, 25 Apr 2019 15:45:27 +0200 From: John Darrington To: gdb-patches@sourceware.org Cc: John Darrington Subject: [PATCH] GDB (s12z): Improve reliability of the stack unwinder. Date: Thu, 25 Apr 2019 15:45:15 +0200 Message-Id: <20190425134515.8202-1-john@darrington.wattle.id.au> 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