Storyyeller / Krakatau

Java decompiler, assembler, and disassembler
GNU General Public License v3.0
1.95k stars 219 forks source link

Incorrect code while decompiling #108

Open samczsun opened 7 years ago

samczsun commented 7 years ago
.version 49 0 
.class public super Test 
.super java/lang/Object 

.method public static main : ([Ljava/lang/String;)V 
    .code stack 10 locals 10 
        .catch java/lang/NoClassDefFoundError from L0 to L3 using L4 
L0:     ldc Class abc 
L2:     pop 
L3:     return 
L4:     getstatic Field java/lang/System out Ljava/io/PrintStream; 
L7:     ldc 'Hacked' 
L9:     invokevirtual Method java/io/PrintStream println (Ljava/lang/Object;)V 
L12:    return 
L13:    
    .end code 
.end method 
.end class 

The following program causes the following code to be decompiled if -xmagicthrow is enabled:

public class Test {
    public static void main(String[] a)
    {
        try
        {
        }
        catch(NoClassDefFoundError ignoredException)
        {
            System.out.println((Object)"Hacked");
            return;
        }
    }
}

and this if not:

public class Test {
    public static void main(String[] a)
    {
    }
}

I remember you had reservations about handling this sort of throwable-based edge case, but I think -xmagicthrow was supposed to be a "this should handle them all", which is why I'm reporting this particular instance

Storyyeller commented 7 years ago

So is the problem that the try block is empty? Because the code itself didn't disappear. I'm not sure if there's a meaningful way to represent an invalid ldc throwing at the Java level.

samczsun commented 7 years ago

I manipulated this particular example to show worst-case. Without the pop Krakatau places the asd.class after the try/catch. Moreover, nearly every other decompiler places the statement inside the try/catch so I assume it's a logical error somewhere within the optimization process

Storyyeller commented 7 years ago

It's not a logical error, just a side effect of the way xmagicthrow is implemented. It doesn't actually stop the optimizer from pruning normal instructions, it just inserts an extra instruction into the IR after every (non jump) bytecode operation which is treated as being able to throw anything. Thus ldc is still assumed to not throw and optimized out as applicable, but the try/catch is maintained.

samczsun commented 7 years ago

I see. Would it be possible to fix this bug? I don't know much of how Krakatau works but I fear that due to the small amount of bytecode required people will happily integrate it into whatever obfuscator they're spinning up and then the one decompiler that actually works decently doesn't.

Storyyeller commented 7 years ago

It's a hard problem and I'm not sure what the right answer is.

samczsun commented 7 years ago

Could there be an option to disable optimizations? I'm not sure how you've designed the framework so that may not be very easy, but sometimes disabling optimizations may be a good thing (E.g. if I insert many many many reducible blocks and force krakatau to try and optimize all of them, then it'll just hang and produce no meaningful output)

Storyyeller commented 7 years ago

The thing is that for normal code, you want the optimizations, because doing otherwise leads to tons of unnecessary clutter in the output.

There's also the separate issue that prior to the addition of EBBs, throw optimization was necessary to get reasonable performance. But that shouldn't be true now, so it is worth revisiting. Still, it would take a fair bit of work to implement and make sure it works, and it seems like it isn't very important.

samczsun commented 7 years ago

Hmm... There really doesn't seem to be a good solution to this.

I suppose a cheap solution would be to just define ldc as capable of throwing an exception. Technically if I was to compile a class file with dependencies that aren't available at runtime I would achieve the same bytecode.

Or maybe it's possible to modify whatever internal structure which stores exceptions thrown by instructions to change ldc if -xmagicthrow is enabled?