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

Add ARM Cortex-M support #651

Closed
wants to merge 1 commit into from
Closed

Add ARM Cortex-M support #651

wants to merge 1 commit into from

Conversation

SWW13
Copy link
Contributor

@SWW13 SWW13 commented May 20, 2021

Add ARM Cortex-M support

Description/Motivation/Screenshots

Add a ARM-M architecture as outlined in #571.

I used ARM-M instead of mARM as the later was turned into uppercase which makes it hard to see the relation to ARM.
There is probably a way to auto detect the arch and don't relay on python set_arch('ARM-M'), but I'm not into the gef code base.

This is more or less my workaround and in the "works for me"-state. I'm happy to help test further improvements, but I don't want to spend to much time polishing this.

image

How Has This Been Tested?

Architecture Yes/No Comments
x86-32 ✖️
x86-64 ✖️
ARM ✖️
AARCH64 ✖️
MIPS ✖️
POWERPC ✖️
SPARC ✖️
RISC-V ✖️
make tests ✖️

Checklist

  • My PR was done against the dev branch, not master.
  • My code follows the code style of this project.
  • My change includes a change to the documentation, if required.
  • My change adds tests as appropriate.
  • I have read and agree to the CONTRIBUTING document.

Copy link

@jerome-pouiller jerome-pouiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the fixes suggested in my comments and after run python set_arch('ARM-M'), this patch works.

gef.py Show resolved Hide resolved
gef.py Show resolved Hide resolved
@Grazfather
Copy link
Collaborator

This might be where the name of the register is defined for the BMP implementation of the gdbserver: https://github.com/blacksphere/blackmagic/blob/87acd99fe4c3b75df1423279c6902d1709202a06/src/target/cortexm.c#L151

@Grazfather
Copy link
Collaborator

This looks good. "Good enough", though I wish we could more elegantly handle both detection and the _PSR register.

We should probably document this somewhere.

@hugsy do you think we should just mention this pi set_arch(...) in the read me, or add a section to the generated docs?

@hugsy
Copy link
Owner

hugsy commented Jun 7, 2021

@hugsy do you think we should just mention this pi set_arch(...) in the read me, or add a section to the generated docs?

That's a good idea. Maybe expose it in the API documentation ("Extending GEF").

@ikcalB
Copy link

ikcalB commented Jun 14, 2021

When trying this branch, somehowthe context display got messed up?

The following settings are needed, so a context is even displayed:

.gef.rc

[context]
clear_screen = False

[gef]
debug = True

Also the colors are gone, when using tmux-setup

@hugsy
Copy link
Owner

hugsy commented Jan 21, 2022

Due to the recent changes from #779, this new arch can be integrated as part of gef-extras. @SWW13 can you move this PR there instead (in gef-extras/archs) ? It should work out of the box with just the ARM_M class prefixed with the decorator

@register_architecture
class ARM_M(ARM):
    aliases = ("ARM-M",)
    [... rest of the class...]

Cheers

@hugsy
Copy link
Owner

hugsy commented Jan 21, 2022

Closing since hugsy/gef-extras#45 is merged

@hugsy hugsy closed this Jan 21, 2022
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.

5 participants