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

precompute regex for all loaded signatures #4

Merged
merged 1 commit into from Aug 21, 2018

Conversation

iximeow
Copy link
Contributor

@iximeow iximeow commented Aug 10, 2018

dunno if rebuilding the signature regex each match_bytes was intentional but creating and compiling the regex is a huge amount of runtime, at least using a crosscompiled-to-windows xori.exe as my test file. Since signatures afaict is immutable everywhere other than init and load_flirts, I believe this is a functionally equivalent change, but I don't have any custom pattern files or many other binaries to test on at the moment.

After this change, diff before after shows no differences in output, but completes in ~2.4 seconds under perf, where it was closer to 10 seconds under perf before.

Tripped across this while looking into something else that ended up not being an improvement, whoops.

@rseymour
Copy link
Contributor

definitely an easy and appreciated win, i'll test it out. I wrote it expecting it would be called once, but it gets called all the time. thanks.

one thing we need to do those is get our contributor license agreement jazz set (if it's necessary) and friendly enough that folks keep contributing. I'm going to hold off on merging until we can get that going, which might not be until Monday or Tuesday. Let me know if you have any concerns, etc. I hate open source license legal hoops but luckily my boss said we can keep calling him a jerk for forcing us to AGPL this. 💀 😆

Also the author of nom has tweeted interest in doing something better than byte regex w/ something like YARA support (but for function sigs.) I'd like to benchmark regexes (compiled once) and his solution which would likely be more straightforward. Thank you so much for your contributions already! Rock on.

@rseymour
Copy link
Contributor

ok the CLA is up. Let me know if you have any issues, questions, etc.

@rseymour
Copy link
Contributor

CLA signed so I think we're good to go here. :)

@rseymour rseymour merged commit d5dcca5 into endgameinc:master Aug 21, 2018
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.

None yet

2 participants