RE: [PATCH v3 5/5] target/hexagon: Remove unreachable

[<ltaylorsimpson@gmail.com>]

> -----Original Message-----
> From: Brian Cain <brian.cain@oss.qualcomm.com>
> Sent: Monday, April 7, 2025 1:27 PM
> To: qemu-devel@nongnu.org
> Cc: brian.cain@oss.qualcomm.com; richard.henderson@linaro.org;
> philmd@linaro.org; matheus.bernardino@oss.qualcomm.com; ale@rev.ng;
> anjo@rev.ng; marco.liebel@oss.qualcomm.com; ltaylorsimpson@gmail.com;
> alex.bennee@linaro.org; quic_mburton@quicinc.com;
> sidneym@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.cain@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?

>      }
> +    /*
> +     * 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.  A better option is to return a special value that indicates "invalid" and have decode_packet return 0 which will cause decode_and_translate_packet to generate the exception before generating the code for any other instructions in the packet.

Do you have a test case for this?

Taylor