On Wed, 2021-04-14 at 18:23 -0700, Max Filippov wrote:
> On Wed, Apr 14, 2021 at 12:43 PM Richard Henderson
> <[email protected]> wrote:
> >
> > On 4/14/21 11:03 AM, Max Filippov wrote:
> > > On Wed, Apr 14, 2021 at 9:51 AM Ilya Leoshkevich <
> > > [email protected]> wrote:
> > > > On Wed, 2021-04-14 at 16:48 +0200, David Hildenbrand wrote:
> > > > > Did you double-check the xtensa issue?
> > > >
> > > > Oh, I'm sorry, I completely forgot about that one. I just ran the
> > > > test locally, and apparently it fails because of this new assert,
> > > > so
> > > > I'll have to write the 4th patch now. Thanks!
> > >
> > > Just curious, what xtensa issue?
> >
> > Returning from xtensa_tr_translate_insn with tb->size == 0.
> >
> > Basically, dc->base.pc_next needs to be incremented even for illegal
> > instructions, preferably by the number of bytes consumed while
> > determining that
> > the insn is illegal.
>
> I see a few places where target/xtensa may do that. E.g. it does that
> on entry
> to an exception handler to allow for debugging its first instruction.
> No guest code
> is consumed to make this decision, would size 1 work in that case?
> I'll take a look.
Returning 1 was my idea as well. Here is what seems to fix the failure
and what I'm currently testing locally:
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -917,6 +917,8 @@ static void disas_xtensa_insn(CPUXtensaState *env,
DisasContext *dc)
"unknown instruction length (pc = %08x)\n",
dc->pc);
gen_exception_cause(dc, ILLEGAL_INSTRUCTION_CAUSE);
+ dc->base.pc_next = dc->pc + 1;
+ dc->base.is_jmp = DISAS_NORETURN;
return;
}
@@ -1274,11 +1276,13 @@ static void
xtensa_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
if ((tb_cflags(dc->base.tb) & CF_USE_ICOUNT)
&& (dc->base.tb->flags & XTENSA_TBFLAG_YIELD)) {
gen_exception(dc, EXCP_YIELD);
+ dc->base.pc_next = dc->pc + 1;
dc->base.is_jmp = DISAS_NORETURN;
return;
}
if (dc->base.tb->flags & XTENSA_TBFLAG_EXCEPTION) {
gen_exception(dc, EXCP_DEBUG);
+ dc->base.pc_next = dc->pc + 1;
dc->base.is_jmp = DISAS_NORETURN;
return;
}
