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

[ARMv6] vcvt instruction makes very un-human readable code #7890

Open
f-raZ0R opened this issue Mar 9, 2025 · 13 comments
Open

[ARMv6] vcvt instruction makes very un-human readable code #7890

f-raZ0R opened this issue Mar 9, 2025 · 13 comments
Assignees
Labels
Feature: Processor/ARM Status: Prioritize This is currently being prioritized Type: Bug Something isn't working

Comments

@f-raZ0R
Copy link

f-raZ0R commented Mar 9, 2025

Probably related to #7377 , or the issue I can't find that had to do with multiple registers being used to store one value that is larger than a single register (an 8 byte length double when registers only hold 4 bytes)

Whenever I'm looking at some code that uses any variation of vcvt (e.g. vcvt.f32.s32), the decompiler ends up spitting out some nonsense involving bitshifts and "VectorSignedToFloat", and it's quite difficult to tell what is actually going on.
It would be nice if the decompiler... didn't do that, and instead did something more... understandable.

Image
Image

I've got 0 clue what this code here is doing, for example, beyond doing Something to calculate a percentage to display? (For context, fVar15 is a Constant that contains 100.0) in_fpscr isn't even a "real variable", it exists solely in vcvt calls and is probably some artifact of the decompiler.

@GhidorahRex
Copy link
Collaborator

in_fpscr is a variable that represents the fpscr register. Here it is specifically looking for the rounding mode which is why it's shifting it by 22 bits and masking it. The in_ prefix specifies that it's an unknown input parameter. Since the fpscr register isn't set in the code anywhere, it doesn't know where it came from.

I admit, it's a bit ugly, but it's still readable. vcvt converts a signed integer to a floating point value. So it is calculating a percentage, as you figured.

@GhidorahRex GhidorahRex added Type: Bug Something isn't working Feature: Processor/ARM Status: Triage Information is being gathered labels Mar 10, 2025
@GhidorahRex GhidorahRex self-assigned this Mar 10, 2025
@GhidorahRex
Copy link
Collaborator

Part of the issue is that we don't have a good default value for fpscr. Looking at ARM documentation, it looks like the reset value of the register is 0x00040000. If you add a line to ARMt_v6.pspec to add this as a edefault value, and restart ghidra, I think it may fix your issue.

    <tracked_set space="ram">
      <set name="spsr" val="0"/>
      <set name="fpscr" val="0x00040000"/>
    </tracked_set>

@f-raZ0R
Copy link
Author

f-raZ0R commented Mar 10, 2025

Image
Not really, all that does is replace in_fpscr with a variable that gets set to 0 at the start of the function, the bitshifts are still very ugly

@f-raZ0R
Copy link
Author

f-raZ0R commented Mar 10, 2025

The variable is a constant of 0 that doesn't get written to anywhere else, but the decompiler never optimizes the bitshifts out and just immediately gives the actual number? That feels weird

@GhidorahRex
Copy link
Collaborator

Image

That's not what I see when I add that in. I would try clearing the bytes and re-disassembling that function maybe.

@f-raZ0R
Copy link
Author

f-raZ0R commented Mar 11, 2025

That didn't seem to fix it, did I not add it in correctly, or otherwise not clear it? I highlighted the entire function, pressed C, then D
Using options like Recreate Function or triggering a re-decompile don't correct it either.
ARMt_v6.pspec.txt

@GhidorahRex
Copy link
Collaborator

You added it correctly. I'm not sure why it is not showing up correctly for you, I'll try to play around with it more

@f-raZ0R
Copy link
Author

f-raZ0R commented Mar 11, 2025

Are you testing this on the latest commit, or in a release build? Was anything done to improve decompilation of bitshifts since the latest release? It's possible that it was already fixed and I just don't have the fix yet.

@GhidorahRex
Copy link
Collaborator

I am testing with the latest build, although I don't believe that should have any impact on it. It's most likely because of the environment. I've taken those bytes in isolation without any surrounding code and placed them in a dummy function in an empty binary I created.

The fact that the ghidra is assigning fpscr to a variable suggests to me that at least at some point it's changing the value of the register somewhere. Probably a vcmp instruction?

@f-raZ0R
Copy link
Author

f-raZ0R commented Mar 11, 2025

i'm looking at other uses of vcvct, and in some cases it's just directly a 0 (when the vcvt happens early on in the function), so i think you're right about some other instruction messing with it.
Trying to figure out what exactly is messing it up is difficult as there's usually quite a lot of code before the offending vcvt instruction.

A pattern I'm noticing is that every function that assigns fpscr to a variable has a vpush instruction near the start of the function, and the ones that don't and correctly simplify the bitshift, don't have a vpush. Could this be related?

@f-raZ0R
Copy link
Author

f-raZ0R commented Mar 11, 2025

Never mind, found a function with a vpush that doesn't mess up the vcvt. Huh

@f-raZ0R
Copy link
Author

f-raZ0R commented Mar 11, 2025

