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

Incorrect code while decompiling #108

Open
samczsun opened this issue Dec 29, 2016 · 8 comments
Open

Incorrect code while decompiling #108

samczsun opened this issue Dec 29, 2016 · 8 comments

Comments

@samczsun
Copy link

.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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

@samczsun
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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?

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

No branches or pull requests

2 participants