IREmitter::emitSpecialNode switch has incorrect EXT_ARMISD cases

[Kloppie5]

IREmitter::emitSpecialNode uses a switch on the OpCode of the node, but some of the cases use the EXT_ARMISD values. These actually correspond to VSTL4 instructions, and none of these cases are actually triggered in the entire test suite, except for ones that are hit through fallthrough. Instead, these and other important instructions like SVC go to the empty default case.
https://github.com/microsoft/llvm-mctoll/blob/0818e44b274204234859ae0e8645d022748e828d/ARM/DAG/IREmitter.cpp#:~:text=case%20EXT_ARMISD%3A%3ABX_RET,%7D%20break%3B

[bharadwajy]

Thanks for your interest in the project and for raising the issue.

Support to raise ARM64 binaries has been lagging for a while now due to paucity of resources.

Any help is highly appreciated.

[Kloppie5]

I have been tinkering with the codebase a bit and I want to help out with improving the ARM lifting. The problem I'm running into is that the way ARM is being lifted is completely different and separate of how x86 is lifted. The changes I have been making on a local branch change huge parts of the codebase; would it be preferred if I were to change the ARM lifting to work in a similar fashion as the x86 lifting, or are you ok with big changes?

[bharadwajy]

Thank you very much for offering to improve support for raising ARM binaries.

As you rightly observed, whatever support that exists for ARM raising is quite different from that for x64. However, I think the implementation of ARM binary raising should / could be similar to that of x64 binaries. It would be good to share the data structures, algorithms and other abstract class infrastructure, whenever possible. That would also give an opportunity to improve the implementation of x64, IMHO, which in turn could establish a good base to add support for other ISA binaries in the future.

Regarding size of patches preferred: while small to medium-sized would be nice, I think patches that encapsulate an incremental feature along with a (couple of?) test(s) to verify the added feature (when applicable) would be very helpful.

HTH!