has_valid_slot_assignment() rejected any packet with two instructions
assigned to the same slot.  That is too strict.  When a memory
instruction is encoded before a slot-flexible instruction in a packet,
the descending slot assignment places the memory op in slot 1 and the
other in slot 0, then the "mem insns to slot 0" fixup moves the memory
op to slot 0 as well, leaving both in slot 0.  Such a packet is valid
and executes correctly, but the uniqueness test flagged it as
HEX_CAUSE_INVALID_PACKET, raising SIGILL in linux-user and a precise
exception in system mode.

For example this packet, with the load encoded first, was wrongly
rejected:

    { r6 = memw(r3+#-4)
      r7 = #0x4ae6 }

Replace the uniqueness test with a slot-exhaustion check: walk the
instructions in encoding order handing out slots in strictly decreasing
order and fail only if an instruction has no valid slot at or below the
running slot.  This accepts packets that legally share a slot while
still rejecting genuinely unassignable packets, such as a memory
instruction grouped with a duplex, or a load followed by an instruction
that requires a high slot.

Signed-off-by: Brian Cain <[email protected]>
---
 target/hexagon/decode.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
index 6eddcca26ed..e6bb7733be8 100644
--- a/target/hexagon/decode.c
+++ b/target/hexagon/decode.c
@@ -549,21 +549,35 @@ static bool decode_parsebits_is_loopend(uint32_t 
encoding32)
     return bits == 0x2;
 }
 
+/*
+ * Check that the packet's instructions can be grouped into slots: walk them
+ * in encoding order handing out slots in strictly decreasing order, and fail
+ * if an instruction has no valid slot at or below the running slot.  Two
+ * instructions may legally share a slot, so this does not require unique
+ * slots, only that every instruction fits.
+ */
 static bool has_valid_slot_assignment(Packet *pkt)
 {
-    int used_slots = 0;
-    for (int i = 0; i < pkt->num_insns; i++) {
-        int slot_mask;
-        Insn *insn = &pkt->insn[i];
-        if (decode_opcode_ends_loop(insn->opcode)) {
+    int i;
+    int slot = 3;
+
+    for (i = 0; i < pkt->num_insns; i++) {
+        SlotMask valid_slots;
+        if (decode_opcode_ends_loop(pkt->insn[i].opcode)) {
             /* We overload slot 0 for endloop. */
             continue;
         }
-        slot_mask = 1 << insn->slot;
-        if (used_slots & slot_mask) {
+        if (slot < 0) {
             return false;
         }
-        used_slots |= slot_mask;
+        valid_slots = get_valid_slots(pkt, i);
+        while (!(valid_slots & (1 << slot))) {
+            if (slot <= 0) {
+                return false;
+            }
+            slot--;
+        }
+        slot--;
     }
     return true;
 }
-- 
2.34.1


Reply via email to