StringDataItem decoding removed #689

nlachfr · 2019-05-01T20:38:33Z

As told in #686, not decoding the strings during parsing could bring a huge performance gain. I did a new class MUTF8String for replacing the results of decode(b) and patch_string(decode(b)) and capable of decoding when needed.

Test with patch_string(decode(b)) :

Runtime : 28.01447200s

              Name               | TotTime  |  Calls   | Source

patch_string                     | 7.291792 |   537811 | mutf8.py:73    
__next__                         | 4.955242 | 16682452 | mutf8.py:57    
decode                           | 3.113555 |   537811 | mutf8.py:1     
read                             | 1.876643 |  5206598 | bytecode.py:795   
<built-in method builtins.ord>   | 1.295802 | 37497133 |
read_null_terminated_string      | 1.071799 |   102793 | dvm.py:97    
<built-in method builtins.chr>   | 0.871598 | 16144642 |
get_byte                         | 0.658616 |  1021791 | dvm.py:200   
<built-in method builtins.len>   | 0.657512 | 17236692 |
get_string                       | 0.512339 |   498603 | dvm.py:7277  
get                              | 0.416270 |   537811 | dvm.py:1904  
<built-in method builtins.isinst | 0.388776 |  5339444 |
__init__                         | 0.388250 |    77741 | dvm.py:6717  
<built-in method _struct.unpack> | 0.256094 |  2400031 |
__init__                         | 0.210497 |    77741 | dvm.py:6483  
readuleb128                      | 0.201282 |   566395 | dvm.py:204   
__init__                         | 0.191760 |   100931 | dvm.py:2421  
<method 'append' of 'bytearray'  | 0.182567 |  2552524 |
<listcomp>                       | 0.182123 |        2 | dvm.py:7028  
__init__                         | 0.181986 |    82413 | dvm.py:2820

Test with MUTF8String(b) :

Runtime : 10.38462600s

              Name               | TotTime  |  Calls   | Source

read                             | 1.849554 |  5206598 | bytecode.py:795   
read_null_terminated_string      | 1.111465 |   102793 | dvm.py:97    
get_string                       | 0.661906 |   498603 | dvm.py:7276  
get_byte                         | 0.621419 |  1021791 | dvm.py:200   
<built-in method builtins.isinst | 0.423100 |  5993425 |
from_bytes                       | 0.414873 |   653981 | mutf8.py:105   
__init__                         | 0.355376 |    77741 | dvm.py:6482  
__init__                         | 0.311230 |   653981 | mutf8.py:93    
__init__                         | 0.302810 |    77741 | dvm.py:6716  
<built-in method _struct.unpack> | 0.245051 |  2400031 |
get_type_ref                     | 0.201302 |   306893 | dvm.py:7342  
readuleb128                      | 0.199724 |   566395 | dvm.py:204   
<built-in method builtins.ord>   | 0.184095 |  5207851 |
<method 'append' of 'bytearray'  | 0.183919 |  2552524 |
add_type_item                    | 0.175883 |       35 | dvm.py:7234  
__init__                         | 0.165159 |   100931 | dvm.py:2420  
get                              | 0.164683 |   537811 | dvm.py:1902  
_load_elements                   | 0.144982 |    44096 | dvm.py:3373  
__init__                         | 0.142053 |     9894 | dvm.py:1456  
get_type                         | 0.126710 |   306893 | dvm.py:7327

It's also possible to optimize read_null_terminated_string as I did in #686.

Resolves : #684

This function regroup functions `decode` and `patch_string`

Moreover, adding size checking inside `decode_and_patch` function

Include `decode` and `encode` functions Warning : `decode` doesn't return a printable string

androguard/core/bytecode.py

Patch concern `read_null_terminated_string` improvement

codecov-io · 2019-05-02T22:04:19Z

Codecov Report

Merging #689 into master will increase coverage by 0.07%.
The diff coverage is 85.91%.

@@            Coverage Diff            @@
##           master    #689      +/-   ##
=========================================
+ Coverage   72.53%   72.6%   +0.07%     
=========================================
  Files          50      50              
  Lines       16057   16188     +131     
=========================================
+ Hits        11647   11754     +107     
- Misses       4410    4434      +24

Impacted Files	Coverage Δ
androguard/decompiler/dad/decompile.py	`51.97% <ø> (ø)`	⬆️
androguard/core/bytecode.py	`43.62% <ø> (-0.23%)`	⬇️
androguard/cli/main.py	`34.39% <100%> (ø)`	⬆️
androguard/core/analysis/analysis.py	`77.15% <100%> (+0.25%)`	⬆️
tests/test_dexcodeparsing.py	`94.87% <100%> (ø)`	⬆️
tests/test_strings.py	`96.29% <100%> (-0.14%)`	⬇️
androguard/decompiler/dad/writer.py	`92.71% <100%> (-3.52%)`	⬇️
androguard/decompiler/decompiler.py	`26.91% <100%> (ø)`	⬆️
tests/test_decompiler.py	`97.5% <100%> (ø)`	⬆️
androguard/core/mutf8.py	`81.3% <81.3%> (ø)`
... and 4 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 55f282b...a39ff67. Read the comment docs.

nlachfr

Changes seems to be ok according to tests.

Test with last updates :

Runtime : 7.62316800s

              Name               | TotTime  |  Calls   | Source

