search

ReVanced / revanced-patcher

💉 ReVanced Patcher used to patch Android applications
https://revanced.app
GNU General Public License v3.0
2.54k stars 206 forks source link

ClassWriter cannot compute stack frames for certain types #13

Closed Sculas closed 2 years ago

Sculas commented 2 years ago

To reproduce this issue, please download the stock YouTube app from any source of your choice. I recommend the Vanced Codebucket mirror. Use dex2jar to convert the APK to JAR. Then run this code:

val testData = javaClass.getResourceAsStream("/stock.jar")!!
Patcher(testData, ByteArrayOutputStream(), emptyArray()).save()

This will fail with this error:

Exception in thread "main" java.lang.TypeNotPresentException: Type ysm not present
    at org.objectweb.asm.ClassWriter.getCommonSuperClass(ClassWriter.java:1025)
    at org.objectweb.asm.SymbolTable.addMergedType(SymbolTable.java:1202)
    at org.objectweb.asm.Frame.merge(Frame.java:1299)
    at org.objectweb.asm.Frame.merge(Frame.java:1197)
    at org.objectweb.asm.MethodWriter.computeAllFrames(MethodWriter.java:1610)
    at org.objectweb.asm.MethodWriter.visitMaxs(MethodWriter.java:1546)
    at org.objectweb.asm.tree.MethodNode.accept(MethodNode.java:769)
    at org.objectweb.asm.tree.MethodNode.accept(MethodNode.java:649)
    at org.objectweb.asm.tree.ClassNode.accept(ClassNode.java:452)
    at app.revanced.patcher.util.Io.saveAsJar(Io.kt:86)
    at app.revanced.patcher.Patcher.save(Patcher.kt:48)
    ... 2 more
Caused by: java.lang.ClassNotFoundException: ysm
    at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
    at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:349)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:348)
    at org.objectweb.asm.ClassWriter.getCommonSuperClass(ClassWriter.java:1023)
    ... 12 more

This exception is thrown because the standard implementation of getCommonSuperClass in ClassWriter uses the system ClassLoader to find said class to get the type out of it (it will invoke Type#getType(Class<?> clazz)).

This issue can be mitigated by telling setting the mode to COMPUTE_MAXS instead of COMPUTE_FRAMES. But this will not work when adding branches (afaik) because only the local vars are computed, and new frames for each branch are not. This means you need to add the stack frame yourself if you patch the code. Since the patcher is also meant as a general-use library, we should not rely on the experience of the developers to write stack frames themselves. We should really fix this issue instead. I'm not yet sure how to test this, I will have to look into adding tests for this case.

The intended solution for this is to make our own ClassWriter implementation and override the getCommonSuperClass to use our class cache and get the types out of that. Note that computing frames is really expensive and will take up some CPU and memory.

Sculas commented 2 years ago

I may have to declare this a wontfix. I'm currently writing the custom implementation of getCommonSuperClass and it seems we also need to load the entire Android API into memory. This implementation already uses a lot of memory, so I think the only way out is to write stack frames yourself. Fixing this issue won't help #14 in any way.

Sculas commented 2 years ago

Work is being done in fix/classwriter. This is a wontfix for now.

Sculas commented 2 years ago

This has been "fixed" in https://github.com/ReVancedTeam/revanced-patcher/commit/c8a017a4c0a659e8450df24e56bfcb90cb0739bd. You have to write stack frames yourself if you modify the stack. I'm going to add helper methods for this.

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version 1.0.0-dev.9 :tada:

The release is available on:

Your semantic-release bot :package::rocket: