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

[WIP] Opcodemadness #681

Merged
merged 65 commits into from Oct 18, 2019
Merged

[WIP] Opcodemadness #681

merged 65 commits into from Oct 18, 2019

Conversation

reox
Copy link
Member

@reox reox commented Apr 23, 2019

changing and refactoring disassembly

@codecov-io
Copy link

codecov-io commented Apr 23, 2019

Codecov Report

Merging #681 into master will increase coverage by 0.72%.
The diff coverage is 87.82%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #681      +/-   ##
=========================================
+ Coverage   72.77%   73.5%   +0.72%     
=========================================
  Files          51      51              
  Lines       16118   16177      +59     
=========================================
+ Hits        11730   11891     +161     
+ Misses       4388    4286     -102
Impacted Files Coverage Δ
androguard/cli/entry_points.py 87.42% <ø> (+0.59%) ⬆️
androguard/decompiler/dad/instruction.py 88.28% <ø> (-0.11%) ⬇️
androguard/core/bytecodes/dvm.py 64.26% <ø> (+0.83%) ⬆️
androguard/decompiler/dad/decompile.py 51.97% <0%> (ø) ⬆️
androguard/core/androconf.py 66.86% <0%> (ø) ⬆️
androguard/cli/main.py 33.54% <0%> (-0.86%) ⬇️
tests/test_analysis.py 99.53% <100%> (-0.01%) ⬇️
tests/test_types.py 95.23% <100%> (+0.23%) ⬆️
androguard/decompiler/dad/opcode_ins.py 87.24% <100%> (-0.08%) ⬇️
androguard/session.py 55.85% <100%> (+0.47%) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f45d4c2...851fcae. Read the comment docs.

@reox reox changed the title Opcodemadness [WIP] Opcodemadness Apr 24, 2019
@reox reox mentioned this pull request May 14, 2019
Copy link
Contributor

@nlachfr nlachfr left a comment

Choose a reason for hiding this comment

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

I'm wondering if you didn't removed unwillingly some of your previous changes during merge.
For example, the following line

self.OP, self.AA, self.BBBBBBBB = unpack("<BBI", buff[:self.length])

is changed with
self.BBBBBBBB = cm.packer["Hi"].unpack(buff[0:6])
self.OP = i16 & 0xff
self.AA = (i16 >> 8) & 0xff

It's the case with many instructions classes (Instruction21h, Instruction21c, Instruction21s, ...)

@reox
Copy link
Member Author

reox commented May 15, 2019

ah yes.... I'm unsure what the best method is there... I tested my approach on several thousand dex files and found out that many have non zero values in the padding... (for Ins 10t, 20t, 30t). This looks like some sort of parser breaker...

for the others, were I changed a H to BB - what do you think would be the best method? I guess BB is faster but twists your brain a little bit more, because of all the LE conversion going on (but I guess thats acceptable :D)

I think I'll change them back to BB - thanks for reminding me!

@nlachfr
Copy link
Contributor

nlachfr commented May 15, 2019

for the others, were I changed a H to BB - what do you think would be the best method? I guess BB is faster but twists your brain a little bit more, because of all the LE conversion going on (but I guess thats acceptable :D)

Maybe keep the one which needs less operations after ? I don't know if such changes in formaters would affect performances

@reox
Copy link
Member Author

reox commented Oct 3, 2019

Well... I found some issues with the interpretation of some opcodes and BOOM you crash the decompiler -.- So there is much much more to do unfortunately...

@reox
Copy link
Member Author

reox commented Oct 17, 2019

For some reason the build fails now but I do not see why... I have not changed anything there and on my machines the called command has an exit code == 0
@MartinThoma do you see anything obvious here? See the travis log https://travis-ci.org/androguard/androguard/jobs/599019570

maybe the problem fixes itself with the next commit, dunno...

Sebastian Bachmann added 7 commits October 17, 2019 10:47
Dropped MethodClassAnalysis and pulled all functions
into MethodAnalysis.
This should also be slightly faster when creating xrefs,
as the method can not be looked up directly instead of using
an exhaustive search.

resolves androguard#412
@reox
Copy link
Member Author

reox commented Oct 18, 2019

@MartinThoma stupid me... I actually changed a lot regarding the callgraph creation! Found the issue and I'm fixing it right now... (I got confused because there were no error messages indicating any fault)

@reox reox merged commit eef9735 into androguard:master Oct 18, 2019
@MartinThoma
Copy link
Contributor

Ok, nice that you found it :-)

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

5 participants