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

extract extra offset/number features #977

Merged
merged 11 commits into from
Apr 7, 2022
Merged

extract extra offset/number features #977

merged 11 commits into from
Apr 7, 2022

Conversation

williballenthin
Copy link
Collaborator

@williballenthin williballenthin commented Apr 6, 2022

closes #320
closes #925

TODO:

  • update rules

Checklist

  • No CHANGELOG update needed
  • No new tests needed
  • No documentation update needed

@williballenthin williballenthin added the enhancement New feature or request label Apr 6, 2022
@williballenthin williballenthin added this to the 4.0.0 milestone Apr 6, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed

@github-actions github-actions bot dismissed their stale review April 6, 2022 20:59

CHANGELOG updated or no update needed, thanks! 😄

@williballenthin
Copy link
Collaborator Author

tests pass on IDA:

image
image

@williballenthin
Copy link
Collaborator Author

ready for review and merge

Copy link
Collaborator

@mr-tz mr-tz left a comment

Choose a reason for hiding this comment

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

awesome, this is another divergence from SMDA and further motivates #937 IMO

@@ -29,6 +29,10 @@ def get_value_str(self):
return capa.render.utils.hex(self.value)


# max recognized structure size (and therefore, offset size)
MAX_STRUCTURE_SIZE = 0x10000
Copy link
Collaborator

Choose a reason for hiding this comment

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

I find this pretty big. How did you pick this value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i wanted a round number large than structures/arrays i commonly work with. since 0x1000 is PAGE_SIZE, the next choice was 0x10000. happy to change if necessary.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, makes sense and I'm fine keeping it as is

tests/fixtures.py Show resolved Hide resolved
@williballenthin williballenthin merged commit b455b67 into master Apr 7, 2022
@williballenthin williballenthin deleted the feature-320 branch April 7, 2022 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants