| Message ID | 20210821195958.41312-7-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Fix insn exception priorities | expand |
On 8/21/21 9:59 PM, Richard Henderson wrote: > Misaligned thumb PC is architecturally impossible. > Assert is better than proceeding, in case we've missed > something somewhere. > > Expand a comment about aligning the pc in gdbstub. > Fail an incoming migrate if a thumb pc is misaligned. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/gdbstub.c | 9 +++++++-- > target/arm/machine.c | 9 +++++++++ > target/arm/translate.c | 3 +++ > 3 files changed, 19 insertions(+), 2 deletions(-) > diff --git a/target/arm/translate.c b/target/arm/translate.c > index dfeaa2321d..a93ea3c47c 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c > @@ -9595,6 +9595,9 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) > uint32_t insn; > bool is_16bit; > > + /* Misaligned thumb PC is architecturally impossible. */ > + assert((dc->base.pc_next & 1) == 0); What about using tcg_debug_assert() instead? > if (arm_check_ss_active(dc) || arm_check_kernelpage(dc)) { > dc->base.pc_next += 2; > return; >
On Sat, 21 Aug 2021 at 21:04, Richard Henderson <richard.henderson@linaro.org> wrote: > > Misaligned thumb PC is architecturally impossible. > Assert is better than proceeding, in case we've missed > something somewhere. > > Expand a comment about aligning the pc in gdbstub. > Fail an incoming migrate if a thumb pc is misaligned. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 8/21/21 1:46 PM, Philippe Mathieu-Daudé wrote: >> + /* Misaligned thumb PC is architecturally impossible. */ >> + assert((dc->base.pc_next & 1) == 0); > > What about using tcg_debug_assert() instead? I don't think we want to let this one compile out. r~
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index 826601b341..a54b42418b 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -76,8 +76,13 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n) tmp = ldl_p(mem_buf); - /* Mask out low bit of PC to workaround gdb bugs. This will probably - cause problems if we ever implement the Jazelle DBX extensions. */ + /* + * Mask out low bits of PC to workaround gdb bugs. + * This avoids an assert in thumb_tr_translate_insn, because it is + * architecturally impossible to misalign the pc. + * This will probably cause problems if we ever implement the + * Jazelle DBX extensions. + */ if (n == 15) { tmp &= ~1; } diff --git a/target/arm/machine.c b/target/arm/machine.c index 81e30de824..b5004a67e9 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -781,6 +781,15 @@ static int cpu_post_load(void *opaque, int version_id) hw_breakpoint_update_all(cpu); hw_watchpoint_update_all(cpu); + /* + * Misaligned thumb pc is architecturally impossible. + * We have an assert in thumb_tr_translate_insn to verify this. + * Fail an incoming migrate to avoid this assert. + */ + if (!is_a64(env) && env->thumb && (env->regs[15] & 1)) { + return -1; + } + if (!kvm_enabled()) { pmu_op_finish(&cpu->env); } diff --git a/target/arm/translate.c b/target/arm/translate.c index dfeaa2321d..a93ea3c47c 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -9595,6 +9595,9 @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) uint32_t insn; bool is_16bit; + /* Misaligned thumb PC is architecturally impossible. */ + assert((dc->base.pc_next & 1) == 0); + if (arm_check_ss_active(dc) || arm_check_kernelpage(dc)) { dc->base.pc_next += 2; return;
Misaligned thumb PC is architecturally impossible. Assert is better than proceeding, in case we've missed something somewhere. Expand a comment about aligning the pc in gdbstub. Fail an incoming migrate if a thumb pc is misaligned. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/gdbstub.c | 9 +++++++-- target/arm/machine.c | 9 +++++++++ target/arm/translate.c | 3 +++ 3 files changed, 19 insertions(+), 2 deletions(-)