read                             | 0.971308 |  2654552 | bytecode.py:795   
get_string                       | 0.612781 |   498603 | dvm.py:7280  
from_bytes                       | 0.356392 |   653981 | mutf8.py:105   
__init__                         | 0.348029 |    77741 | dvm.py:6486  
get_byte                         | 0.323302 |  1021791 | dvm.py:204   
__init__                         | 0.312980 |   653981 | mutf8.py:93    
__init__                         | 0.303062 |    77741 | dvm.py:6720  
<built-in method builtins.isinst | 0.255098 |  3441379 |
<built-in method _struct.unpack> | 0.241028 |  2400031 |
get_type_ref                     | 0.224705 |   306893 | dvm.py:7346  
readuleb128                      | 0.195157 |   566395 | dvm.py:208   
add_type_item                    | 0.175564 |       35 | dvm.py:7238  
__init__                         | 0.165144 |   100931 | dvm.py:2424  
get                              | 0.164950 |   537811 | dvm.py:1906  
__init__                         | 0.141590 |     9894 | dvm.py:1460  
_load_elements                   | 0.141182 |    44096 | dvm.py:3377  
read_null_terminated_string      | 0.127051 |   102793 | dvm.py:97    
get_type                         | 0.126590 |   306893 | dvm.py:7331  
__init__                         | 0.115519 |    11024 | dvm.py:3258  
__init__                         | 0.111835 |    82413 | dvm.py:2823

It may require to update documentation.

What do you think about moving MUTF8String into androguard.core.bytecodes.dvm ? And maybe renaming it as DalvikString ?

reox · 2019-05-03T09:03:41Z

wow, you have been very busy :)

mh MUTF8 is not only used in Dalvik but in the whole java world... So I would leave it as MUTF8String.
We should consider making the module depth more flat - so maybe move it where it fits? dunno really..

nlachfr · 2019-05-03T11:48:28Z

@reox Maybe put mutf8.py package to androguard.core ?

nlachfr · 2019-05-09T19:43:36Z

I moved the package to androguard.core.mutf8 instead of androguard.core.bytecodes.mutf8.
Do you think it's ready to merge @reox ?

reox · 2019-05-10T07:43:29Z

sure! I pushed some bugfixes, that unfortunately broke your PR, sorry!

you could also merge all your three PRs into a single one. I think it is enough to merge the branches locally and push one - that should automatically close the other two and add them to one PR

reox · 2019-05-10T07:44:04Z

ah I meant your two PRs :D

…arser/androguard#684-1 (PR androguard#689)

Fixing MUTF8String bug when calling __getitem__ with int

nlachfr · 2019-05-10T19:04:46Z

#686 merged and PR is ready to be merged :)

you could also merge all your three PRs into a single one. I think it is enough to merge the branches locally and push one - that should automatically close the other two and add them to one PR

I don't know how it works, #686 is still there

reox · 2019-05-12T09:09:10Z

Mhh last time I did that, the other PR would close... maybe that changed or I did something different :D

anyways, merging these now!

reox · 2019-05-12T09:09:59Z

Ah... Just realized: The other PR was closed automatically when this one was merged!
So I just had wrong memories ;)

nlachfr added 9 commits April 25, 2019 11:42

Adding function for MUTF-8 decoding and patching

2fcc17e

This function regroup functions `decode` and `patch_string`

Replacing path_string calls with decode_and_patch

51f8083

Moreover, adding size checking inside `decode_and_patch` function

Adding mutf8 tests and fixing bugs

26557ef

Improving read_null_terminated_string performance

e0f9625

Removing duplicate object creation

80a26ca

Removing SV and SVs classes

5d9e496

Adding MUTF8String class for dexparser

b0b91ab

Include `decode` and `encode` functions Warning : `decode` doesn't return a printable string

Changing tests for new mutf8 package

1d4531b

Applying MUTF8String to dexparser

6a6f2a9

reox reviewed May 2, 2019

View reviewed changes

androguard/core/bytecode.py Outdated Show resolved Hide resolved

nlachfr mentioned this pull request May 2, 2019

[RDY] Improving DEX parsing time #686

Merged

nlachfr added 4 commits May 2, 2019 12:26

Applying patch of androguard#684 and fixing bugs

8623899

Patch concern `read_null_terminated_string` improvement

Improving MUTF8String methods

2acf45d

Changing DAD strings with MUTF8String equivalents

14d6b0a

Fixing decompiler bugs with MUTF8String

ad1fffe

nlachfr commented May 2, 2019

View reviewed changes

Removing unicode related methods

46be252

nlachfr changed the title ~~[WIP] StringDataItem decoding removed~~ StringDataItem decoding removed May 8, 2019

Changing mutf8 package location

50ccf77

nlachfr added 2 commits May 10, 2019 10:39

Merge branch 'dexparser/androguard#684' (PR androguard#686) into dexp…

c90783c

…arser/androguard#684-1 (PR androguard#689)

Merge branch 'master' into dexparser/androguard#684-1

a39ff67

Fixing MUTF8String bug when calling __getitem__ with int

reox merged commit c5104fb into androguard:master May 12, 2019

nlachfr deleted the dexparser/#684-1 branch May 12, 2019 10:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

StringDataItem decoding removed #689

StringDataItem decoding removed #689

nlachfr commented May 1, 2019

codecov-io commented May 2, 2019 •

edited

nlachfr left a comment

reox commented May 3, 2019

nlachfr commented May 3, 2019

nlachfr commented May 9, 2019

reox commented May 10, 2019

reox commented May 10, 2019

nlachfr commented May 10, 2019

reox commented May 12, 2019

reox commented May 12, 2019

StringDataItem decoding removed #689

StringDataItem decoding removed #689

Conversation

nlachfr commented May 1, 2019

codecov-io commented May 2, 2019 • edited

Codecov Report

nlachfr left a comment

Choose a reason for hiding this comment

reox commented May 3, 2019

nlachfr commented May 3, 2019

nlachfr commented May 9, 2019

reox commented May 10, 2019

reox commented May 10, 2019

nlachfr commented May 10, 2019

reox commented May 12, 2019

reox commented May 12, 2019

codecov-io commented May 2, 2019 •

edited