* [Qemu-devel] [PATCH] target-arm: Fix UNDEF cases in Thumb load/store
@ 2011-03-10 16:48 Peter Maydell
2011-03-22 6:54 ` Aurelien Jarno
0 siblings, 1 reply; 2+ messages in thread
From: Peter Maydell @ 2011-03-10 16:48 UTC (permalink / raw)
To: qemu-devel; +Cc: patches
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.maydell@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(-)
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] target-arm: Fix UNDEF cases in Thumb load/store
2011-03-10 16:48 [Qemu-devel] [PATCH] target-arm: Fix UNDEF cases in Thumb load/store Peter Maydell
@ 2011-03-22 6:54 ` Aurelien Jarno
0 siblings, 0 replies; 2+ messages in thread
From: Aurelien Jarno @ 2011-03-22 6:54 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, patches
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.maydell@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
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-03-22 7:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-10 16:48 [Qemu-devel] [PATCH] target-arm: Fix UNDEF cases in Thumb load/store Peter Maydell
2011-03-22 6:54 ` Aurelien Jarno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).