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

archinfo.arch_arm.is_arm_arch returns False for AArch64 #109

Closed
LukeSerne opened this issue Mar 7, 2022 · 5 comments
Closed

archinfo.arch_arm.is_arm_arch returns False for AArch64 #109

LukeSerne opened this issue Mar 7, 2022 · 5 comments

Comments

@LukeSerne
Copy link

Description

The function archinfo.arch_arm.is_arm_arch returns False for archinfo.ArchAArch64:

>>> import archinfo
>>> archinfo.arch_arm.is_arm_arch(archinfo.ArchAArch64)
False

The implementation of is_arm_arch (also) checks if the name string starts with "AArch". However, archinfo.ArchAArch64.name is "AARCH64". In fact, there is no arch that has a name that starts with "AArch". As such, I assume this is a bug - either in is_arm_arch or in the definition of archinfo.ArchAArch64.name.

Possible fix

A possible fix could be to change the implementation of is_arm_arch slightly to:

def is_arm_arch(a):
    return a.name.startswith('ARM') or a.name.startswith('AARCH')
@rhelmot rhelmot closed this as completed in edfe83d Mar 7, 2022
@rhelmot
Copy link
Member

rhelmot commented Mar 7, 2022

oops!!!!!

I would really rather the aarch64 arch be AArch, but it's too late for that I guess!

@rhelmot rhelmot reopened this Mar 7, 2022
@rhelmot
Copy link
Member

rhelmot commented Mar 7, 2022

It looks like the above commit causes several of our tests to fail. I'm going to revert it until I have time to investigate.

@rhelmot rhelmot closed this as completed in 69a3367 Mar 7, 2022
@rhelmot rhelmot reopened this Mar 7, 2022
@ltfish
Copy link
Member

ltfish commented Mar 7, 2022

Hi all, I believe there is some logic in angr that relies on “AArch64 not being ARM.” I’ve been writing code with the understanding that is_arm_arch() is testing if the architecture is an ARM 32-bit architecture, because, obviously, AArch64 has a lot of differences than ARM 32.

@LukeSerne
Copy link
Author

In that case, it might be better to just change the implementation to the below. It doesn't change functionality, so all tests should be unaffected, but it's not suggesting it returns True for AArch64.

def is_arm_arch(a):
    return a.name.startswith('ARM')

@rhelmot
Copy link
Member

rhelmot commented Apr 20, 2022

I've applied the recommended patch. Thanks!

@rhelmot rhelmot closed this as completed Apr 20, 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

No branches or pull requests

3 participants