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

The definition and use of [UC_ARM_REG_OTHER_SP, UC_ARM_REG_SPSEL, UC_ARM_REG_CURR_SP_MODE_IS_PSP] registers are unnecessary. #3

Open
shandianchengzi opened this issue Jan 2, 2023 · 2 comments · May be fixed by #4

Comments

@shandianchengzi
Copy link

Dear team,

I wanted to bring to your attention an issue I encountered while studying your code. I noticed that you have defined three additional registers that seem to have the same function as those in the original unicorn. However, these registers do not conform to ARM register definitions.

After further investigation, I realized that these registers are only used for stack swapping during interrupts. In order to properly change the stack during ARM interrupts, the value of UCARMREGCONTROL [1] should be modified to determine the current process stack being used. This is also mentioned in The Definitive Guide to ARM Cortex M3 and Cortex M4 Processors, as shown in the attached figure.
Snipaste_2023-01-02_19-50-58

I tried making this change in the code and the running results showed that unicorn was able to correctly handle the stack change request. I would like to submit my changes for your review so that you can see the specific location of the modification.

Thank you for your attention to this matter.

Best regards.

@Scepticz
Copy link
Contributor

Scepticz commented Jan 8, 2023

Hello shandianchengzi,

thank you for looking into the details of this. To what extend did you test these changes? Is there any functional difference in how the emulator behaves for specific targets / inputs?

I feel like I will have to look into this a bit more to fully understand these changes. However, it sounds reasonable to me that I just overlooked something which was already present in Unicorn. Getting rid of the extra/custom register definitions would be a great thing to do.

Best
Tobi

@shandianchengzi
Copy link
Author

shandianchengzi commented Jan 9, 2023

Hello shandianchengzi,

thank you for looking into the details of this. To what extend did you test these changes? Is there any functional difference in how the emulator behaves for specific targets / inputs?

I feel like I will have to look into this a bit more to fully understand these changes. However, it sounds reasonable to me that I just overlooked something which was already present in Unicorn. Getting rid of the extra/custom register definitions would be a great thing to do.

Best Tobi

Hi Tobi,

Thank you for your prompt response. I have noticed that your emulator is based on unicorn 1.0.3 and I hope to make it compatible with unicorn 2. Therefore, I have checked the details of your modifications to unicorn and hope to add the modifications directly to unicorn2.

Then I found that the env.v7m.other_sp attribute used in the OTHER_SP register has been used in MSP and PSP registers, and the SPSEL is the second bit of the CONTROL register, not a separate register.

So I'm not sure whether it is necessary to incorporate your new features into unicorn 2, and I took a look at its details. Maybe I don't understand this code well enough. Please forgive me if these registers play a function I didn't notice.

Thank you again for your work on this project.

Best,
shandianchengzi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants