On Mon, 14 Apr 2025 11:19:38 -0600 <ltaylorsimp...@gmail.com> wrote:
>
> > -----Original Message-----
> > From: Brian Cain <brian.c...@oss.qualcomm.com>
> > Sent: Monday, April 7, 2025 1:27 PM
> > To: qemu-devel@nongnu.org
> > Cc: brian.c...@oss.qualcomm.com; richard.hender...@linaro.org;
> > phi...@linaro.org; matheus.bernard...@oss.qualcomm.com; a...@rev.ng;
> > a...@rev.ng; marco.lie...@oss.qualcomm.com; ltaylorsimp...@gmail.com;
> > alex.ben...@linaro.org; quic_mbur...@quicinc.com;
> > sidn...@quicinc.com
> > Subject: [PATCH v3 5/5] target/hexagon: Remove unreachable
> >
> > We should raise an exception in the event that we encounter a packet that
> > can't be correctly decoded, not fault.
> >
> > Signed-off-by: Brian Cain <brian.c...@oss.qualcomm.com>
> > ---
> > target/hexagon/decode.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c index
> > b5ece60450..1db7f1950f 100644
> > --- a/target/hexagon/decode.c
> > +++ b/target/hexagon/decode.c
> > @@ -489,7 +489,6 @@ decode_insns(DisasContext *ctx, Insn *insn, uint32_t
> > encoding)
> > insn->iclass = iclass_bits(encoding);
> > return 1;
> > }
> > - g_assert_not_reached();
> > } else {
> > uint32_t iclass = get_duplex_iclass(encoding);
> > unsigned int slot0_subinsn = get_slot0_subinsn(encoding); @@ -512,6
> > +511,11 @@ decode_insns(DisasContext *ctx, Insn *insn, uint32_t encoding)
> > }
> > g_assert_not_reached();
>
> Why leave this one rather than raising an exception?
Good point. I think this one should be removed as well. We have removed it
downstream already.
> > }
> > + /*
> > + * invalid/unrecognized opcode; return 1 and let gen_insn() raise an
> > + * exception when it sees this empty insn.
> > + */
> > + return 1;
>
> You should set insn->generate to NULL if you want to guarantee that gen_insn
> will raise an exception.
The caller already memset's it to 0 before passing `insn` down.
> Do you have a test case for this?
We do have a softmmu test for this downstream. Maybe we can adjust it for
user-mode and upstream it with this patch.
