Left shift of negative value is UB for `AArch64_SSHLLv2i32_shift`

[tom91136]

The following templated function is UB if N is a signed type and a negative value appears in the left shift on line 710:

https://github.com/UoB-HPC/SimEng/blob/62c2890960c5b88adf0cbaae84bd723eded77cb5/src/include/simeng/arch/aarch64/helpers/neon.hh#L700-L714

The described UB happened at the specialisation site:

https://github.com/UoB-HPC/SimEng/blob/62c2890960c5b88adf0cbaae84bd723eded77cb5/src/lib/arch/aarch64/Instruction_execute.cc#L3794

One of the tests actually checks for this:

https://github.com/UoB-HPC/SimEng/blob/62c2890960c5b88adf0cbaae84bd723eded77cb5/test/regression/aarch64/instructions/neon.cc#L2907-L2932

So heap[1] is negative which triggers UB, this happens for the other negative values as well. Note that on x86 the actual behaviour from GCC produced the expected outcome (as if applied on an unsigned int) but I can imagine Clang throwing a runtime error which is much better. Looking at how vecShllShift_vecImm is used, we get the following:

Instruction_execute.cc:3794 neonHelp::vecShllShift_vecImm<int64_t, int32_t, 2>
Instruction_execute.cc:3799 neonHelp::vecShllShift_vecImm<int64_t, int32_t, 2>
Instruction_execute.cc:4785 neonHelp::vecShllShift_vecImm<uint16_t, uint8_t, 8>
Instruction_execute.cc:4790 neonHelp::vecShllShift_vecImm<uint32_t, uint16_t, 4>
Instruction_execute.cc:4795 neonHelp::vecShllShift_vecImm<uint32_t, uint16_t, 4>
Instruction_execute.cc:4800 neonHelp::vecShllShift_vecImm<uint16_t, uint8_t, 8>

So I think the fix is to simply use neonHelp::vecShllShift_vecImm<uint64_t, uint32_t, 2> for Instruction_execute.cc:3794 and Instruction_execute.cc:3799. We can then guard this by adding the following assertion to the first line in neonHelp::vecShllShift_vecImm:

static_assert(std::is_unsigned_v<N>, "LHS of SHL must be unsigned, UB otherwise");

I've tested locally with this change and all AArch64 test passes. Let me know if PRs are welcome.

[FinnWilkinson]

Thanks for finding this Tom. Yes pull requests are welcome

[FinnWilkinson]

After looking into the instruction SSHLL, this performs a signed shift. We will need to make sure that your proposed change does not effect the intended functionality i.e. possibly by adding additional tests & verifying against hardware

[FinnWilkinson]

After verifying on hardware, I think a viable solution will be to cast the input vector elements to their unsigned variant (as for all representations UINT_MAX > INT_MAX && UINT_MAX > abs(INT_MIN)) shift them by the imm value specified in the instruction, before casting back to their original type. Let me know what you think?