[PATCH 2/3] target/hexagon: Reject duplex encodings with duplicate dest registers

Brian Cain posted 3 patches 2 days, 17 hours ago
Maintainers: Brian Cain <brian.cain@oss.qualcomm.com>, Laurent Vivier <laurent@vivier.eu>, Pierrick Bouvier <pierrick.bouvier@linaro.org>
There is a newer version of this series
[PATCH 2/3] target/hexagon: Reject duplex encodings with duplicate dest registers
Posted by Brian Cain 2 days, 17 hours ago
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>
---
 target/hexagon/decode.c           | 12 ++++++++++++
 tests/tcg/hexagon/invalid-dups.c  | 23 +++++++++++++++++++++++
 tests/tcg/hexagon/Makefile.target |  6 ++++++
 3 files changed, 41 insertions(+)
 create mode 100644 tests/tcg/hexagon/invalid-dups.c

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-dups.c b/tests/tcg/hexagon/invalid-dups.c
new file mode 100644
index 0000000000..cb37ef7066
--- /dev/null
+++ b/tests/tcg/hexagon/invalid-dups.c
@@ -0,0 +1,23 @@
+/*
+ * Test that duplex encodings with duplicate destination registers are rejected.
+ *
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+/*
+ * 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.
+ */
+
+int main()
+{
+    asm volatile(
+        ".word 0x00000000\n"
+        : : : "r0", "memory");
+    return 0;
+}
diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target
index b0e20139c2..7199e29a30 100644
--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -52,6 +52,7 @@ HEX_TESTS += hvx_misc
 HEX_TESTS += hvx_histogram
 HEX_TESTS += invalid-slots
 HEX_TESTS += invalid-duplex
+HEX_TESTS += invalid-dups
 HEX_TESTS += unaligned_pc
 
 run-and-check-exception = $(call run-test,$2,$3 2>$2.stderr; \
@@ -68,6 +69,11 @@ run-invalid-duplex: invalid-duplex
 		$(QEMU) $(QEMU_OPTS) $< ; test $$? -eq 132, \
 		TEST, invalid-duplex on $(TARGET_NAME))
 
+run-invalid-dups: invalid-dups
+	$(call quiet-command, \
+		$(QEMU) $(QEMU_OPTS) $< ; test $$? -eq 132, \
+		TEST, invalid-dups on $(TARGET_NAME))
+
 HEX_TESTS += test_abs
 HEX_TESTS += test_bitcnt
 HEX_TESTS += test_bitsplit
-- 
2.34.1

Re: [PATCH 2/3] target/hexagon: Reject duplex encodings with duplicate dest registers
Posted by Pierrick Bouvier 2 days, 13 hours ago
On 2/6/26 10:38 AM, Brian Cain 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>
> ---
>   target/hexagon/decode.c           | 12 ++++++++++++
>   tests/tcg/hexagon/invalid-dups.c  | 23 +++++++++++++++++++++++
>   tests/tcg/hexagon/Makefile.target |  6 ++++++
>   3 files changed, 41 insertions(+)
>   create mode 100644 tests/tcg/hexagon/invalid-dups.c
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>