From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25263 invoked by alias); 17 May 2011 17:14:42 -0000 Received: (qmail 25248 invoked by uid 22791); 17 May 2011 17:14:38 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,SPF_SOFTFAIL,TW_EG,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate4.uk.ibm.com (HELO mtagate4.uk.ibm.com) (194.196.100.164) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 17 May 2011 17:14:21 +0000 Received: from d06nrmr1307.portsmouth.uk.ibm.com (d06nrmr1307.portsmouth.uk.ibm.com [9.149.38.129]) by mtagate4.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p4HHEDr3015304 for ; Tue, 17 May 2011 17:14:13 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1307.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p4HHEC632478262 for ; Tue, 17 May 2011 18:14:12 +0100 Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p4HHECTu018427 for ; Tue, 17 May 2011 11:14:12 -0600 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with SMTP id p4HHEBTp018414; Tue, 17 May 2011 11:14:11 -0600 Message-Id: <201105171714.p4HHEBTp018414@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Tue, 17 May 2011 19:14:11 +0200 Subject: Re: [try 2nd 5/8] Displaced stepping for Thumb 32-bit insns To: yao@codesourcery.com (Yao Qi) Date: Tue, 17 May 2011 17:14:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: <4DC2A4FD.5040300@codesourcery.com> from "Yao Qi" at May 05, 2011 09:24:13 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-05/txt/msg00381.txt.bz2 Yao Qi wrote: > +static int > +thumb2_copy_preload (struct gdbarch *gdbarch, uint16_t insn1, uint16_t insn2, > + struct regcache *regs, struct displaced_step_closure *dsc) > +{ > + unsigned int rn = bits (insn1, 0, 3); > + if (rn == ARM_PC_REGNUM) > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, "preload", dsc); > + > + if (debug_displaced) > + fprintf_unfiltered (gdb_stdlog, "displaced: copying preload insn %.4x%.4x\n", > + insn1, insn2); > + > + dsc->modinsn[0] = insn1 & 0xfff0; > + dsc->modinsn[1] = insn2; > + dsc->numinsns = 2; > + > + install_preload (gdbarch, regs, dsc, rn); > + > + return 0; > +} > +static int > +thumb2_copy_preload_reg (struct gdbarch *gdbarch, uint16_t insn1, > + uint16_t insn2, struct regcache *regs, > + struct displaced_step_closure *dsc) > +{ > + unsigned int rn = bits (insn1, 0, 3); > + unsigned int rm = bits (insn2, 0, 3); > + > + if (rn != ARM_PC_REGNUM && rm != ARM_PC_REGNUM) > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, "preload reg", > + dsc); > + > + if (debug_displaced) > + fprintf_unfiltered (gdb_stdlog, "displaced: copying preload insn %.4x%.4x\n", > + insn1, insn1); > + > + dsc->modinsn[0] = insn1 & 0xfff0; > + dsc->modinsn[1] = (insn2 & 0xfff0) | 0x1; > + dsc->numinsns = 2; > + > + install_preload_reg (gdbarch, regs, dsc, rn, rm); > + return 0; > +} Handling of preload instructions seems wrong for a couple of reasons: - In Thumb mode, PLD/PLI with register offset must not use PC as offset register, so those can just be copied unmodified. The only instructions to be treated specially are the "literal" variants, which do encode PC-relative offsets. This means a separate thumb2_copy_preload_reg shouldn't be needed. - However, you cannot just transform a PLD/PLI "literal" (i.e. PC + immediate) into an "immediate" (i.e. register + immediate) version, since in Thumb mode the "literal" version supports a 12-bit immediate, while the immediate version only supports an 8-bit immediate. I guess you could either add the immediate to the PC during preparation stage and then use an "immediate" instruction with immediate zero, or else load the immediate into a second register and use a "register" version of the instruction. > +static int > +thumb2_copy_copro_load_store (struct gdbarch *gdbarch, uint16_t insn1, > + uint16_t insn2, struct regcache *regs, > + struct displaced_step_closure *dsc) > +{ > + unsigned int rn = bits (insn1, 0, 3); > + > + if (rn == ARM_PC_REGNUM) > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "copro load/store", dsc); > + > + if (debug_displaced) > + fprintf_unfiltered (gdb_stdlog, "displaced: copying coprocessor " > + "load/store insn %.4x%.4x\n", insn1, insn2); > + > + dsc->modinsn[0] = insn1 & 0xfff0; > + dsc->modinsn[1] = insn2; > + dsc->numinsns = 2; This doesn't look right: you're replacing the RN register if it is anything *but* 15 -- but those cases do not need to be replaced! In fact, unless I'm missing something, in Thumb mode no coprocessor instruction actually uses the PC (either RN == 15 indicates some other operation, or else it is specified as unpredictable). So those should simply all be copied unmodified ... > +static int > +thumb2_copy_b_bl_blx (struct gdbarch *gdbarch, uint16_t insn1, > + uint16_t insn2, struct regcache *regs, > + struct displaced_step_closure *dsc) > +{ > + int link = bit (insn2, 14); > + int exchange = link && !bit (insn2, 12); > + int cond = INST_AL; > + long offset =0; > + int j1 = bit (insn2, 13); > + int j2 = bit (insn2, 11); > + int s = sbits (insn1, 10, 10); > + int i1 = !(j1 ^ bit (insn1, 10)); > + int i2 = !(j2 ^ bit (insn1, 10)); > + > + if (!link && !exchange) /* B */ > + { > + cond = bits (insn1, 6, 9); Only encoding T3 has condition bits, not T4. > + offset = (bits (insn2, 0, 10) << 1); > + if (bit (insn2, 12)) /* Encoding T4 */ > + { > + offset |= (bits (insn1, 0, 9) << 12) > + | (i2 << 22) > + | (i1 << 23) > + | (s << 24); > + } > + else /* Encoding T3 */ > + offset |= (bits (insn1, 0, 5) << 12) > + | (j1 << 18) > + | (j2 << 19) > + | (s << 20); > + } > + else > + { > + offset = (bits (insn1, 0, 9) << 12); > + offset |= ((i2 << 22) | (i1 << 23) | (s << 24)); > + offset |= exchange ? > + (bits (insn2, 1, 10) << 2) : (bits (insn2, 0, 10) << 1); > + } > + > + if (debug_displaced) > + fprintf_unfiltered (gdb_stdlog, "displaced: copying %s immediate insn " > + "%.4x %.4x with offset %.8lx\n", > + (exchange) ? "blx" : "bl", > + insn1, insn2, offset); > + > + dsc->modinsn[0] = THUMB_NOP; > + > + install_b_bl_blx (gdbarch, regs, dsc, cond, exchange, 1, offset); Why do you always pass 1 for link? Shouldn't "link" be passed? > +static int > +thumb2_copy_alu_reg (struct gdbarch *gdbarch, uint16_t insn1, > + uint16_t insn2, struct regcache *regs, > + struct displaced_step_closure *dsc) > +{ > + unsigned int op2 = bits (insn2, 4, 7); > + int is_mov = (op2 == 0x0); > + unsigned int rn, rm, rd; > + > + rn = bits (insn1, 0, 3); /* Rn */ > + rm = bits (insn2, 0, 3); /* Rm */ > + rd = bits (insn2, 8, 11); /* Rd */ > + > + /* In Thumb-2, rn, rm and rd can't be r15. */ This isn't quite true ... otherwise we wouldn't need the routine at all. > + if (rn != ARM_PC_REGNUM && rm != ARM_PC_REGNUM > + && rd != ARM_PC_REGNUM) > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, "ALU reg", dsc); > + > + if (debug_displaced) > + fprintf_unfiltered (gdb_stdlog, "displaced: copying reg %s insn %.4x%.4x\n", > + "ALU", insn1, insn2); > + > + if (is_mov) > + dsc->modinsn[0] = insn1; > + else > + dsc->modinsn[0] = ((insn1 & 0xfff0) | 0x1); > + > + dsc->modinsn[1] = ((insn2 & 0xf0f0) | 0x2); > + dsc->numinsns = 2; This doesn't look right. It looks like this function is called for all instructions in tables A6-22 through A6-26; those encodings differ significantly in how their fields are used. Some of them have the Rn, Rm, Rd fields as above, but others just have some of them. For some, a register field content of 15 does indeed refer to the PC and needs to be replaced; for others a register field content of 15 means instead that a different operation is to be performed (e.g. ADD vs TST, EOR vs TEQ ...) and so it must *not* be replaced; and for yet others, a register field content of 15 is unpredictable. In fact, I think only a very small number of instructions in this category actually may refer to the PC (only MOV?), so there needs to the be more instruction decoding to actually identify those. > static int > +thumb2_copy_ldr_str_ldrb_strb (struct gdbarch *gdbarch, uint16_t insn1, > + uint16_t insn2, struct regcache *regs, > + struct displaced_step_closure *dsc, > + int load, int byte, int usermode, int writeback) Hmmm ... this function is called for *halfwords* as well, not just for bytes and words. This means the "byte" operand is no longer sufficient to uniquely determine the size -- note that when calling down to the install_ routine, xfersize is always set to 1 or 4. > +{ > + int immed = !bit (insn1, 9); > + unsigned int rt = bits (insn2, 12, 15); > + unsigned int rn = bits (insn1, 0, 3); > + unsigned int rm = bits (insn2, 0, 3); /* Only valid if !immed. */ > + > + if (rt != ARM_PC_REGNUM && rn != ARM_PC_REGNUM && rm != ARM_PC_REGNUM) rm shouldn't be checked if immed is true > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, "load/store", > + dsc); > + > + if (debug_displaced) > + fprintf_unfiltered (gdb_stdlog, > + "displaced: copying %s%s r%d [r%d] insn %.4x%.4x\n", > + load ? (byte ? "ldrb" : "ldr") > + : (byte ? "strb" : "str"), usermode ? "t" : "", > + rt, rn, insn1, insn2); > + > + install_ldr_str_ldrb_strb (gdbarch, regs, dsc, load, immed, writeback, byte, > + usermode, rt, rm, rn); > + > + if (load || rt != ARM_PC_REGNUM) > + { > + dsc->u.ldst.restore_r4 = 0; > + > + if (immed) > + /* {ldr,str}[b] rt, [rn, #imm], etc. > + -> > + {ldr,str}[b] r0, [r2, #imm]. */ > + { > + dsc->modinsn[0] = (insn1 & 0xfff0) | 0x2; > + dsc->modinsn[1] = insn2 & 0x0fff; > + } > + else > + /* {ldr,str}[b] rt, [rn, rm], etc. > + -> > + {ldr,str}[b] r0, [r2, r3]. */ > + { > + dsc->modinsn[0] = (insn1 & 0xfff0) | 0x2; > + dsc->modinsn[1] = (insn2 & 0x0ff0) | 0x3; > + } > + > + dsc->numinsns = 2; > + } > + else > + { > + /* In Thumb-32 instructions, the behavior is unpredictable when Rt is > + PC, while the behavior is undefined when Rn is PC. Shortly, neither > + Rt nor Rn can be PC. */ > + > + gdb_assert (0); > + } > + > + return 0; > +} > +/* Decode extension register load/store. Exactly the same as > + arm_decode_ext_reg_ld_st. */ > + > +static int > +thumb2_decode_ext_reg_ld_st (struct gdbarch *gdbarch, uint16_t insn1, > + uint16_t insn2, struct regcache *regs, > + struct displaced_step_closure *dsc) > +{ > + unsigned int opcode = bits (insn1, 4, 8); > + > + switch (opcode) > + { > + case 0x04: case 0x05: > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "vfp/neon vmov", dsc); > + > + case 0x08: case 0x0c: /* 01x00 */ > + case 0x0a: case 0x0e: /* 01x10 */ > + case 0x12: case 0x16: /* 10x10 */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "vfp/neon vstm/vpush", dsc); > + > + case 0x09: case 0x0d: /* 01x01 */ > + case 0x0b: case 0x0f: /* 01x11 */ > + case 0x13: case 0x17: /* 10x11 */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "vfp/neon vldm/vpop", dsc); > + > + case 0x10: case 0x14: case 0x18: case 0x1c: /* vstr. */ > + case 0x11: case 0x15: case 0x19: case 0x1d: /* vldr. */ > + return thumb2_copy_copro_load_store (gdbarch, insn1, insn2, regs, dsc); See the comment at thumb2_copy_copro_load_store: since that function will always copy the instruction unmodified, so can this function. > +static int > +thumb2_decode_svc_copro (struct gdbarch *gdbarch, uint16_t insn1, > + uint16_t insn2, struct regcache *regs, > + struct displaced_step_closure *dsc) > +{ > + unsigned int coproc = bits (insn2, 8, 11); > + unsigned int op1 = bits (insn1, 4, 9); > + unsigned int bit_5_8 = bits (insn1, 5, 8); > + unsigned int bit_9 = bit (insn1, 9); > + unsigned int bit_4 = bit (insn1, 4); > + unsigned int rn = bits (insn1, 0, 3); > + > + if (bit_9 == 0) > + { > + if (bit_5_8 == 2) > + { > + if ((coproc & 0xe) == 0xa) /* 64-bit xfer. */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "neon 64bit xfer", dsc); > + else > + { > + if (bit_4) /* MRRC/MRRC2 */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "mrrc/mrrc2", dsc); > + else /* MCRR/MCRR2 */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "mcrr/mcrr2", dsc); > + } > + } > + else if (bit_5_8 == 0) /* UNDEFINED. */ > + return thumb_32bit_copy_undef (gdbarch, insn1, insn2, dsc); > + else > + { > + /*coproc is 101x. SIMD/VFP, ext registers load/store. */ > + if ((coproc & 0xe) == 0xa) > + return thumb2_decode_ext_reg_ld_st (gdbarch, insn1, insn2, regs, > + dsc); > + else /* coproc is not 101x. */ > + { > + if (bit_4 == 0) /* STC/STC2. */ > + return thumb2_copy_copro_load_store (gdbarch, insn1, insn2, > + regs, dsc); > + else > + { > + if (rn == 0xf) /* LDC/LDC2 literal. */ > + return thumb2_copy_copro_load_store (gdbarch, insn1, insn2, > + regs, dsc); > + else /* LDC/LDC2 immeidate. */ > + return thumb2_copy_copro_load_store (gdbarch, insn1, insn2, > + regs, dsc); > + } > + } See above ... I don't think any of those instructions can ever use the PC in Thumb mode, so this can be simplified. > + } > + } > + else > + { > + unsigned int op = bit (insn2, 4); > + unsigned int bit_8 = bit (insn1, 8); > + > + if (bit_8) /* Advanced SIMD */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "neon", dsc); > + else > + { > + /*coproc is 101x. */ > + if ((coproc & 0xe) == 0xa) > + { > + if (op) /* 8,16,32-bit xfer. */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "neon 8/16/32 bit xfer", > + dsc); > + else /* VFP data processing. */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "vfp dataproc", dsc); > + } > + else > + { > + if (op) > + { > + if (bit_4) /* MRC/MRC2 */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "mrc/mrc2", dsc); > + else /* MCR/MCR2 */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "mcr/mcr2", dsc); > + } > + else /* CDP/CDP 2 */ > + return thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "cdp/cdp2", dsc); > + } Likewise I'm not sure there is any need to decode to such depth, if the instruction in the end all can be copied unmodified. > static int > +thumb_copy_pc_relative_32bit (struct gdbarch *gdbarch, struct regcache *regs, > + struct displaced_step_closure *dsc, > + int rd, unsigned int imm) > +{ > + /* Encoding T3: ADDS Rd, Rd, #imm */ Why do you refer to ADDS? The instruction you generate is ADD (with no S bit), which is actually correct -- so it seems just the comment is wrong. > + dsc->modinsn[0] = (0xf100 | rd); > + dsc->modinsn[1] = (0x0 | (rd << 8) | imm); > + > + dsc->numinsns = 2; > + > + install_pc_relative (gdbarch, regs, dsc, rd); > + > + return 0; > +} > + > +static int > +thumb_decode_pc_relative_32bit (struct gdbarch *gdbarch, uint16_t insn1, > + uint16_t insn2, struct regcache *regs, > + struct displaced_step_closure *dsc) > +{ > + unsigned int rd = bits (insn2, 8, 11); > + /* Since immeidate has the same encoding in both ADR and ADDS, so we simply typo > + extract raw immediate encoding rather than computing immediate. When > + generating ADDS instruction, we can simply perform OR operation to set > + immediate into ADDS. */ See above for ADDS vs. ADD. > + unsigned int imm = (insn2 & 0x70ff) | (bit (insn1, 10) << 26); The last bit will get lost, since thumb_copy_pc_relative_32bit only or's the value to the second 16-bit halfword. > + if (debug_displaced) > + fprintf_unfiltered (gdb_stdlog, > + "displaced: copying thumb adr r%d, #%d insn %.4x%.4x\n", > + rd, imm, insn1, insn2); > + > + return thumb_copy_pc_relative_32bit (gdbarch, regs, dsc, rd, imm); > +} B.t.w. I think the distinction between a _decode_ and a _copy_ routine is pointless in this case since the _decode_ routine is only ever called for one single instruction that matches ... it doesn't actually decode anything. > +static int > +decode_thumb_32bit_ld_mem_hints (struct gdbarch *gdbarch, > + uint16_t insn1, uint16_t insn2, > + struct regcache *regs, > + struct displaced_step_closure *dsc) > +{ > + int rd = bits (insn2, 12, 15); > + int user_mode = (bits (insn2, 8, 11) == 0xe); > + int err = 0; > + int writeback = 0; > + > + switch (bits (insn1, 5, 6)) > + { > + case 0: /* Load byte and memory hints */ > + if (rd == 0xf) /* PLD/PLI */ > + { > + if (bits (insn2, 6, 11)) This check doesn't look right to me. > + return thumb2_copy_preload (gdbarch, insn1, insn2, regs, dsc); > + else > + return thumb2_copy_preload_reg (gdbarch, insn1, insn2, regs, dsc); In any case, see the comments above on handling preload instructions. You should only need to handle the "literal" variants. > + } > + else > + { > + int op1 = bits (insn1, 7, 8); > + > + if ((op1 == 0 || op1 == 2) && bit (insn2, 11)) > + writeback = bit (insn2, 8); > + > + return thumb2_copy_ldr_str_ldrb_strb (gdbarch, insn1, insn2, regs, > + dsc, 1, 1, user_mode, > + writeback); > + } > + > + break; > + case 1: /* Load halfword and memory hints */ > + if (rd == 0xf) /* PLD{W} and Unalloc memory hint */ > + { > + if (bits (insn2, 6, 11)) > + return thumb2_copy_preload (gdbarch, insn1, insn2, regs, dsc); > + else > + return thumb2_copy_preload_reg (gdbarch, insn1, insn2, regs, dsc); See above. > + } > + else > + { > + int op1 = bits (insn1, 7, 8); > + > + if ((op1 == 0 || op1 == 2) && bit (insn2, 11)) > + writeback = bit (insn2, 8); > + return thumb2_copy_ldr_str_ldrb_strb (gdbarch, insn1, insn2, regs, > + dsc, 1, 0, user_mode, > + writeback); > + } > + break; > + case 2: /* Load word */ > + { > + int op1 = bits (insn1, 7, 8); > + > + if ((op1 == 0 || op1 == 2) && bit (insn2, 11)) > + writeback = bit (insn2, 8); > + > + return thumb2_copy_ldr_str_ldrb_strb (gdbarch, insn1, insn2, regs, dsc, > + 1, 0, user_mode, writeback); > + break; > + } > + default: > + return thumb_32bit_copy_undef (gdbarch, insn1, insn2, dsc); > + break; > + } > + return 0; > +} > static void > thumb_process_displaced_32bit_insn (struct gdbarch *gdbarch, uint16_t insn1, > uint16_t insn2, struct regcache *regs, > struct displaced_step_closure *dsc) > { > - error (_("Displaced stepping is only supported in ARM mode and Thumb 16bit instructions")); > + int err = 0; > + unsigned short op = bit (insn2, 15); > + unsigned int op1 = bits (insn1, 11, 12); > + > + switch (op1) > + { > + case 1: > + { > + switch (bits (insn1, 9, 10)) > + { > + case 0: /* load/store multiple */ > + switch (bits (insn1, 7, 8)) > + { > + case 0: case 3: /* SRS, RFE */ > + err = thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "srs/rfe", dsc); > + break; > + case 1: case 2: /* LDM/STM/PUSH/POP */ > + /* These Thumb 32-bit insns have the same encodings as ARM > + counterparts. */ "same encodings" isn't quite true ... > + err = thumb2_copy_block_xfer (gdbarch, insn1, insn2, regs, dsc); > + } > + break; Hmm, it seems this case is missing code to handle the load/store dual, load/store exclusive, and table branch instructions (page A6-24 / table A6-17); there should be a check whether bit 6 is zero or one somewhere. > + case 1: > + /* Data-processing (shift register). In ARM archtecture reference > + manual, this entry is > + "Data-processing (shifted register) on page A6-31". However, > + instructions in table A6-31 shows that they are `alu_reg' > + instructions. There is no alu_shifted_reg instructions in > + Thumb-2. */ Well ... they are not *register*-shifted register instructions like there are in ARM mode (i.e. register shifted by another register), but they are still *shifted* register instructions (i.e. register shifted by an immediate). > + err = thumb2_copy_alu_reg (gdbarch, insn1, insn2, regs, > + dsc); (see comments at that function ...) > + break; > + default: /* Coprocessor instructions */ > + /* Thumb 32bit coprocessor instructions have the same encoding > + as ARM's. */ (see above as to "same encoding" ... also, some ARM coprocessor instruction may in fact use the PC, while no Thumb coprocessor instruction can ... so there is probably no need to decode them further at this point) > + err = thumb2_decode_svc_copro (gdbarch, insn1, insn2, regs, dsc); > + break; > + } > + break; > + } > + case 2: /* op1 = 2 */ > + if (op) /* Branch and misc control. */ > + { > + if (bit (insn2, 14)) /* BLX/BL */ > + err = thumb2_copy_b_bl_blx (gdbarch, insn1, insn2, regs, dsc); > + else if (!bits (insn2, 12, 14) && bits (insn1, 8, 10) != 0x7) I don't understand this condition, but it looks wrong to me ... > + /* Conditional Branch */ > + err = thumb2_copy_b_bl_blx (gdbarch, insn1, insn2, regs, dsc); > + else > + err = thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "misc ctrl", dsc); > + } > + else > + { > + if (bit (insn1, 9)) /* Data processing (plain binary imm) */ > + { > + int op = bits (insn1, 4, 8); > + int rn = bits (insn1, 0, 4); > + if ((op == 0 || op == 0xa) && rn == 0xf) > + err = thumb_decode_pc_relative_32bit (gdbarch, insn1, insn2, > + regs, dsc); > + else > + err = thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "dp/pb", dsc); > + } > + else /* Data processing (modified immeidate) */ > + err = thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "dp/mi", dsc); > + } > + break; > + case 3: /* op1 = 3 */ > + switch (bits (insn1, 9, 10)) > + { > + case 0: > + if (bit (insn1, 4)) > + err = decode_thumb_32bit_ld_mem_hints (gdbarch, insn1, insn2, > + regs, dsc); > + else > + { > + if (bit (insn1, 8)) /* NEON Load/Store */ > + err = thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "neon elt/struct load/store", > + dsc); > + else /* Store single data item */ > + { > + int user_mode = (bits (insn2, 8, 11) == 0xe); > + int byte = (bits (insn1, 5, 7) == 0 > + || bits (insn1, 5, 7) == 4); > + int writeback = 0; > + > + if (bits (insn1, 5, 7) < 3 && bit (insn2, 11)) > + writeback = bit (insn2, 8); If things get this complicated, a decode routine might be appropriate. > + > + err = thumb2_copy_ldr_str_ldrb_strb (gdbarch, insn1, insn2, > + regs, dsc, 0, byte, > + user_mode, writeback); > + } > + } > + break; > + case 1: /* op1 = 3, bits (9, 10) == 1 */ > + switch (bits (insn1, 7, 8)) > + { > + case 0: case 1: /* Data processing (register) */ > + err = thumb2_copy_alu_reg (gdbarch, insn1, insn2, regs, dsc); > + break; > + case 2: /* Multiply and absolute difference */ > + err = thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "mul/mua/diff", dsc); > + break; > + case 3: /* Long multiply and divide */ > + err = thumb_copy_unmodified_32bit (gdbarch, insn1, insn2, > + "lmul/lmua", dsc); > + break; > + } > + break; > + default: /* Coprocessor instructions */ > + err = thumb2_decode_svc_copro (gdbarch, insn1, insn2, regs, dsc); > + break; > + } > + break; > + default: > + err = 1; > + } > + > + if (err) > + internal_error (__FILE__, __LINE__, > + _("thumb_process_displaced_32bit_insn: Instruction decode error")); > + > } Thanks, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com