Properly deobfuscate lambda expressions

[Aaron1011]

Tl;dr: While the invokedynamic opcode is properly obfuscated by SpecialSoruce (run during a gradle build when using ForgeGradle), FML's remapper does not reverse this process. This leads to an AbstractMethodError when running an obfuscated jar file in a deobfuscated environment (e.g. an IDE).

Background

Lambdas in Java are implemented through a combination of the invokedynamic opcode introduced in JRE 7, and LambdaFactory#metafactory, introduced with Java 8. Here's a rough outline of the process:

1. The compiler moves the lambda body into a synthetic private method in the same class where it was declared. All parameters for the implemented interface, along with any variables 'captured' from the enclosing scope. For example, in this snippet:

String str = "Foo"
Runnable myRun = () -> System.out.println(str);

str is 'captured' by the lambda. Note that unused variables from the scope will not be 'captured' and passed in as arguments to the synthetic private method.

2. The compiler emits an invokedynamic opcode, which (essentially) will take the name of interface method being implemented (run in the example), and two things, as arguments. The last two 'arguments' aren't really important for obfuscation purposes - they specify the signature of an anonymous class that will be generated at runtime to complete the lambda implementation, as well as the 'bootstrap method' (always LambdaFactory#metafactory for Java) which is responsible for kicking off the whole sequence of dynamic class generation that happens at runtime.
   3, The first time the invokedynamic opcode is encountered at runtime, the 'bootstrap method' is run to generate a CallSite, which is used to actually get a class implementing the target interface (Runnable in this case).

The issue

When lambdas are used to implement an obfuscated interface (e.g. ItemMeshDefinition), the parameters to invokedynamic need to be modified so that the class generated by LambdaMetaFactory will implement the proper interface. This is accomplished in SpecialSoure (which runs during a gradle build in a ForgeGradle project).

However, Forge does not reverse this process in its DeobfuscationTransformer. While Minecraft itself does not currently use any lambdas, this can create a problem when an obfuscated build of a mod is run in a deobfuscated (development) environment. When the mod gets classloaded, Forge will properly remap all normal obfuscated references in it to deobfuscated ones, allowing it to run in an environment with MCP names. However, invokedynamic is not currently remapped during the process. The end result is that Java's LambdaMetaFactory will end up generating a class implementing the proper interface, but will generate a method with the obfuscated name, rather than the deobfuscated one.

To see this issue in action, download this test plugin (source here). It's compiled for Forge 1.10, but the interface it implements stays the same between 1.10 and 1.11 - so it should be possible to run under 1.11 by simply changing the build.gradle. Run its production build in your IDE by placing it in the mods folder in your run directory, and observe the AbstractMethodError on startup.

The fix

As both SpecialSource and FMLRemappingAdapter use the same underlying ASM Remapper, the code is nearly identical. I've removed some redundant calls done in the SpecialSource version , such as calling remapper.mapValue, as this is automatically done in the superclass.

Note: You could make the case that this is something that should be fixed in ASM commons, not Forge. However, it's important to keep in mind that ASM works with Java bytecode in general (e.g. from another JVM langauge), not necessary bytecode generated from Java source code. The invokedynamic instruction is a very general-purpose tool, and generating lambdas is only one small possible use of it. Specifically, the class can define the 'static arguments' to the bootstrap method to be absolutely anything it wants - ASM would need to special-case the particular bootstrap methods used by java, which could technically change at any time.

[williewillus]

You say invokespecial a lot in this post, did you mean invokedynamic?

[Aaron1011]

@williewillus: Oops - I was thinking invokedynamic as I was writing this, but I ended up writing something else. Fixed.

[LexManos]

Right, tho this is only a issue in dev-time as mods reobf to srg names at compile, so don't use runtime. So i'll get around it eventually.
Should of guessed Forge would need it to when I looked into/wrote it for SS... {You don't need the 'taken from SS' Cuz I was the one who wrote it ;) }

[mezz]

Does this PR solve this issue? MinecraftForge/ForgeGradle#336

[Aaron1011]

@mezz: Yes, it does.

[Aaron1011]

{You don't need the 'taken from SS' Cuz I was the one who wrote it ;) }

@LexManos Oh, okay. I just wanted to make it clear that it wasn't my code I was using.

[mezz]

Tested with Railcraft and this fix works as expected.

[LexManos]

As you still haven't updated to remove the comments, and this is for 1.10.2, And it's just a ease of life thing for people in dev, which shouldn't be the case anymore as modders SHOULD be deving for 1.11...

[Aaron1011]

@LexManos: Sorry, I didn't know that you wanted me to remove the comments - I thought you were just pointing it out.

[mezz]

@Aaron1011 I rebased this PR on to 1.11 and it has been merged there #3552

[mezz]

@Aaron1011 can you remove the comments? We'd still like to pull this for 1.10.

[Aaron1011]

@mezz: Ooops - I somehow missed seeing your comment, I've removed the comments as you requested.

[mezz]

Thanks!