A duplex encoding like 0x00000000 decodes as two loads that both write r0.
Add a check in decode_insns() after both sub-instructions decode
successfully to verify they don't write the same destination register.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2696
Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
target/hexagon/decode.c | 12 ++++++++++++
tests/tcg/hexagon/invalid-encoding.c | 29 ++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
index 69ba1ec96c..90499fc320 100644
--- a/target/hexagon/decode.c
+++ b/target/hexagon/decode.c
@@ -501,12 +501,24 @@ decode_insns(DisasContext *ctx, Insn *insn, uint32_t encoding)
/* The slot1 subinsn needs to be in the packet first */
if (decode_slot1_subinsn(ctx, slot1_subinsn)) {
+ Insn *slot1_insn = insn;
insn->generate = opcode_genptr[insn->opcode];
insn->iclass = iclass_bits(encoding);
ctx->insn = ++insn;
if (decode_slot0_subinsn(ctx, slot0_subinsn)) {
insn->generate = opcode_genptr[insn->opcode];
insn->iclass = iclass_bits(encoding);
+ /*
+ * Check that the two sub-instructions don't write the same
+ * destination register (e.g., encoding 0x0 decodes as two
+ * loads both writing R0, which is an invalid packet).
+ */
+ if (insn->dest_idx >= 0 && slot1_insn->dest_idx >= 0 &&
+ insn->regno[insn->dest_idx] ==
+ slot1_insn->regno[slot1_insn->dest_idx]) {
+ ctx->insn = --insn;
+ return 0;
+ }
return 2;
}
/*
diff --git a/tests/tcg/hexagon/invalid-encoding.c b/tests/tcg/hexagon/invalid-encoding.c
index 010a5eb741..1bbd312b61 100644
--- a/tests/tcg/hexagon/invalid-encoding.c
+++ b/tests/tcg/hexagon/invalid-encoding.c
@@ -65,6 +65,34 @@ static int test_invalid_duplex(void)
return sig;
}
+/*
+ * Duplex with duplicate destination registers (issue #2696):
+ * The encoding 0x00000000 decodes as a duplex with parse bits
+ * [15:14] = 0b00:
+ * slot1: SL1_loadri_io R0 = memw(R0+#0x0)
+ * slot0: SL1_loadri_io R0 = memw(R0+#0x0)
+ *
+ * Both sub-instructions write R0, which is an invalid packet (duplicate
+ * destination register). This should raise SIGILL.
+ */
+static int test_invalid_dups(void)
+{
+ int sig;
+
+ asm volatile(
+ "r0 = #0\n"
+ "r1 = ##1f\n"
+ "memw(%1) = r1\n"
+ ".word 0x00000000\n"
+ "1:\n"
+ "%0 = r0\n"
+ : "=r"(sig)
+ : "r"(&resume_pc)
+ : "r0", "r1", "memory");
+
+ return sig;
+}
+
int main()
{
struct sigaction act;
@@ -75,6 +103,7 @@ int main()
assert(sigaction(SIGILL, &act, NULL) == 0);
assert(test_invalid_duplex() == SIGILL);
+ assert(test_invalid_dups() == SIGILL);
puts("PASS");
return EXIT_SUCCESS;
--
2.34.1
On Sat, Feb 7, 2026 at 11:06 AM Brian Cain <brian.cain@oss.qualcomm.com>
wrote:
> A duplex encoding like 0x00000000 decodes as two loads that both write r0.
>
> Add a check in decode_insns() after both sub-instructions decode
> successfully to verify they don't write the same destination register.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2696
> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
> target/hexagon/decode.c | 12 ++++++++++++
> tests/tcg/hexagon/invalid-encoding.c | 29 ++++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+)
>
> diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
> index 69ba1ec96c..90499fc320 100644
> --- a/target/hexagon/decode.c
> +++ b/target/hexagon/decode.c
> @@ -501,12 +501,24 @@ decode_insns(DisasContext *ctx, Insn *insn, uint32_t
> encoding)
>
> /* The slot1 subinsn needs to be in the packet first */
> if (decode_slot1_subinsn(ctx, slot1_subinsn)) {
> + Insn *slot1_insn = insn;
> insn->generate = opcode_genptr[insn->opcode];
> insn->iclass = iclass_bits(encoding);
> ctx->insn = ++insn;
> if (decode_slot0_subinsn(ctx, slot0_subinsn)) {
> insn->generate = opcode_genptr[insn->opcode];
> insn->iclass = iclass_bits(encoding);
> + /*
> + * Check that the two sub-instructions don't write the
> same
> + * destination register (e.g., encoding 0x0 decodes as two
> + * loads both writing R0, which is an invalid packet).
> + */
> + if (insn->dest_idx >= 0 && slot1_insn->dest_idx >= 0 &&
> + insn->regno[insn->dest_idx] ==
> + slot1_insn->regno[slot1_insn->dest_idx]) {
> + ctx->insn = --insn;
> + return 0;
> + }
>
> Isn't this a more general problem than what is checked here? What if two
non-duplex instructions write the same register? What if an instruction
writes more than one register (e.g., post-increment load)?
Let the decoding go ahead and finish, then add a check for duplicate writes
for the whole packet. Look at ctx_log_reg_write - called during
analyze_packet.
Thanks,
Taylor
On Sat, Feb 7, 2026 at 3:58 PM Taylor Simpson <ltaylorsimpson@gmail.com> wrote:
>
>
>
> On Sat, Feb 7, 2026 at 11:06 AM Brian Cain <brian.cain@oss.qualcomm.com> wrote:
>>
>> A duplex encoding like 0x00000000 decodes as two loads that both write r0.
>>
>> Add a check in decode_insns() after both sub-instructions decode
>> successfully to verify they don't write the same destination register.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2696
>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>> target/hexagon/decode.c | 12 ++++++++++++
>> tests/tcg/hexagon/invalid-encoding.c | 29 ++++++++++++++++++++++++++++
>> 2 files changed, 41 insertions(+)
>>
>> diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
>> index 69ba1ec96c..90499fc320 100644
>> --- a/target/hexagon/decode.c
>> +++ b/target/hexagon/decode.c
>> @@ -501,12 +501,24 @@ decode_insns(DisasContext *ctx, Insn *insn, uint32_t encoding)
>>
>> /* The slot1 subinsn needs to be in the packet first */
>> if (decode_slot1_subinsn(ctx, slot1_subinsn)) {
>> + Insn *slot1_insn = insn;
>> insn->generate = opcode_genptr[insn->opcode];
>> insn->iclass = iclass_bits(encoding);
>> ctx->insn = ++insn;
>> if (decode_slot0_subinsn(ctx, slot0_subinsn)) {
>> insn->generate = opcode_genptr[insn->opcode];
>> insn->iclass = iclass_bits(encoding);
>> + /*
>> + * Check that the two sub-instructions don't write the same
>> + * destination register (e.g., encoding 0x0 decodes as two
>> + * loads both writing R0, which is an invalid packet).
>> + */
>> + if (insn->dest_idx >= 0 && slot1_insn->dest_idx >= 0 &&
>> + insn->regno[insn->dest_idx] ==
>> + slot1_insn->regno[slot1_insn->dest_idx]) {
>> + ctx->insn = --insn;
>> + return 0;
>> + }
>>
> Isn't this a more general problem than what is checked here? What if two non-duplex instructions write the same register? What if an instruction writes more than one register (e.g., post-increment load)?
It is, yeah. I mentioned in the cover letter that I planned to land that later.
But ok -- I can move it here into this series.
> Let the decoding go ahead and finish, then add a check for duplicate writes for the whole packet. Look at ctx_log_reg_write - called during analyze_packet.
>
> Thanks,
> Taylor
>
On Sat, Feb 7, 2026 at 3:04 PM Brian Cain <brian.cain@oss.qualcomm.com>
wrote:
> On Sat, Feb 7, 2026 at 3:58 PM Taylor Simpson <ltaylorsimpson@gmail.com>
> wrote:
> >
> >
> >
> > On Sat, Feb 7, 2026 at 11:06 AM Brian Cain <brian.cain@oss.qualcomm.com>
> wrote:
> >>
> >> A duplex encoding like 0x00000000 decodes as two loads that both write
> r0.
> >>
> >> Add a check in decode_insns() after both sub-instructions decode
> >> successfully to verify they don't write the same destination register.
> >>
> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2696
> >> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> >> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> >> ---
> >> target/hexagon/decode.c | 12 ++++++++++++
> >> tests/tcg/hexagon/invalid-encoding.c | 29 ++++++++++++++++++++++++++++
> >> 2 files changed, 41 insertions(+)
> >>
> >> diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
> >> index 69ba1ec96c..90499fc320 100644
> >> --- a/target/hexagon/decode.c
> >> +++ b/target/hexagon/decode.c
> >> @@ -501,12 +501,24 @@ decode_insns(DisasContext *ctx, Insn *insn,
> uint32_t encoding)
> >>
> >> /* The slot1 subinsn needs to be in the packet first */
> >> if (decode_slot1_subinsn(ctx, slot1_subinsn)) {
> >> + Insn *slot1_insn = insn;
> >> insn->generate = opcode_genptr[insn->opcode];
> >> insn->iclass = iclass_bits(encoding);
> >> ctx->insn = ++insn;
> >> if (decode_slot0_subinsn(ctx, slot0_subinsn)) {
> >> insn->generate = opcode_genptr[insn->opcode];
> >> insn->iclass = iclass_bits(encoding);
> >> + /*
> >> + * Check that the two sub-instructions don't write the
> same
> >> + * destination register (e.g., encoding 0x0 decodes as
> two
> >> + * loads both writing R0, which is an invalid packet).
> >> + */
> >> + if (insn->dest_idx >= 0 && slot1_insn->dest_idx >= 0 &&
> >> + insn->regno[insn->dest_idx] ==
> >> + slot1_insn->regno[slot1_insn->dest_idx]) {
> >> + ctx->insn = --insn;
> >> + return 0;
> >> + }
> >>
> > Isn't this a more general problem than what is checked here? What if
> two non-duplex instructions write the same register? What if an
> instruction writes more than one register (e.g., post-increment load)?
>
> It is, yeah. I mentioned in the cover letter that I planned to land that
> later.
>
> But ok -- I can move it here into this series.
>
Sorry, I missed that comment in the cover letter.
Taylor
© 2016 - 2026 Red Hat, Inc.