[ARM32] Hook for "svc" always returns interrupt number 2

[kittennbfive]

Hello,

sorry to bother you again. I think this time i found a real bug regarding ARM32 "svc" instruction (supervisor call).

I compiled current master (f8c6db9) on Linux.

Please consider the attached testcase.

Expected output:

start test
svc 0
svc 1
svc 2
svc 3
svc 4
svc 5
svc 6
test done

Real output:

start test
svc 2
svc 2
svc 2
svc 2
svc 2
svc 2
svc 2
test done

I spent a lot of time tracking this down but as i am not familiar with QEMU/Unicorn-internals i don't know the correct way to fix this.

If an svc $n instruction is encountered then https://github.com/unicorn-engine/unicorn/blob/master/qemu/target/arm/op_helper.c#L30 is somehow called with excp set to EXCP_SWI (==2, the wrong value we are always seeing, defined inside https://github.com/unicorn-engine/unicorn/blob/master/qemu/target/arm/cpu.h ). The actual svc call number is inside syndrome with some other informations (i don't understand the details).

At https://github.com/unicorn-engine/unicorn/blob/master/qemu/target/arm/op_helper.c#L49 cs->exception_index is then set to excp (which is 2), but syndrome (which contails the actual call number we are interested in) is not stored inside cs. Maybe there is the (first part of) the problem?

See also the comment at https://github.com/unicorn-engine/unicorn/blob/master/qemu/target/arm/cpu.h#L528 .

I have hacked together a patch, attached here, that works for me; but as i have basically no idea what i am doing i won't open a PR. Somebody else should look at this. No hurry, for me the bug is kinda "resolved".

Thank you.

main.c.txt
arm_svc_patch.diff.txt

[wtdcode]

Qiling workarounds this by: https://github.com/qilingframework/qiling/blob/a430518ef2026c3fa9d69df5831581b9ac3aa368/qiling/os/posix/syscall/abi/arm.py#L26

Your patch is correct at first glance. Nice catch!

[kittennbfive]

Thank you. Should i open a PR?

[wtdcode]

Thank you. Should i open a PR?

Sure please do so. This also let CI check if it breaks other things.

[kittennbfive]

PR is there after some false start. I am not very familiar with github...

[wtdcode]

@amaanq Gives a much better way to handle this, please refer to this example:

https://github.com/unicorn-engine/unicorn/pull/2155/files#diff-b1dab5e34001e4643d754694930562a45821b8bcc8fda0a72720c06d1c5d032fR1001

The change will live at dev branch until next release.

[kittennbfive]

No problem for me, thank you!

[kittennbfive]

Just for the record: My patch (that has been superseeded) would break unit test test_arm_m_exc_return_hook_interrupt().

This issue can be closed.