Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ARM] compute cf for shift/rotate #1457

Merged
merged 5 commits into from
Mar 21, 2024

Conversation

W0ni
Copy link
Contributor

@W0ni W0ni commented Sep 6, 2023

This partially addresses #1456

  • Moves rrx computation inside the functions, this allows to do the carry flag computation related to RRX
  • Adds a shifter/rotate template for the carry flag similar to the one in x86

It is still (at least) left to:

  • compute the carry flag of some instructions with immediates.
    Here is an example A32ExpandImm_C (https://developer.arm.com/documentation/ddi0597/2023-06/Shared-Pseudocode/aarch32-functions-common?lang=en#impl-aarch32.A32ExpandImm_C.2). Used by ANDS, BICS, EORS, MOVS, MVNS, ORRS, TEQ, TST with immediates.
    The equation is something like cf= ROR(zExt32(imm[0:8], imm[8:12] << 1).MSB() (if 2*imm[8:12] != 0). This is tricky because the semantics don't know the rotate amount that was used. Maybe the additional_info object on instructions could store useful data like rotate amount and maybe the encoding category (A1, A2, T1, T2...)?

  • Update the semantic tests to include the carry flag.

Related additional work:

  • Maybe we could also clean up a bit the code (use flag functions more often, replace isinstance for Expr* by is_XXX...)
  • Maybe handle the exception when setflags AND dst=PC?

@serpilliere
Copy link
Contributor

Hi @W0ni,
Thanks for the PR: We will look into it soon!
(nice tipo catch on the "compote".)

@W0ni
Copy link
Contributor Author

W0ni commented Sep 13, 2023

Hello @serpilliere ,
While adding the tests for MOVS, I noticed a bug in the disassembly of the shifters. I fixed it in the commit 71e4e09 along with the related tests. (See DecodeImmShift at https://developer.arm.com/documentation/ddi0597/2023-06/Shared-Pseudocode/aarch32-functions-common?lang=en#impl-aarch32.DecodeImmShift.2 for reference)
I also added the tests for the MOVS instuctions in 1885f3c.

@W0ni W0ni marked this pull request as ready for review September 13, 2023 10:45
@W0ni W0ni changed the title Draft: [ARM] compute cf for shift/rotate [ARM] compute cf for shift/rotate Sep 13, 2023
@serpilliere
Copy link
Contributor

Hi @W0ni !

I was looking at your patch regarding the disassembling fix, more precisely the support of ASR/LSR and RRX during instruction decoding. I think we should add a regression test on the disassemble engine itself (test/arm/arch.py), in order to test the freshly added code.

Do you want to add it to the PR, or do you prefer I add it?
(I was thinking of an asm/disasm of the fixed versions)

@serpilliere serpilliere merged commit 88432b0 into cea-sec:master Mar 21, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants