Re: Re: Re: NOP ATHROW hack defeated !

On 05/01/2011 05:15 PM, Eric Bruneton wrote:
> 01/05/2011 16:16, Rémi Forax wrote:
>> On 05/01/2011 11:09 AM, Eric Bruneton wrote:
>>> 30/04/2011 10:20, Eric Bruneton wrote:
>>>> 29/04/2011 03:04, Rémi Forax wrote:
>>>>> Charles Nutter (a JRuby guy) was able to generate a code that defeat
>>>>> the infamous NOP ... ATHROW hack.
>>>>>
>>>>> see http://asm.ow2.org/doc/developer-guide.html#deadcode
>>>>> for the background.
>>>>>
>>>>> Basically, when a code is dead, not reachable, ASM replaces the
>>>>> offending code by
>>>>> NOP ... ATHROW and generate a dedicated stackmap: []
>>>>> [java/lang/Throwable].
>>>>> But if the unreachable code is in a try/catch, we create a path
>>>>> where no
>>>>> local variable
>>>>> are available anymore. So the split verifier will reject the
>>>>> bytecode if
>>>>> a local variable is used in the exception handler of the try/catch.
>>>>>
>>>>> The program below generates a bytecode which is rejected by the split
>>>>> verifier.
>>>>> [forax@localhost asm4-test]$ java deadcode
>>>>> Exception in thread "main" java.lang.VerifyError: Stack map does not
>>>>> match the one at exception handler 7 in method
>>>>> deadcode.foo(Ljava/lang/Object;)V at offset 3
>>>>> at java.lang.Class.getDeclaredMethods0(Native Method)
>>>>> at java.lang.Class.privateGetDeclaredMethods(Class.java:2440)
>>>>> at java.lang.Class.getMethod0(Class.java:2683)
>>>>> at java.lang.Class.getMethod(Class.java:1618)
>>>>> at sun.launcher.LauncherHelper.getMainMethod(LauncherHelper.java:484)
>>>>> at
>>>>> sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:476)
>>>>>
>>>>>
>>>>> Eric, do you see a solution for that ?
>>>>
>>>> I think we can find which locals to put in the stackmap frame 
>>>> generated
>>>> for a dead block by using the locals from the first frame of the
>>>> innermost exception handler for this block(*) (if any). Using the
>>>> innermost handler is necessary to get the most specific types for 
>>>> these
>>>> locals (the other handlers will necessarily have these types or super
>>>> types of these types in their stackmap frames, since they handle more
>>>> code). Also, since dead blocks are handled after the fix point 
>>>> algorithm
>>>> is finished, we have the final frames for the exception handlers at 
>>>> this
>>>> point.
>>>>
>>>> The only problem is that exception handlers do not have to be properly
>>>> nested (nothing enforces this in the specification, although compilers
>>>> always generate nested handlers because Java try/catch are nested by
>>>> construction). And in this case the "innermost handler" is not well
>>>> defined.
>>>
>>> there is another solution, which is simpler and works in all cases (I
>>> think): remove the exception handlers for deadcode (see attached 
>>> patch).
>>>
>>> Eric
>>
>> It seems the patch was too shy to show up :)
>
> oups! here it is
>
> Eric

I think the null check in removeHandler should be moved when
removeHandler is called.

Also, instead of

   if (i&&  j) {
   } else if (i&&  !j) {
   } else if (!i&&  j) {
   } else { // !i&&  !j

if think that

if (i) {
   if (j) {    // i && j
   } else { // i && !j
   }
} else {
   if (j) {    // !i && j
   } else { // !i && !j
   }
}

generates less code for doing the same thing.
Because && is lazy, the compiler creates two jumps for i && j.

I also wonder if removeHandler cannot be turned to a while
instead of being recursive, anyway, it's for dealing with dead code,
it doesn't have to be fast.

Rémi

-- 
You receive this message as a subscriber of the [email protected] mailing list.
To unsubscribe: mailto:[email protected]
For general help: mailto:[email protected]?subject=help
OW2 mailing lists service home page: http://www.ow2.org/wws