On Thu, Mar 10, 2011 at 04:48:49PM +0000, Peter Maydell wrote:
> Decode of Thumb load/store was merging together the cases of 'bit 11==0'
> (reg+reg LSL imm) and 'bit 11==1' (reg+imm). This happens to work for
> valid instruction patterns but meant that we would not UNDEF for the
> cases the architecture mandates that we must. Make the decode actually
> look at bit 11 as well as [10..8] so that we UNDEF in the right places.
>
> This change also removes what was a spurious unreachable 'case 8',
> and correctly frees TCG temporaries on the illegal-insn codepaths.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> This patch was mostly prompted by that dodgy 'case 8' which I noted
> when doing the preload/hint space patches a month or so ago; I have
> finally added support for testing loads and stores to risu, so I
> can confirm that this patch doesn't break the non-UNDEF cases.
>
> target-arm/translate.c | 29 ++++++++++++++++++-----------
> 1 files changed, 18 insertions(+), 11 deletions(-)
Thanks, applied.
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 062de5e..0afdbfb 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -8378,39 +8378,42 @@ static int disas_thumb2_insn(CPUState *env,
> DisasContext *s, uint16_t insn_hw1)
> tcg_gen_addi_i32(addr, addr, imm);
> } else {
> imm = insn & 0xff;
> - switch ((insn >> 8) & 7) {
> - case 0: case 8: /* Shifted Register. */
> + switch ((insn >> 8) & 0xf) {
> + case 0x0: /* Shifted Register. */
> shift = (insn >> 4) & 0xf;
> - if (shift > 3)
> + if (shift > 3) {
> + tcg_temp_free_i32(addr);
> goto illegal_op;
> + }
> tmp = load_reg(s, rm);
> if (shift)
> tcg_gen_shli_i32(tmp, tmp, shift);
> tcg_gen_add_i32(addr, addr, tmp);
> tcg_temp_free_i32(tmp);
> break;
> - case 4: /* Negative offset. */
> + case 0xc: /* Negative offset. */
> tcg_gen_addi_i32(addr, addr, -imm);
> break;
> - case 6: /* User privilege. */
> + case 0xe: /* User privilege. */
> tcg_gen_addi_i32(addr, addr, imm);
> user = 1;
> break;
> - case 1: /* Post-decrement. */
> + case 0x9: /* Post-decrement. */
> imm = -imm;
> /* Fall through. */
> - case 3: /* Post-increment. */
> + case 0xb: /* Post-increment. */
> postinc = 1;
> writeback = 1;
> break;
> - case 5: /* Pre-decrement. */
> + case 0xd: /* Pre-decrement. */
> imm = -imm;
> /* Fall through. */
> - case 7: /* Pre-increment. */
> + case 0xf: /* Pre-increment. */
> tcg_gen_addi_i32(addr, addr, imm);
> writeback = 1;
> break;
> default:
> + tcg_temp_free_i32(addr);
> goto illegal_op;
> }
> }
> @@ -8423,7 +8426,9 @@ static int disas_thumb2_insn(CPUState *env,
> DisasContext *s, uint16_t insn_hw1)
> case 1: tmp = gen_ld16u(addr, user); break;
> case 5: tmp = gen_ld16s(addr, user); break;
> case 2: tmp = gen_ld32(addr, user); break;
> - default: goto illegal_op;
> + default:
> + tcg_temp_free_i32(addr);
> + goto illegal_op;
> }
> if (rs == 15) {
> gen_bx(s, tmp);
> @@ -8437,7 +8442,9 @@ static int disas_thumb2_insn(CPUState *env,
> DisasContext *s, uint16_t insn_hw1)
> case 0: gen_st8(tmp, addr, user); break;
> case 1: gen_st16(tmp, addr, user); break;
> case 2: gen_st32(tmp, addr, user); break;
> - default: goto illegal_op;
> + default:
> + tcg_temp_free_i32(addr);
> + goto illegal_op;
> }
> }
> if (postinc)
> --
> 1.7.1
>
>
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
