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

[Decompiler][Function Editor] Custom storage XMM registers are changed automatically by Ghidra to incorrect registers #7427

Open
CebbyS opened this issue Jan 30, 2025 · 12 comments
Assignees
Labels
Status: Triage Information is being gathered Status: Waiting on customer Waiting for customer feedback

Comments

@CebbyS
Copy link

CebbyS commented Jan 30, 2025

Describe the bug
Ghidra does not automatically detect function arguments when they are passed through XMM registers. Such parameters have to be manually defined enabling custom storage for function definition. Example - doubles and floats can be passed through the XMM registers.

When the custom storage option is enabled and custom arguments are added with the storage pointing to XMM{N} registers, strange issue occurs - after performing local variable name changes, commiting parameters, performing undo operation (not immediately after changing function definition, but later). All XMM registers change with the following pattern:

  • XMM1 → XMM2
  • XMM2 → XMM4
  • XMM3 → XMM6
  • XMM4 → XMM8

It indicates that there is a bug for the register indexing/mapping

Steps to reproduce

  1. Enable custom storage for function using XMM registers as parameters
  2. Add the missing parameter to the function definition.
  3. Set the parameter storage to XMM1_Qa register with datatype double
  4. Save the function definition and close the Function Editor window
  5. Rename of any local function variable (Just to be able to perform Undo).
  6. Right click in the decompiler window and then click Commit Local Parameter Names
  7. Perform [Ctrl+Z] Undo Operation
  8. The previously custom defined parameter is now changed into XMM2_Qa register with datatype double

Expected behavior
XMM registers should not be changed by when specified for function with custom storage enabled

Screenshots
How the function definition should look like (Uses the XMM1_Qa register)
Image

How it is retyped after performing operations (Changed to the XMM2_Qa register)
Image

Environment (please complete the following information):

  • OS: Windows 11 Pro - version: 23H2 - build: 22631.4751
  • Java Version: Amazon Corretto JDK 21.0.5_11
  • Ghidra Version: 11.2.1
  • Ghidra Origin: Official GitHub Distro
@ghidra1
Copy link
Collaborator

ghidra1 commented Feb 4, 2025

@CebbyS , I am very confused by your problem scenario description. Could you please provide a step-by-step procedure for reproducing the problem you are trying to describe.

@CebbyS
Copy link
Author

CebbyS commented Feb 4, 2025

Hi, @ghidra1!

Sorry, just now noticed the message. I will update the description for step by step to reproduce the issue I am having

@ryanmkurtz ryanmkurtz added the Status: Waiting on customer Waiting for customer feedback label Feb 4, 2025
@CebbyS
Copy link
Author

CebbyS commented Feb 5, 2025

So I chaned the description and added the steps:

Steps to reproduce

  • Enable custom storage for function using XMM registers as parameters
  • Add the missing parameter to the function definition.
  • Set the parameter storage to XMM1_Qa register with datatype double
  • Save the function definition and close the Function Editor window
  • Rename of any local function variable (Just to be able to perform Undo).
  • Right click in the decompiler window and then click Commit Local Parameter Names
  • Perform [Ctrl+Z] Undo Operation
  • The previously custom defined parameter is now changed into XMM2_Qa register with datatype double

@ghidra1
Copy link
Collaborator

ghidra1 commented Feb 6, 2025

@CebbyS ,You would have produced multiple change transactions within the decompiler:

  1. Rename of local variable
  2. "Commit Local Names" (your description did not match an action name)

Did you try to undo both of these transactions? You only did a single undo.

I suggest watching the param storage in the assembly listing for the function to see which of the two actions caused the storage to change since I was unable to trigger the storage change. If it was the param rename - you did not undo that first transaction. A local register variable declared at the function entry point (i.e., first use offset) will conflict with a parameter using the same storage.

I would be curious to know what param storage changes are observed for the function in the assembly listing param/variable listing. Specifically, what param changes are observed when the local variable is renamed, and what changes are observed when the second commit action is performed.

@CebbyS
Copy link
Author

CebbyS commented Feb 6, 2025

So what I did here:

  • Started with parameter with storage of XMM4_Qa register
  • Changed the parameter to use XMM1_Qa register
  • Renamed variable to use different name (In real case scenario, lets say I want to revert back the change)
  • I press Ctrl+Z to undo the rename action

What happened:

  • Started with parameter with storage of XMM4_Qa register
  • Changed to XMM1_Qa register and did not save anything
  • Renamed variable
  • Reverted variable change with Ctrl+Z
  • Storage for parameter was changed from XMM1_Qa to XMM2_Qa (It should have been XMM4_Qa if the action would undo the change)

