Storyyeller / Krakatau

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

Everything is being casted #61

Closed samczsun closed 8 years ago

samczsun commented 8 years ago

In the latest release it appears that everything is casted to the class that the invoke instruction specifies. It makes everything a bit of a pain to read as this:

getServer().getPluginManager().registerEvents(this, this);

Is turned into this:

((org.bukkit.Server)((org.bukkit.plugin.java.JavaPlugin)this).getServer()).getPluginManager().registerEvents((org.bukkit.event.Listener)(Object)this, (org.bukkit.plugin.Plugin)(Object)this);
Storyyeller commented 8 years ago

That's probably a result of assuming any class which can't be loaded is an interface. Have you tried specifying the jar with said classes via -path?

In general, Krakatau wasn't designed to handle types which it doesn't have access to. Which is a problem for usability, but it's hard to change.

samczsun commented 8 years ago

I did include all the relevant classes in the classpath. Here's what it looks like when I don't include the relevant classes.

    //ClassLoaderError: 
    //ClassNotFoundException: org/bukkit/plugin/java/JavaPlugin
    public void onEnable();
Storyyeller commented 8 years ago

That's odd. Does this happen with every application or just specific ones?

samczsun commented 8 years ago

I compared between two different classes that do the same thing. One has an extraneous goto inserted.

The first one looks like this disassembled:

L4:     aload_0 
L5:     invokevirtual Method org/bukkit/plugin/java/JavaPlugin getServer ()Lorg/bukkit/Server; 
L8:     checkcast org/bukkit/Server 
L11:    goto L14 
L14:    invokeinterface InterfaceMethod org/bukkit/Server getPluginManager ()Lorg/bukkit/plugin/PluginManager; 1

And results in this code

((org.bukkit.Server)((org.bukkit.plugin.java.JavaPlugin)this).getServer()).getPluginManager()

The second one looks like this disassembled

L3:     aload_0 
L4:     invokevirtual Method class/that/extends/JavaPlugin getServer ()Lorg/bukkit/Server; 
L7:     invokeinterface InterfaceMethod org/bukkit/Server getPluginManager ()Lorg/bukkit/plugin/PluginManager; 1 

And results in this code:

this.getServer().getPluginManager()

I'm assuming it's likely the goto, but could also be the fact that the first one references the exact class that implements the method getServer() (org/bukkit/plugin/java/JavaPlugin) vs the second one which relies on dynamic dispatch

Storyyeller commented 8 years ago

That looks like how it's supposed to work, actually.

In the first example, this doesn't have the type org/bukkit/plugin/java/JavaPlugin, so Krakatau adds an explicit cast to be sure. Then the second cast is due to the org/bukkit/Server checkcast.

Storyyeller commented 8 years ago

Actually the first cast should probably have been removed since this is presumably assignable to org/bukkit/plugin/java/JavaPlugin. The second cast has to be there either way though.

samczsun commented 8 years ago

Perhaps a few optimizations could be added? For example, the first can be removed if the hierarchy exists and is valid. The second can be removed because the return type specifically states org/bukkit/Server

Storyyeller commented 8 years ago

Actually the second technically can't be removed, because interfaces aren't type checked, and hence getServer can return something that doesn't implement org/bukkit/Server.

samczsun commented 8 years ago

TIL

Fair enough, then

Storyyeller commented 8 years ago

You could reduce the casts generated by setting ALWAYS_CAST_PARAMS = 0 in ast.py