Add duplicate class name support to the deobfuscator

This allows two different classes in different modules to be refactored
to the same name - for example, Node in client and unpackclass.

Under the hood, it is implemented by prefixing each class name with the
library name and an exclamation mark (which is highly unlikely to appear
in a class name, as it is invalid Java syntax).

At first, prefixing class names with the library name feels like a bit
of a hack. However, it is much simpler than trying to track libraries
throughout the existing code. Furthermore, it allows us to avoid
forking ASM classes like Remapper.

The Fernflower driver was also changed to deobfuscate each library in
its own context, rather than trying to decompile them all in one go - by
the time classes reach Fernflower, the prefixes have already been
removed and Fernflower can't deal with duplicate class names either.

Signed-off-by: Graham <gpe@openrs2.dev>

asm/src/main/java/dev/openrs2/asm/filter/Glob.kt

@@ -22,7 +22,11 @@ object Glob {
                     continue
                 }
 
-                regex.append("[^/]*")
+                /*
+                 * The deobfuscator uses ! in class names to separate the
+                 * library name from the rest of the package/class name.
+                 */
+                regex.append("[^/!]*")
             }
 
             when (ch) {

decompiler/src/main/java/dev/openrs2/decompiler/DecompileCommand.kt

@@ -9,29 +9,47 @@ fun main(args: Array<String>) = DecompileCommand().main(args)
 class DecompileCommand : CliktCommand(name = "decompile") {
     override fun run() {
         val deobOutput = Paths.get("nonfree/var/cache/deob")
-        val sources = listOf(
-            deobOutput.resolve("runescape_gl.jar"),
-            deobOutput.resolve("jaggl.jar"),
-            deobOutput.resolve("loader_gl.jar"),
-            deobOutput.resolve("signlink_gl.jar"),
-            deobOutput.resolve("unpack_gl.jar"),
-            deobOutput.resolve("unpackclass_gl.jar")
+
+        val client = deobOutput.resolve("runescape_gl.jar")
+        val gl = deobOutput.resolve("jaggl.jar")
+        val loader = deobOutput.resolve("loader_gl.jar")
+        val signlink = deobOutput.resolve("signlink_gl.jar")
+        val unpack = deobOutput.resolve("unpack_gl.jar")
+        val unpackClass = deobOutput.resolve("unpackclass_gl.jar")
+
+        val decompiler = Decompiler(
+            Library(
+                source = client,
+                destination = getDestination("client"),
+                dependencies = listOf(gl, signlink)
+            ),
+            Library(
+                source = gl,
+                destination = getDestination("gl")
+            ),
+            Library(
+                source = loader,
+                destination = getDestination("loader"),
+                dependencies = listOf(signlink, unpack)
+            ),
+            Library(
+                source = signlink,
+                destination = getDestination("signlink")
+            ),
+            Library(
+                source = unpack,
+                destination = getDestination("unpack")
+            ),
+            Library(
+                source = unpackClass,
+                destination = getDestination("unpackclass"),
+                dependencies = listOf(unpack)
+            )
         )
-        Decompiler(sources, ::getDestination).use {
-            it.run()
-        }
+        decompiler.run()
     }
 
-    private fun getDestination(archive: String): Path {
-        var dir = archive.replace(JAR_SUFFIX_REGEX, "")
-        when (dir) {
-            "runescape" -> dir = "client"
-            "jaggl" -> dir = "gl"
-        }
+    private fun getDestination(dir: String): Path {
         return Paths.get("nonfree").resolve(dir).resolve("src/main/java")
     }
-
-    private companion object {
-        private val JAR_SUFFIX_REGEX = Regex("(?:_gl)?[.]jar$")
-    }
 }

decompiler/src/main/java/dev/openrs2/decompiler/Decompiler.kt

@@ -2,25 +2,21 @@ package dev.openrs2.decompiler
 
 import org.jetbrains.java.decompiler.main.Fernflower
 import org.jetbrains.java.decompiler.main.extern.IFernflowerPreferences
-import java.io.Closeable
-import java.nio.file.Path
-
-class Decompiler(
-    private val sources: List<Path>,
-    destination: (String) -> Path
-) : Closeable {
-    private val io = DecompilerIo(destination)
-    private val fernflower = Fernflower(io, io, OPTIONS, Slf4jFernflowerLogger)
 
+class Decompiler(private vararg val libraries: Library) {
     fun run() {
-        for (source in sources) {
-            fernflower.addSource(source.toFile())
-        }
-        fernflower.decompileContext()
-    }
+        for (library in libraries) {
+            DecompilerIo(library.destination).use { io ->
+                val fernflower = Fernflower(io, io, OPTIONS, Slf4jFernflowerLogger)
 
-    override fun close() {
-        io.close()
+                for (dependency in library.dependencies) {
+                    fernflower.addLibrary(dependency.toFile())
+                }
+                fernflower.addSource(library.source.toFile())
+
+                fernflower.decompileContext()
+            }
+        }
     }
 
     private companion object {

decompiler/src/main/java/dev/openrs2/decompiler/DecompilerIo.kt

@@ -8,7 +8,7 @@ import java.nio.file.Path
 import java.util.jar.JarFile
 import java.util.jar.Manifest
 
-class DecompilerIo(private val destination: (String) -> Path) : IBytecodeProvider, IResultSaver, Closeable {
+class DecompilerIo(private val destination: Path) : IBytecodeProvider, IResultSaver, Closeable {
     private val inputJars = mutableMapOf<String, JarFile>()
 
     override fun getBytecode(externalPath: String, internalPath: String?): ByteArray {
@@ -62,7 +62,7 @@ class DecompilerIo(private val destination: (String) -> Path) : IBytecodeProvide
         entryName: String,
         content: String
     ) {
-        val p = destination(archiveName).resolve(entryName)
+        val p = destination.resolve(entryName)
         Files.createDirectories(p.parent)
         Files.newBufferedWriter(p).use {
             it.write(content)

decompiler/src/main/java/dev/openrs2/decompiler/Library.kt

@@ -0,0 +1,9 @@
+package dev.openrs2.decompiler
+
+import java.nio.file.Path
+
+class Library(
+    val source: Path,
+    val destination: Path,
+    val dependencies: List<Path> = emptyList()
+)

deob/src/main/java/dev/openrs2/deob/Deobfuscator.kt

@@ -7,7 +7,8 @@ import dev.openrs2.asm.io.JarLibraryReader
 import dev.openrs2.asm.io.JarLibraryWriter
 import dev.openrs2.asm.io.Pack200LibraryReader
 import dev.openrs2.asm.transform.Transformer
-import dev.openrs2.deob.remap.PrefixRemapper
+import dev.openrs2.deob.remap.ClassNamePrefixRemapper
+import dev.openrs2.deob.remap.StripClassNamePrefixRemapper
 import java.nio.file.Files
 import java.nio.file.Path
 import javax.inject.Inject
@@ -15,7 +16,6 @@ import javax.inject.Singleton
 
 @Singleton
 class Deobfuscator @Inject constructor(
-    private val profile: Profile,
     @DeobfuscatorQualifier private val transformers: Set<@JvmSuppressWildcards Transformer>
 ) {
     fun run(input: Path, output: Path) {
@@ -36,10 +36,47 @@ class Deobfuscator @Inject constructor(
         val unpack = Library("unpack")
         unpack.add(loader.remove("unpack")!!)
 
-        // prefix remaining loader/unpacker classes (to avoid conflicts when we rename in the same classpath as the client)
-        logger.info { "Prefixing loader and unpackclass class names" }
-        loader.remap(PrefixRemapper.create(loader, "loader_", profile.excludedClasses))
-        unpackClass.remap(PrefixRemapper.create(unpackClass, "unpackclass_", profile.excludedClasses))
+        /*
+         * Prefix class names with the name of the library the class
+         * came from (e.g. `a` => `client!a`).
+         *
+         * Using ! as the separator was chosen because it is not valid in Java
+         * source code, so we won't expect to see it in the obfuscator's input.
+         * Furthermore, if any prefixes accidentally remain unstripped, the
+         * problem will be detected quickly as the deobfuscator's output will
+         * not compile. It also mirrors the syntax used in JarURLConnection,
+         * which has a similar purpose.
+         *
+         * In the early parts of the deobfuscation pipeline, this allows us to
+         * disambiguate a small number of classes in the signlink which clash
+         * with classes in the client.
+         *
+         * After name mapping has been performed, it allows us to disambiguate
+         * classes across separate libraries that have been refactored and
+         * given the same name.
+         *
+         * For example, the client and unpackclass both contain many common
+         * classes (e.g. the exception wrapper, linked list/node classes,
+         * bzip2/gzip decompression classes, and so on). Giving these the same
+         * names across both the client and unpackclass is desirable.
+         *
+         * (Unfortunately we can't deduplicate the classes, as they both expose
+         * different sets of fields/methods, presumably as a result of the
+         * obfuscator removing unused code.)
+         */
+        val clientRemapper = ClassNamePrefixRemapper(client, gl, signlink)
+        val glRemapper = ClassNamePrefixRemapper(gl)
+        val loaderRemapper = ClassNamePrefixRemapper(loader, signlink, unpack)
+        val signlinkRemapper = ClassNamePrefixRemapper(signlink)
+        val unpackClassRemapper = ClassNamePrefixRemapper(unpackClass, unpack)
+        val unpackRemapper = ClassNamePrefixRemapper(unpack)
+
+        client.remap(clientRemapper)
+        gl.remap(glRemapper)
+        loader.remap(loaderRemapper)
+        signlink.remap(signlinkRemapper)
+        unpack.remap(unpackRemapper)
+        unpackClass.remap(unpackClassRemapper)
 
         // bundle libraries together into a common classpath
         val runtime = ClassLoader.getPlatformClassLoader()
@@ -56,6 +93,14 @@ class Deobfuscator @Inject constructor(
             transformer.transform(classPath)
         }
 
+        // strip class name prefixes
+        client.remap(StripClassNamePrefixRemapper)
+        gl.remap(StripClassNamePrefixRemapper)
+        loader.remap(StripClassNamePrefixRemapper)
+        signlink.remap(StripClassNamePrefixRemapper)
+        unpack.remap(StripClassNamePrefixRemapper)
+        unpackClass.remap(StripClassNamePrefixRemapper)
+
         // write output jars
         logger.info { "Writing output jars" }
 

deob/src/main/java/dev/openrs2/deob/SignedClassUtils.kt

@@ -4,9 +4,6 @@ import com.github.michaelbull.logging.InlineLogger
 import dev.openrs2.asm.InsnMatcher
 import dev.openrs2.asm.classpath.Library
 import org.objectweb.asm.Type
-import org.objectweb.asm.commons.ClassRemapper
-import org.objectweb.asm.commons.SimpleRemapper
-import org.objectweb.asm.tree.ClassNode
 import org.objectweb.asm.tree.LdcInsnNode
 import org.objectweb.asm.tree.MethodNode
 
@@ -23,13 +20,6 @@ object SignedClassUtils {
         val dependencies = findDependencies(loader, signedClasses)
         logger.info { "Identified signed class dependencies $dependencies" }
 
-        // rename dependencies of signed classes so they don't clash with client classes
-        val mapping = mutableMapOf<String, String>()
-        for (dependency in dependencies) {
-            mapping[dependency] = "loader_$dependency"
-        }
-        val remapper = SimpleRemapper(mapping)
-
         // delete original signed classes (these have no dependencies)
         for (name in signedClasses) {
             client.remove(name)
@@ -37,12 +27,7 @@ object SignedClassUtils {
 
         // move loader signed classes to signlink
         for (name in signedClasses union dependencies) {
-            val `in` = loader.remove(name)!!
-
-            val out = ClassNode()
-            `in`.accept(ClassRemapper(out, remapper))
-
-            signLink.add(out)
+            signLink.add(loader.remove(name)!!)
         }
     }
 

deob/src/main/java/dev/openrs2/deob/remap/ClassMappingGenerator.kt

@@ -19,7 +19,11 @@ class ClassMappingGenerator(
             populateMapping(clazz)
         }
 
-        mapping.replaceAll(nameMap::mapClassName)
+        mapping.replaceAll { k, v ->
+            val (library, default) = v.splitAtLibraryBoundary()
+            val name = nameMap.mapClassName(k, default)
+            return@replaceAll "$library!$name"
+        }
 
         return mapping
     }
@@ -51,12 +55,12 @@ class ClassMappingGenerator(
 
     private fun generateName(clazz: ClassMetadata): String {
         val name = clazz.name
-        var mappedName = name.substring(0, name.lastIndexOf('/') + 1)
+        var mappedName = name.getLibraryAndPackageName()
 
         val superClass = clazz.superClass
         if (superClass != null && superClass.name != "java/lang/Object") {
             var superName = populateMapping(superClass)
-            superName = superName.substring(superName.lastIndexOf('/') + 1)
+            superName = superName.getClassName()
             mappedName += nameGenerator.generate(superName + "_Sub")
         } else if (clazz.`interface`) {
             mappedName += nameGenerator.generate("Interface")

deob/src/main/java/dev/openrs2/deob/remap/ClassNamePrefix.kt

@@ -0,0 +1,44 @@
+package dev.openrs2.deob.remap
+
+import dev.openrs2.asm.classpath.ExtendedRemapper
+import dev.openrs2.asm.classpath.Library
+
+private val BOUNDARY_CHARS = charArrayOf('/', '!')
+
+fun String.splitAtLibraryBoundary(): Pair<String, String> {
+    val index = indexOf('!')
+    return Pair(substring(0, index), substring(index + 1))
+}
+
+fun String.getLibraryAndPackageName(): String {
+    return substring(0, lastIndexOfAny(BOUNDARY_CHARS) + 1)
+}
+
+fun String.getClassName(): String {
+    return substring(lastIndexOfAny(BOUNDARY_CHARS) + 1)
+}
+
+class ClassNamePrefixRemapper(vararg libraries: Library) : ExtendedRemapper() {
+    private val mapping = mutableMapOf<String, String>()
+
+    init {
+        for (library in libraries) {
+            for (clazz in library) {
+                require(!clazz.name.contains('!')) {
+                    "Input class name contains !, which conflicts with library separator"
+                }
+                mapping.putIfAbsent(clazz.name, "${library.name}!${clazz.name}")
+            }
+        }
+    }
+
+    override fun map(internalName: String): String {
+        return mapping.getOrDefault(internalName, internalName)
+    }
+}
+
+object StripClassNamePrefixRemapper : ExtendedRemapper() {
+    override fun map(internalName: String): String {
+        return internalName.substring(internalName.indexOf('!') + 1)
+    }
+}

deob/src/main/java/dev/openrs2/deob/remap/FieldMappingGenerator.kt

@@ -61,7 +61,7 @@ class FieldMappingGenerator(
             }
             Type.OBJECT -> {
                 val className = classMapping.getOrDefault(elementType.internalName, elementType.internalName)
-                className.substring(className.lastIndexOf('/') + 1) + dimensions
+                className.getClassName() + dimensions
             }
             else -> throw IllegalArgumentException("Unknown field type $elementType")
         }

deob/src/main/java/dev/openrs2/deob/remap/PrefixRemapper.kt

@@ -1,29 +0,0 @@
-package dev.openrs2.deob.remap
-
-import dev.openrs2.asm.classpath.ExtendedRemapper
-import dev.openrs2.asm.classpath.Library
-import dev.openrs2.asm.filter.ClassFilter
-
-class PrefixRemapper(private val prefix: String, private val classes: Set<String>) : ExtendedRemapper() {
-    override fun map(internalName: String): String {
-        return if (classes.contains(internalName)) {
-            prefix + internalName
-        } else {
-            internalName
-        }
-    }
-
-    companion object {
-        fun create(library: Library, prefix: String, excluded: ClassFilter): ExtendedRemapper {
-            val classes = mutableSetOf<String>()
-
-            for (clazz in library) {
-                if (!excluded.matches(clazz.name)) {
-                    classes += clazz.name
-                }
-            }
-
-            return PrefixRemapper(prefix, classes)
-        }
-    }
-}

deob/src/main/java/dev/openrs2/deob/remap/StaticFieldUnscrambler.kt

@@ -51,7 +51,7 @@ class StaticFieldUnscrambler(
                     val member = MemberRef(clazz, field)
                     val partition = inheritedFieldSets[member]!!
                     val owner = nameMap.mapFieldOwner(partition, generator.generate())
-                    fields[partition] = StaticField(owner, simpleInitializers[desc])
+                    fields[partition] = StaticField("${library.name}!$owner", simpleInitializers[desc])
                 }
             }
         }

deob/src/main/java/dev/openrs2/deob/remap/StaticMethodUnscrambler.kt

@@ -37,7 +37,8 @@ class StaticMethodUnscrambler(
 
                     val member = MemberRef(clazz, method)
                     val partition = inheritedMethodSets[member]!!
-                    owners[partition] = nameMap.mapMethodOwner(partition, generator.generate())
+                    val owner = nameMap.mapMethodOwner(partition, generator.generate())
+                    owners[partition] = "${library.name}!$owner"
                 }
             }
         }

deob/src/main/java/dev/openrs2/deob/remap/TypedRemapper.kt

@@ -57,8 +57,6 @@ class TypedRemapper private constructor(
     companion object {
         private val logger = InlineLogger()
 
-        private val LIBRARY_PREFIX_REGEX = Regex("^(?:loader|unpackclass)_")
-
         fun create(classPath: ClassPath, profile: Profile, nameMap: NameMap): TypedRemapper {
             val inheritedFieldSets = classPath.createInheritedFieldSets()
             val inheritedMethodSets = classPath.createInheritedMethodSets()
@@ -133,7 +131,7 @@ class TypedRemapper private constructor(
         }
 
         private fun verifyMapping(name: String, mappedName: String, maxObfuscatedNameLen: Int) {
-            val originalName = name.replace(LIBRARY_PREFIX_REGEX, "")
+            val originalName = StripClassNamePrefixRemapper.map(name)
             if (originalName.length > maxObfuscatedNameLen && originalName != mappedName) {
                 logger.warn { "Remapping probably unobfuscated name $originalName to $mappedName" }
             }

deob/src/main/java/dev/openrs2/deob/transform/ResetTransformer.kt

@@ -35,7 +35,7 @@ class ResetTransformer : Transformer() {
                     val masterReset = findMasterReset(method) ?: continue
                     logger.info { "Identified master reset method $masterReset" }
 
-                    val resetClass = classPath.getClassNode("client")!!
+                    val resetClass = classPath.getClassNode("client!client")!!
                     val resetMethod = resetClass.methods.first {
                         it.name == masterReset.name && it.desc == masterReset.desc
                     }

share/deob/profile.yaml

@@ -1,36 +1,36 @@
 ---
 excluded_classes:
-  - "client"
-  - "com/sun/opengl/impl/x11/**"
-  - "jagex3/jagmisc/jagmisc"
-  - "jaggl/**"
-  - "javax/media/opengl/**"
-  - "loader"
-  - "unpack"
-  - "unpackclass"
+  - "*!client"
+  - "*!com/sun/opengl/impl/x11/**"
+  - "*!jagex3/jagmisc/jagmisc"
+  - "*!jaggl/**"
+  - "*!javax/media/opengl/**"
+  - "*!loader"
+  - "*!unpack"
+  - "*!unpackclass"
 excluded_methods:
-  - "**.<clinit> *"
-  - "**.<init> *"
-  - "**.main *"
-  - "**.providesignlink *"
-  - "**.quit *"
-  - "com/sun/opengl/impl/x11/**.* *"
-  - "jaggl/**.* *"
-  - "javax/media/opengl/**.* *"
+  - "*!**.<clinit> *"
+  - "*!**.<init> *"
+  - "*!**.main *"
+  - "*!**.providesignlink *"
+  - "*!**.quit *"
+  - "*!com/sun/opengl/impl/x11/**.* *"
+  - "*!jaggl/**.* *"
+  - "*!javax/media/opengl/**.* *"
 excluded_fields:
-  - "**.cache *"
-  - "com/sun/opengl/impl/x11/**.* *"
-  - "jaggl/**.* *"
-  - "javax/media/opengl/**.* *"
+  - "*!**.cache *"
+  - "*!com/sun/opengl/impl/x11/**.* *"
+  - "*!jaggl/**.* *"
+  - "*!javax/media/opengl/**.* *"
 entry_points:
-  - "**.<clinit> *"
-  - "**.main *"
-  - "**.providesignlink *"
-  - "client.<init> *"
-  - "com/sun/opengl/impl/x11/DRIHack.begin *"
-  - "com/sun/opengl/impl/x11/DRIHack.end *"
-  - "loader.<init> *"
-  - "unpackclass.<init> *"
+  - "*!**.<clinit> *"
+  - "*!**.main *"
+  - "*!**.providesignlink *"
+  - "*!client.<init> *"
+  - "*!com/sun/opengl/impl/x11/DRIHack.begin *"
+  - "*!com/sun/opengl/impl/x11/DRIHack.end *"
+  - "*!loader.<init> *"
+  - "*!unpackclass.<init> *"
 scrambled_libraries:
   - client
 max_obfuscated_name_len: 2