On 30 January 2015 at 12:36, Kirill Batuzov <batuz...@ispras.ru> wrote:
> The documentation states that if LSB > MSB in BFI instruction behaviour
> is unpredictable. Currently QEMU crashes because of assertion failure in
> this case:
>
> tcg/tcg-op.h:2061: tcg_gen_deposit_i32: Assertion `len <= 32' failed.
>
> While assertion failure may meet the "unpredictable" definition this
> behaviour is undesirable because it allows an unprivileged guest program
> to crash the emulator with the OS and other programs.
Thanks for this bug fix. Some minor nits:
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index bdfcdf1..2821289 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -8739,6 +8739,8 @@ static void disas_arm_insn(DisasContext *s, unsigned
> int insn)
> ARCH(6T2);
> shift = (insn >> 7) & 0x1f;
> i = (insn >> 16) & 0x1f;
> + if (i < shift)
> + goto illegal_op;
This needs braces to comply with coding style. checkpatch.pl
should warn you about this.
I also like to comment this kind of check with
/* UNPREDICTABLE; we choose to UNDEF */
> i = i + 1 - shift;
> if (rm == 15) {
> tmp = tcg_temp_new_i32();
I checked the Thumb decoder, and that code seems to have
this test already, so it's just the ARM decoder that
needs fixing.
thanks
-- PMM
