Remapping java reflect/invoke calls

[liach]

In tiny remapper, as far as I know, currently we do not handle reflect and invokes.

This may be a problem as Minecraft does use reflection and method handles, namely to interact with LWJGL. Hence, if lwjgl for a minecraft client is remapped, the game will fail because of reflection issues.

The remapping IMO should be done as such:

1. Generate stub class and methods as the remapped targets for some java lang invoke/java lang reflect/Class.forName calls
   1. In the stub methods, we update target names passed with the remapping data
   2. When the reflection aims to get java APIs, such as ClassLoader.defineClass, return the stub APIs to ensure correctness
2. In addition, there are other apis that needs care, namely unsafe and the field offset calls afaik (remap the passed field names, too, as they use this to steal fields nowadays)

After these steps are done, the remapping should be good. Please point out other edge cases.

A way to test such remapping may be testing with bukkit plugins like protocollib which uses bytebuddy (that reflects to get ClassLoader.defineClass). Other plugins using reflection may be good targets to test against as well.

[LogicFan]

I will argue this is a complex work. I think we would have a complete list of what method can be used for reflection first before any further discussion.

[liach]

The main ones that I found by looking at jdk javadocs:

• java.lang.reflect.*: actually none, as the reflections are done by giving names to the Class methods
• java.lang.Class: forName varieties, getDeclaredMethod getDeclaredField getMethod getField
• java.lang.ClassLoader: most methods, including protected and potentially private ones (for they may do reflection)
• java.lang.invoke.*:
  • ConstantBootstraps: getStaticFinal invoke staticFieldVarHandle (may handle them with constant invokedynamic/dynamic instead)
  • MethodHandles$Lookup: bind defineClass defineHiddenClass defineHiddenClassWithClassData findXxx methods
  • MethodType: fromMethodDescriptorString​

For unsafe, only defineAnonymousClass needs patching; if other patches are done right, the code would pass to unsafe with safe fields/methods for offsets and things should be thus fine.

Another category of classes worthy of notice is the ConstantDesc ones, where there are a few apis for building constant models from raw descriptors.

The strategy for looking up is to find class definition and string-taking methods and evaluate them.

[liach]

@sfPlayer1 How is the feasibility of this? I think we might include another flag for the remapper builder (and a corresponding command line flag) for such remapping.

[sfPlayer1]

This would be a reasonable extension I think, could even go as far as handling dynamic reflection by injecting some remapping runtime bits.

Static reflection remapping needs some analysis to associate type and string constants (LDC) with the actual reflection lookup method (Class.forName etc). Even some simple bytecode shapes that are easy to cover already go a long way in making reflection convenient.

The class defining ones aren't particularly important. The Class.forName overloads, Class.get*Field, Class.get*Method, some of the corresponding Lookup methods and Atomic*FieldUpdater should suffice.

[LogicFan]

Just to clarify the goal for this. Do you want to "remap" all the reflections including

void reflect_helper(Stirng className) {
  Class.forName(className)
}

or just string literal one like this

Class.forName("bla/bla/MyClass");

but not the first example.

[sfPlayer1]

The first is the dynamic case, which would require insertion of some runtime logic, the second is the static case and can be done directly (Class.forName uses dot-form btw). Supporting both would be nice, starting with only the 2nd is perfectly adequate. Either way, it is an open ended feature request that will have to wait until there's time for it.

[liach]

Yeah, Imo a first step before implementing either cases may be emitting warnings for such encountered class definition/reflection calls and noticing that remapping may not handle these perfectly, which can indicate most of the first case scenarios (unless they depend on some third-party libraries that is outside of remapping). The handling of second case should be easy once bytecode analysis is done.

[LogicFan]

Here's my idea, this extension will contain two parts, one is an extension of tiny-remapper, the second one is a runtime which doing the string literal remap for the runtime.

The tiny-remapper extension will redirect all method calls mentioned above to a static method in the runtime package, and let the runtime do the actual remapping. It will be the user's responsibility to add a runtime jar & correct mapping file to the classpath (which will be integrated into the loom).

Some questions remain, what if a jar is remapped multiple times, then the loom will need to merge the current mapping file & the mapping file in the jar. This also means the jar needs to distribute with a mapping file in jar to have the runtime remapping capiblity.

[sfPlayer1]

I'd focus on statically determinable reflection remapping first, the dynamic part poses some legal challenged when proprietary mappings like mojang's are used and bundled with the jar (which is also what'd make it easiest to handle..).