AArch32: vpop (sreg variant) had faulty mult_addr ordering

[Sleigh-InSPECtor]

As part of a research project testing the accuracy of the SLEIGH specifications compared to real hardware, we observed an unexpected behaviour in the vpop instruction for both, AArch32 (ARM:LE:32:v8) & Thumb (ARM:LE:32:v8T).

According to the manual, it loads multiple consecutive Advanced SIMD and floating-point register file registers from the stack. However, we noticed the output was incorrect.

---

e.g, for AArch32 with,

Instruction: 0x020abd7c, vpopvc {s0,s1}
initial_memory: { "0x0": [ 0xbb, 0xda, 0x60, 0xca, 0xe8, 0x7c, 0x26, 0xcf ] }

We get:

Hardware: { "q0": 0xcf267ce8ca60dabb, "sp": 0x8 }
Patched Spec: { "q0": 0xcf267ce8ca60dabb, "sp": 0x8 }
Existing Spec: { "q0": 0xca60dabbcf267ce8, "sp": 0x8 }

---

e.g, for Thumb with,

Instruction: 0xbdec020a, vpop {s0,s1}
initial_memory: { "0x0": [ 0xbb, 0xda, 0x60, 0xca, 0xe8, 0x7c, 0x26, 0xcf ] }

We get:

Hardware: { "q0": 0xcf267ce8ca60dabb, "sp": 0x8 }
Patched Spec: { "q0": 0xcf267ce8ca60dabb, "sp": 0x8 }
Existing Spec: { "q0": 0xca60dabbcf267ce8, "sp": 0x8 }

---

Note: The patched spec does not introduce any disassembly changes to the best of our knowledge.

[GhidorahRex]

Should counter be used in place of c0007 - 1 for clarity?

[GhidorahRex]

The operational encoding for vldm uses addition, not subtraction. The sleigh for the vldm* instructions do the same thing. So why is this instruction that is an alias for vldm using subtraction?

[Sleigh-InSPECtor]

Yeah, checking through the implementation more carefully, it looks like the actual discrepancy between the vpop semantics and the vldm semantics is caused by a missing build statement.

The existing unpatched Ghidra generates Pcode that looks something like:

020abdec "vpop {s0,s1}"

mult_addr = sp
sp = sp + 0x8:4
tmp:4 = *[ram]mult_addr
s1 = tmp
mult_addr = mult_addr + 0x4:4
tmp:4 = *[ram]mult_addr
s0 = tmp

i.e., s0 and s1 are loaded in the wrong order.

The old 'fix' kept the operations in the same order, but adjusted the way mult_addr was computed so the values for s0 and s1 were loaded from the correct addresses.

The PR has been updated with a much simpler fix that simply adds the missing build so that the operations are ordered correctly (matching the vldm and being closer to reference manual):

Pcode after new patch:

mult_addr = sp
sp = sp + 0x8:4
tmp:4 = *[ram]mult_addr
s0 = tmp
mult_addr = mult_addr + 0x4:4
tmp:4 = *[ram]mult_addr
s1 = tmp

The same thing is true for #6604 (which has also been updated).