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

tracers: fix err in 4byte, add some opcode analysis tools #16954

Merged
merged 1 commit into from
Jun 20, 2018

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Jun 12, 2018

This PR

  • Fixes an error in the tracer 4byteTracer, which failed to check if a call destination was a precompile correctly, thus generating a lot of false positives.
  • Adds a opcodeHistogram which just counts opcodes.
  • Adds a bigram which counts bigrams
  • Adds trigram which counts trigrams.

Although these are probably not useful in any general sense, having more examples that people can look at and base their own tracers from has a value too, thus I included the tracers in this PR.

@holiman holiman requested a review from karalabe as a code owner June 12, 2018 08:35
@karalabe
Copy link
Member

karalabe commented Jun 14, 2018

Nitpick, but could you give an example of the 3 tracer results here in the description? Since we suck at documentation, would be nice to have some minor trace at least here of what these do :)

@karalabe karalabe added this to the 1.8.12 milestone Jun 14, 2018
Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

Please name all tracers with a _tracer suffix. Otherwise you have callTracer and bigrams and opcountTracer and trigrams. Lets keep all tracers uniform.

@karalabe
Copy link
Member

Also, please rename opcodeHistorgram to unigram (if you introduced bigram and trigram too, lets keep them consistent).

@karalabe
Copy link
Member

Alternatively maybe we could do ngram + introduce some parametrization?

@karalabe
Copy link
Member

Fixed up the tracer names to unigramTracer, bigramTracer and trigramTracer.

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants