QuiltMC / enigma

A deobfuscation/remapping tool for Java bytecode, fork of cuchaz's Enigma.
GNU Lesser General Public License v3.0
59 stars 31 forks source link

Renaming an interface method that has bridges renames one of the bridges #212

Open 770grappenmaker opened 4 months ago

770grappenmaker commented 4 months ago

For example, when mapping QM, I stumbled upon the class net/minecraft/unmapped/C_bhbelvxt, which appears to be some sort of collection type that extends the capabilities of a Deque with some of the methods from List. Its first method is m_dnwfaduj, but of course since I haven't mapped it yet enigma draws it as gray (the proposed name is reversed, which seems correct). If I then give it a name (either by marking as unmapped or renaming) it has two unexpected behaviours:

This might have something to do with the fact a name is proposed.

I tried overwriting the entry in the mapping file as METHOD m_dnwfaduj reversed ()Lnet/minecraft/unmapped/C_bhbelvxt;, and it appears that enigma straight up ignores it. Saving the mappings or requesting to drop invalid mappings does not seem to have an effect.

770grappenmaker commented 4 months ago

Update: it does not have something to do with the fact a name is proposed. Manually building enigma to strip the call to registerSpecializedMethodNamingService indeed removes the bridge method name proposal, but then if I rename it to reversed, or any other name apparently, it does now update the name to appear as "mapped", but the following lines are added to its .mapping file:

    METHOD reversed reversed ()Ljava/util/Deque;
    METHOD reversed reversed ()Ljava/util/SequencedCollection;

Additionally, by removing the proposal, the decompilation of the implementation, net/minecraft/unmapped/C_scihbqqa, successfully remaps, which is extra weird since the return types do not match.

770grappenmaker commented 4 months ago

Update: I decided to poke around in the code, and interestingly, in EntryRemapper#doPutMapping, if I print the "resolved entries" (I guess this is for resolving overloads etc?), it reads

Resolved [net/minecraft/unmapped/C_bhbelvxt.reversed()Ljava/util/Deque;, net/minecraft/unmapped/C_bhbelvxt.reversed()Ljava/util/SequencedCollection;] while putting net/minecraft/unmapped/C_bhbelvxt.m_dnwfaduj()Lnet/minecraft/unmapped/C_bhbelvxt;
770grappenmaker commented 4 months ago

If I straight up disable bridge indexing, the issue is fixed. (commenting this.getIndex(BridgeMethodIndex.class).findBridgeMethods();)

IotaBread commented 4 months ago

Update: I decided to poke around in the code, and interestingly, in EntryRemapper#doPutMapping, if I print the "resolved entries" (I guess this is for resolving overloads etc?), it reads

Of course it's an entry resolving issue

ix0rai commented 4 months ago

pretty much the most classic component to break

770grappenmaker commented 4 months ago

I can see why this problem hasn't occurred yet before: according to a quick script I wrote, the following classes (mapped if name is known for the latest build of QM) have potential bridge methods:

net/minecraft/unmapped/C_bhbelvxt
net/minecraft/village/VillagerDataContainer
net/minecraft/client/option/Option$IntRangeBaseValueSet

(the strategy for determining these is basically checking whether for some interface there exists a method such that it is not ACC_PRIVATE, ACC_STATIC or ACC_FINAL, and calls only a single other method, apparently proguard strips ACC_BRIDGE?) The only one that has a bridge method related to a java/ type is our perpetrator C_bhbelvxt... can't believe this bug occurs for a single class...

For reference, I used this snippet of code

val MethodNode.isPotentialBridge get() =
    access and ACC_SYNTHETIC != 0
            && access and ACC_PRIVATE == 0
            && access and ACC_STATIC == 0
            && access and ACC_FINAL == 0
            && instructions.countIsInstance<MethodInsnNode>() == 1

fun main() {
    JarInputStream(Path("""/path/to/1.21-hashed.jar""").inputStream()).use { input ->
        val itfWithBridge = generateSequence { input.nextJarEntry }
            .filter { it.name.endsWith(".class") }
            .map { ClassReader(input.readBytes()) }
            .filter { it.access and ACC_INTERFACE != 0 }
            .filter { reader ->
                ClassNode().also { reader.accept(it, ClassReader.SKIP_DEBUG) }.methods.any { it.isPotentialBridge }
            }.map { it.className }

        val mappingsPath = """/path/to/quilt-mappings-1.21+build.9-mergedv2/mappings/mappings.tiny"""
        val mappings = Path(mappingsPath).useLines { TinyMappingsV2Format.parse(it) }
        val remapper = SimpleRemapper(mappings.asASMMapping(
            from = "hashed",
            to = "named",
            includeMethods = false,
            includeFields = false
        ))

        itfWithBridge.forEach { println(remapper.map(it)) }
    }
}