md-5 / SpecialSource

Automatic generator and renamer of jar obfuscation mappings.
Other
206 stars 49 forks source link

SS doesn't remap methods implemented in a lambda #36

Closed matthewprenger closed 8 years ago

matthewprenger commented 8 years ago

This problem has come up as more and more mods are moving to Java 8. SpecialSource appears to not fully remap the method names of interfaces and abstract classes when they are implemented using a lambda.

Test class:

public class Test {
    public static void main(String[] args) {
        Foo f = () -> "Matthew";

        System.out.println(f.getName());
    }

    private interface Foo {
        String getName();
    }
}

SRG line:

MD: Test$Foo/getName ()Ljava/lang/String; Test$Foo/getFoo ()Ljava/lang/String;

Error:

Exception in thread "main" java.lang.AbstractMethodError: Test$$Lambda$1/791452441.getFoo()Ljava/lang/String;
    at Test.main(Test.java:5)

Source: MinecraftForge/ForgeGradle#336

md-5 commented 8 years ago

This is verging on impossible to solve without reimplementing half of Java within SS as far as I can tell. You're welcome to look at the bytecode and offer suggestions

matthewprenger commented 8 years ago

@md-5 Figured it out. The key is overriding mapInvokeDynamicMethodName in the ASM Remapper class. This remapper correctly remaps the above test code:

final Remapper remapper = new Remapper() {
    @Override
    public String mapMethodName(String owner, String name, String desc) {
        if ("getName".equals(name)) {
            return "getFoo";
        }
        return name;
    }

    @Override
    public String mapInvokeDynamicMethodName(String name, String desc) {
        if ("getName".equals(name)) {
            return "getFoo";
        }
        return name;
    }
};

Note: the desc parameter of mapInvokeDynamicMethodName in this case is: ()LTest$Foo;

md-5 commented 8 years ago

You still need to evaluate desc to determine the owner which you have not done. I'd also bet there are plenty of other valid invokedynamic constructions that are declared differently.

matthewprenger commented 8 years ago

Yeah I didn't bother evaluating desc for this simple example, but I understand that it's needed.

LexManos commented 8 years ago

Potential solution for this, it has not been excessivly tested as there is no test framework for SS that I can find. Should work for everything if i'm reading the specs on lambdas correctly. However it would be nice to find really complex lambdas to test. https://github.com/LexManos/SpecialSource/commit/ef5ac81fdd55e18a6357ed45c1b6303906906c07

matthewprenger commented 8 years ago

This does seem to fix the above test code.