Skip to content

Commit

Permalink
fix: don't rename bridged overridden methods (#1672)
Browse files Browse the repository at this point in the history
  • Loading branch information
skylot committed Sep 23, 2022
1 parent 78aadda commit 79477a2
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
import jadx.core.codegen.json.JsonMappingGen;
import jadx.core.deobf.Deobfuscator;
import jadx.core.deobf.NameMapper;
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.attributes.AType;
import jadx.core.dex.attributes.nodes.MethodOverrideAttr;
import jadx.core.dex.attributes.nodes.RenameReasonAttr;
import jadx.core.dex.info.ClassInfo;
import jadx.core.dex.info.FieldInfo;
Expand Down Expand Up @@ -195,14 +198,31 @@ private static void checkMethods(Deobfuscator deobfuscator, ClassNode cls, JadxA
Set<String> names = new HashSet<>(methods.size());
for (MethodNode mth : methods) {
String signature = mth.getMethodInfo().makeSignature(true, false);
if (!names.add(signature)) {
if (!names.add(signature) && canRename(mth)) {
deobfuscator.forceRenameMethod(mth);
mth.addAttr(new RenameReasonAttr("collision with other method in class"));
}
}
}
}

private static boolean canRename(MethodNode mth) {
if (mth.contains(AFlag.DONT_RENAME)) {
return false;
}
MethodOverrideAttr overrideAttr = mth.get(AType.METHOD_OVERRIDE);
if (overrideAttr != null) {
for (MethodNode relatedMth : overrideAttr.getRelatedMthNodes()) {
if (relatedMth != mth && mth.getParentClass().equals(relatedMth.getParentClass())) {
// ignore rename if exists related method from same class (bridge method in most cases)
// such rename will also rename current method and will not help to resolve name collision
return false;
}
}
}
return true;
}

private static void processRootPackages(Deobfuscator deobfuscator, RootNode root, List<ClassNode> classes) {
Set<String> rootPkgs = collectRootPkgs(classes);
root.getCacheStorage().setRootPkgs(rootPkgs);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package jadx.tests.integration.inline;

import org.assertj.core.api.Condition;

import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.IntegrationTest;
import jadx.tests.api.extensions.profiles.TestProfile;
import jadx.tests.api.extensions.profiles.TestWithProfiles;

import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;

public class TestSyntheticBridgeRename extends IntegrationTest {

@SuppressWarnings("InnerClassMayBeStatic")
public static class TestCls {
private abstract class Inner<V> {
public abstract V get(String value);
}

public class IntInner extends Inner<Integer> {
public Integer get(String value) {
return value.length();
}
}

public void test() {
IntInner inner = new IntInner();
call(inner.get("a"));
}

private static void call(Integer value) {
}
}

@TestWithProfiles({ TestProfile.DX_J8, TestProfile.JAVA8 })
public void test() {
ClassNode cls = getClassNode(TestCls.class);
assertThat(searchCls(cls.getInnerClasses(), "IntInner").getMethods())
.as("check that bridge method was generated by compiler")
.haveAtLeastOne(new Condition<>(mth -> mth.getAccessFlags().isBridge(), "bridge"));

assertThat(cls)
.code()
.doesNotContain("mo0get")
.containsOne("call(inner.get(\"a\"));");
}
}

0 comments on commit 79477a2

Please sign in to comment.