Image

Sometimes, if I save the project with the changes performed - function parameter set to use XMM1_Qa register, close and reopen the project, the parameter once again is changed to use XMM2_Qa register. I cannot find the exact trigger point, I suspected the synchronization with the database, but I don't have a clue how to debug this problem.

What I can tell, is that if I only work on this single function, without moving into another function, Ctrl+Z does not cause the issue anymore. And closing/reopening project either.

@ghidra1
Copy link
Collaborator

ghidra1 commented Feb 6, 2025

While you are doing this in the decompiler I am most interested in seeing all the listed variables in the assembly listing rather than the operand field. I was hoping to see how the param storage was affected with each action. Still images of this block may help:

  1. Before Function Edit
  2. After custom storage edit with Function Editor
  3. After rename of local hash variable
  4. After one undo
  5. After second undo

Unfortunately, the param storage display is off the top of the assembly listing. I wish I could pause your animation which is quite useful but everything happens so fast - my eyes can't see all the areas at once to see what is changing. I think I will need to see it first hand some how to troubleshoot. This is what I see/assume:

  1. Set custom storage on "mScale" register param via Function Editor from XMM4_Qa to XMM1_Qa
  2. Rename "mSeconds" local hash variable to "seconds" (no storage changes can be seen)

After these two changes you should have two things on the undo stack. Clicking the little down-arrow on the undo button should provide a general description of the transactions. Before doing your Ctrl-Z, could you verify on the Undo button what the undo descriptions are. The first undo should just be reverting the rename of the local hash variable and not affect the parameters in any way. I can't tell what is happening after the undo in your animation. Could you try simply doing an undo and redo after using the Function Editor without the local variable rename and see how that behaves.

@ghidra1
Copy link
Collaborator

ghidra1 commented Feb 6, 2025

It would appear in my trying this that an Auto Analysis transaction is added in between the Rename Local Variable and the Edit Function transactions. It is possible that the auto-analysis made its own change if this is your case. You should check the parameter storage in the assembly listing after each step to see when it changes.

@CebbyS
Copy link
Author

CebbyS commented Feb 6, 2025

Thank you very much for all the guidance. Hopefully the screenshots I will provide here, will have all the information necessary.

  • Before Function Edit:
    Image
  • After Custom Storage Edit:
    Image
  • Rename Of Hash Variable:
    Image
  • Undo Options:
    Image
  • Pressing Undo again updates the XMM1_Qa to XMM2_Qa
    Image
    Image
  • Pressing Redo does not update the XMM2_Qa back to XMM1_Qa
  • Changes after Undo action:
    Image

I just noticed that there is this undefined8 parameter with the same space, but removing it and performing the same steps produce the same results.
Image

@ghidra1
Copy link
Collaborator

ghidra1 commented Feb 6, 2025

The key to the problem is the interaction between the in_XMM1_Qa local variable and the parameter with the same storage. When you first started both the custom param storage and the local variable occupied the same storage. Was this initial state using custom storage for the parameter?

Any chance you can copy all the bytes for the function (Copy Special... on selected function body -> Byte String). It may help to reproduce the situation - although unclear if it will be enough.

@ghidra1 ghidra1 self-assigned this Feb 6, 2025
@ghidra1 ghidra1 added the Status: Triage Information is being gathered label Feb 6, 2025
@CebbyS
Copy link
Author

CebbyS commented Feb 9, 2025

Here is the byte string of the function:
register-timer.txt

And I assume yes, it was the initial state, when the parameter occupied the same storage. As when the function originally was decompiled, XMM0 was considered to be local param with the prefix in_XMM0_... (Do not recall whether it was W, DW, QW)

I have this project for ~4 years, so I have been working on it through several Ghidra versions, if it changes anything about the database, the way how the data/models/entities were stored then and now.

@ghidra1
Copy link
Collaborator

ghidra1 commented Feb 11, 2025

@CebbyS , I can't click on any of your images anymore to view them - not sure why the fullsize got removed from GitHub. Kinda complicates things.

@ghidra1
Copy link
Collaborator

ghidra1 commented Feb 11, 2025

I can't seem to get your exact situation to occur using your bytes. Is your binary x86 32 or 64-bit? Which compiler spec ID? I tried both but the decompilation differs too much.

What happens if you delete and re-create the Function with the latest version of Ghidra? Doing this are you able to reproduce the problem? I also tried with 11.2.1 with no luck. I also puzzled why your Edit Function did not trigger an Auto Analysis transaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage Information is being gathered Status: Waiting on customer Waiting for customer feedback
Projects
None yet
Development

No branches or pull requests

3 participants