Re: [try 2nd 5/8] Displaced stepping for Thumb 32-bit insns

Ulrich Weigand
2011-05-17T17:14:42Z

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]<cond> rt, [rn, #imm], etc.
> + ->
> + {ldr,str}[b]<cond> r0, [r2, #imm]. */
> + {
> + dsc->modinsn[0] = (insn1 & 0xfff0) | 0x2;
> + dsc->modinsn[1] = insn2 & 0x0fff;
> + }
> + else
> + /* {ldr,str}[b]<cond> rt, [rn, rm], etc.
> + ->
> + {ldr,str}[b]<cond> 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