found a short enough function with a broken vcvt decompilation

                             *                          FUNCTION                          *
                             **************************************************************
                             undefined FUN_00425b60()
             undefined         <UNASSIGNED>   <RETURN>
                             FUN_00425b60                                    XREF[1]:     FUN_00105dc4:00105dce(c)  
        00425b60 30 40 2d e9     stmdb      sp!,{r4,r5,lr}
        00425b64 01 50 a0 e1     cpy        r5,r1
        00425b68 f0 20 d1 e1     ldrsh      r2,[r1,#0x0]
        00425b6c e8 10 9f e5     ldr        r1=>DAT_0076f17e,[PTR_DAT_00425c5c]              = 0076f17e
        00425b70 00 40 a0 e1     cpy        r4,r0
        00425b74 8f 81 f3 fa     blx        FUN_001061b8                                     undefined FUN_001061b8()
        00425b78 f2 20 d5 e1     ldrsh      r2,[r5,#0x2]
        00425b7c dc 10 9f e5     ldr        r1=>DAT_0076f180,[PTR_DAT_00425c60]              = 0076f180
        00425b80 02 00 84 e2     add        r0,r4,#0x2
        00425b84 8b 81 f3 fa     blx        FUN_001061b8                                     undefined FUN_001061b8()
        00425b88 d4 00 9f e5     ldr        r0,[PTR_DAT_00425c64]                            = 0076f17c
        00425b8c 00 00 d0 e5     ldrb       r0,[r0,#0x0]=>DAT_0076f17c
        00425b90 00 00 50 e3     cmp        r0,#0x0
        00425b94 05 00 a0 11     cpyne      r0,r5
        00425b98 74 81 f3 1b     blne       FUN_00106170                                     undefined FUN_00106170()
        00425b9c c4 10 9f e5     ldr        r1,[PTR_DAT_00425c68]                            = 007cbe1c
        00425ba0 f0 00 d4 e1     ldrsh      r0,[r4,#0x0]
        00425ba4 f8 20 d1 e1     ldrsh      r2,[r1,#0x8]=>DAT_007cbe24                       = ??
        00425ba8 00 0a d1 ed     vldr.32    s1,[r1]=>DAT_007cbe1c                            = ??
        00425bac 02 00 40 e0     sub        r0,r0,r2
        00425bb0 10 0a 00 ee     vmov       s0,r0
        00425bb4 c0 0a b8 ee     vcvt.f32   s0,s0
        00425bb8 20 0a 20 ee     vmul.f32   s0,s0,s1
        00425bbc c0 0a bd ee     vcvt.s32   s0,s0
        00425bc0 10 0a 10 ee     vmov       r0,s0
        00425bc4 70 00 bf e6     sxth       r0,r0
        00425bc8 b0 00 c4 e1     strh       r0,[r4,#0x0]
        00425bcc f2 20 d4 e1     ldrsh      r2,[r4,#0x2]
        00425bd0 fa 30 d1 e1     ldrsh      r3,[r1,#0xa]=>DAT_007cbe24+2
        00425bd4 01 0a d1 ed     vldr.32    s1,[r1,#0x4]=>DAT_007cbe20                       = ??
        00425bd8 00 00 50 e3     cmp        r0,#0x0
        00425bdc 03 10 42 e0     sub        r1,r2,r3
        00425be0 10 1a 00 ee     vmov       s0,r1
        00425be4 00 20 60 b2     rsblt      r2,r0,#0x0
        00425be8 00 20 a0 a1     cpyge      r2,r0
        00425bec 72 20 bf e6     sxth       r2,r2
        00425bf0 07 30 02 e2     and        r3,r2,#0x7
        00425bf4 c0 0a b8 ee     vcvt.f32   s0,s0
        00425bf8 03 00 53 e3     cmp        r3,#0x3
        00425bfc 01 30 a0 23     movcs      r3,#0x1
        00425c00 00 30 a0 33     movcc      r3,#0x0
        00425c04 00 00 50 e3     cmp        r0,#0x0
        00425c08 c2 01 83 e0     add        r0,r3,r2, asr #0x3
        00425c0c 00 00 60 b2     rsblt      r0,r0,#0x0
        00425c10 20 0a 20 ee     vmul.f32   s0,s0,s1
        00425c14 c0 0a bd ee     vcvt.s32   s0,s0
        00425c18 10 1a 10 ee     vmov       r1,s0
        00425c1c 71 10 bf e6     sxth       r1,r1
        00425c20 b2 10 c4 e1     strh       r1,[r4,#0x2]
        00425c24 00 00 51 e3     cmp        r1,#0x0
        00425c28 b0 00 c4 e1     strh       r0,[r4,#0x0]
        00425c2c 00 00 61 b2     rsblt      r0,r1,#0x0
        00425c30 01 00 a0 a1     cpyge      r0,r1
        00425c34 70 00 bf e6     sxth       r0,r0
        00425c38 07 20 00 e2     and        r2,r0,#0x7
        00425c3c 03 00 52 e3     cmp        r2,#0x3
        00425c40 01 20 a0 23     movcs      r2,#0x1
        00425c44 00 20 a0 33     movcc      r2,#0x0
        00425c48 00 00 51 e3     cmp        r1,#0x0
        00425c4c c0 01 82 e0     add        r0,r2,r0, asr #0x3
        00425c50 00 00 60 b2     rsblt      r0,r0,#0x0
        00425c54 b2 00 c4 e1     strh       r0,[r4,#0x2]
        00425c58 30 80 bd e8     ldmia      sp!,{r4,r5,pc}

all the vcvts here end up assigning fpscr to a variable. some other functions of similar length don't though, and i'm not really seeing a pattern of what breaks it

@GhidorahRex
Copy link
Collaborator

In that case it's because it can't be sure it wasn't modified in a called function.

It's probably not possible to completely eliminate the shifts, but in general this is why we prefer to use dummy registers for flags and manually pack/unpack the flags in the status register before it is read or after it's written. We do that with the main flags in ARM32, but not with the floating point status register.

@GhidorahRex GhidorahRex added Status: Prioritize This is currently being prioritized and removed Status: Triage Information is being gathered labels Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Processor/ARM Status: Prioritize This is currently being prioritized Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants