pombreda / smali

Automatically exported from code.google.com/p/smali
0 stars 0 forks source link

baksmali doesn't handle certain cases with try/catch blocks correctly when deodexing unresolvable instructions. #33

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
There are 2 main issues - the first is that if all the code within a try block 
is commented out due to an unresolvable instruction, baksmali should remove the 
try block (but it doesn't currently). An alternate fix might be to introduce a 
single nop within the try block in these cases.

The second issue is that in some cases baksmali will replace an unresolvable 
instruction with an invoke, and the following dead code extends up to an 
exception handler, and the goto/jump/branch just before the exception handler 
is commented out, causing dalvik to think that execution could continue on from 
the replaced invoke instruction directly to the move-exception instruction, 
which is illegal. Always replacing these instructions with a throw instead 
should fix this issue, by preventing dalvik from thinking that execution could 
continue past the replaced instruction.

This will be fixed in the next release :)

Original issue reported on code.google.com by JesusFr...@gmail.com on 10 Jun 2010 at 2:34

GoogleCodeExporter commented 9 years ago
Some additional discussion of this is in the comments on issue 29: 
http://code.google.com/p/smali/issues/detail?id=29

Original comment by JesusFr...@gmail.com on 10 Jun 2010 at 2:36

GoogleCodeExporter commented 9 years ago
Keep up the great work JF! :-)

P

Original comment by p...@modaco.com on 10 Jun 2010 at 2:38

GoogleCodeExporter commented 9 years ago
I've committed some experimental changes which should better handle cases like 
these. Please grab a copy of the ExperimentalDeodexChange branch and try it 
out, and let me know how it works for you :)

svn checkout http://smali.googlecode.com/svn/branches/ExperimentalDeodexChange 
smali

Original comment by JesusFr...@gmail.com on 11 Jun 2010 at 5:38

GoogleCodeExporter commented 9 years ago
So far so good... :-)

Original comment by p...@modaco.com on 11 Jun 2010 at 12:54

GoogleCodeExporter commented 9 years ago
Great! :)

I had an epiphany last night, which led to this implementation. Before, I was 
commenting out the dead code after a deodexeable instruction, because there 
were some cases where that following code wouldn't pass dalvik's validation, 
due to the missing register information from the replaced instruction.

But then I realized - dalvik doesn't validate dead code :). By always replacing 
the deodexable instructions with a throw, it renders the following code really 
dead. Before, in the cases where I was replacing them with a different invoke 
instruction, dalvik still thought that execution could continue on past the 
invoke, and so it would procede to validate the following code. Since execution 
can never procede past a throw, dalvik's verifier never reaches the following 
dead code, and I don't have to worry about making it verification safe - I can 
just leave it as-is instead of commenting it out. I just have to replace any 
other dead odex instructions in the following code with a throw as well.

Original comment by JesusFr...@gmail.com on 11 Jun 2010 at 1:20

GoogleCodeExporter commented 9 years ago
Just wanted to let you know that everything is well for me after your latest 
update. Deodexed Froyo is on my n1 with no issues atm. Keep up the good work JF!

Original comment by JrEE...@gmail.com on 12 Jun 2010 at 2:31

GoogleCodeExporter commented 9 years ago
Great! Thanks for trying it out and letting me know Paul and jrEE2kX :)

Original comment by JesusFr...@gmail.com on 12 Jun 2010 at 3:53

GoogleCodeExporter commented 9 years ago
The changes have been merged to trunk, and are included in 1.2.3.

Original comment by JesusFr...@gmail.com on 13 Jun 2010 at 9:03