Skip to content

Commit

Permalink
riscv: Invalid 32-bit instruction should not decrement pc
Browse files Browse the repository at this point in the history
This line appears to be trying to undo the effect of adding 4 to pc above,
but does so incorrectly and so ends up returning with next_pc earlier than
it was prior to decoding.

This causes the translator to malfunction because it does not expect
pc_next to decrease during decoding: this is effectively reporting that
the invalid construction has a negative size, which is impossible. The
decoder uses the increase in next_pc to decide the translation block size,
but converts it to uint16_t thereby causing a block containing _only_ an
invalid instruction to be treated as having size 65532 (reinterpreted -4)
and therefore the translation loop tries to find the next translation block
at 65532 bytes after the invalid instruction, which can cause a spurious
instruction access/page fault if the page containing that address is not
mapped as executable.

In practice we don't need to readjust the pc at all here because it is
correct to report that the invalid instruction is four bytes long. This
allows the translation loop to correctly find the next instruction, and
to avoid producing spurious TLB fills that might cause incorrect exceptions.
  • Loading branch information
apparentlymart committed Aug 28, 2024
1 parent 9712ac7 commit 9b75f8c
Showing 1 changed file with 0 additions and 1 deletion.
1 change: 0 additions & 1 deletion qemu/target/riscv/translate.c
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,6 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
translator_lduw(tcg_ctx, env, ctx->base.pc_next + 2));
ctx->pc_succ_insn = ctx->base.pc_next + 4;
if (!decode_insn32(ctx, opcode32)) {
ctx->pc_succ_insn = ctx->base.pc_next - 4;
gen_exception_illegal(ctx);
}
}
Expand Down

0 comments on commit 9b75f8c

Please sign in to comment.