fix: force rename by checks from RenameVisitor (#432)

skylot · Jan 18, 2019 · da41efa · da41efa
1 parent 9e0cd2e
commit da41efa
Show file tree

Hide file tree

Showing 7 changed files with 128 additions and 18 deletions.
diff --git a/jadx-core/src/main/java/jadx/core/deobf/Deobfuscator.java b/jadx-core/src/main/java/jadx/core/deobf/Deobfuscator.java
@@ -243,6 +243,10 @@ public void renameField(FieldNode field) {
 		}
 	}
 
+	public void forceRenameField(FieldNode field) {
+		field.getFieldInfo().setAlias(makeFieldAlias(field));
+	}
+
 	public void renameMethod(MethodNode mth) {
 		String alias = getMethodAlias(mth);
 		if (alias != null) {
@@ -253,6 +257,13 @@ public void renameMethod(MethodNode mth) {
 		}
 	}
 
+	public void forceRenameMethod(MethodNode mth) {
+		mth.getMethodInfo().setAlias(makeMethodAlias(mth));
+		if (mth.isVirtual()) {
+			resolveOverriding(mth);
+		}
+	}
+
 	public void addPackagePreset(String origPkgName, String pkgAlias) {
 		PackageNode pkg = getPackageNode(origPkgName, true);
 		pkg.setAlias(pkgAlias);

diff --git a/jadx-core/src/main/java/jadx/core/dex/info/FieldInfo.java b/jadx-core/src/main/java/jadx/core/dex/info/FieldInfo.java
@@ -65,6 +65,10 @@ public boolean isRenamed() {
 		return !name.equals(alias);
 	}
 
+	public boolean equalsNameAndType(FieldInfo other) {
+		return name.equals(other.name) && type.equals(other.type);
+	}
+
 	@Override
 	public boolean equals(Object o) {
 		if (this == o) {

diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java
@@ -321,6 +321,15 @@ public FieldNode searchField(FieldInfo field) {
 		return null;
 	}
 
+	public FieldNode searchFieldByNameAndType(FieldInfo field) {
+		for (FieldNode f : fields) {
+			if (f.getFieldInfo().equalsNameAndType(field)) {
+				return f;
+			}
+		}
+		return null;
+	}
+
 	public FieldNode searchFieldByName(String name) {
 		for (FieldNode f : fields) {
 			if (f.getName().equals(name)) {

diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/DexNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/DexNode.java
@@ -150,16 +150,15 @@ public FieldNode resolveField(FieldInfo field) {
 
 	@Nullable
 	FieldNode deepResolveField(@NotNull ClassNode cls, FieldInfo fieldInfo) {
-		FieldNode field = cls.searchFieldByName(fieldInfo.getName());
+		FieldNode field = cls.searchFieldByNameAndType(fieldInfo);
 		if (field != null) {
 			return field;
 		}
-		FieldNode found;
 		ArgType superClass = cls.getSuperClass();
 		if (superClass != null) {
 			ClassNode superNode = resolveClass(superClass);
 			if (superNode != null) {
-				found = deepResolveField(superNode, fieldInfo);
+				FieldNode found = deepResolveField(superNode, fieldInfo);
 				if (found != null) {
 					return found;
 				}
@@ -168,7 +167,7 @@ FieldNode deepResolveField(@NotNull ClassNode cls, FieldInfo fieldInfo) {
 		for (ArgType iFaceType : cls.getInterfaces()) {
 			ClassNode iFaceNode = resolveClass(iFaceType);
 			if (iFaceNode != null) {
-				found = deepResolveField(iFaceNode, fieldInfo);
+				FieldNode found = deepResolveField(iFaceNode, fieldInfo);
 				if (found != null) {
 					return found;
 				}

diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/RenameVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/RenameVisitor.java
@@ -12,14 +12,14 @@
 import jadx.core.deobf.Deobfuscator;
 import jadx.core.deobf.NameMapper;
 import jadx.core.dex.attributes.AFlag;
+import jadx.core.dex.info.AccessInfo;
 import jadx.core.dex.info.ClassInfo;
 import jadx.core.dex.info.FieldInfo;
 import jadx.core.dex.nodes.ClassNode;
 import jadx.core.dex.nodes.DexNode;
 import jadx.core.dex.nodes.FieldNode;
 import jadx.core.dex.nodes.MethodNode;
 import jadx.core.dex.nodes.RootNode;
-import jadx.core.utils.exceptions.JadxException;
 import jadx.core.utils.files.FileUtils;
 import jadx.core.utils.files.InputFile;
 
@@ -49,20 +49,12 @@ public void init(RootNode root) {
 		checkClasses(root, isCaseSensitive);
 	}
 
-	@Override
-	public boolean visit(ClassNode cls) throws JadxException {
-		checkFields(cls);
-		checkMethods(cls);
-		for (ClassNode inner : cls.getInnerClasses()) {
-			visit(inner);
-		}
-		return false;
-	}
-
 	private void checkClasses(RootNode root, boolean caseSensitive) {
 		Set<String> clsNames = new HashSet<>();
 		for (ClassNode cls : root.getClasses(true)) {
 			checkClassName(cls);
+			checkFields(cls);
+			checkMethods(cls);
 			if (!caseSensitive) {
 				ClassInfo classInfo = cls.getClassInfo();
 				String clsFileName = classInfo.getAlias().getFullPath();
@@ -103,20 +95,24 @@ private void checkFields(ClassNode cls) {
 			FieldInfo fieldInfo = field.getFieldInfo();
 			String fieldName = fieldInfo.getAlias();
 			if (!names.add(fieldName) || !NameMapper.isValidIdentifier(fieldName)) {
-				deobfuscator.renameField(field);
+				deobfuscator.forceRenameField(field);
 			}
 		}
 	}
 
 	private void checkMethods(ClassNode cls) {
 		Set<String> names = new HashSet<>();
 		for (MethodNode mth : cls.getMethods()) {
-			if (mth.contains(AFlag.DONT_GENERATE) || mth.getAccessFlags().isConstructor()) {
+			AccessInfo accessFlags = mth.getAccessFlags();
+			if (accessFlags.isConstructor()
+					|| accessFlags.isBridge()
+					|| accessFlags.isSynthetic()
+					|| mth.contains(AFlag.DONT_GENERATE)) {
 				continue;
 			}
 			String signature = mth.getMethodInfo().makeSignature(false);
 			if (!names.add(signature) || !NameMapper.isValidIdentifier(mth.getAlias())) {
-				deobfuscator.renameMethod(mth);
+				deobfuscator.forceRenameMethod(mth);
 			}
 		}
 	}

diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/TestDuplicatedNames.java b/jadx-core/src/test/java/jadx/tests/integration/names/TestDuplicatedNames.java
@@ -0,0 +1,51 @@
+package jadx.tests.integration.names;
+
+import org.junit.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.junit.Assert.assertThat;
+
+public class TestDuplicatedNames extends SmaliTest {
+/*
+	public static class TestCls {
+
+		public Object fieldName;
+		public String fieldName;
+
+		public Object run() {
+			return this.fieldName;
+		}
+
+		public String run() {
+			return this.fieldName;
+		}
+	}
+*/
+	@Test
+	public void test() {
+		commonChecks();
+	}
+
+	@Test
+	public void testWithDeobf() {
+		enableDeobfuscation();
+		commonChecks();
+	}
+
+	private void commonChecks() {
+		ClassNode cls = getClassNodeFromSmaliWithPath("names", "TestDuplicatedNames");
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("Object fieldName;"));
+		assertThat(code, containsOne("String f0fieldName"));
+
+		assertThat(code, containsOne("this.fieldName"));
+		assertThat(code, containsOne("this.f0fieldName"));
+
+		assertThat(code, containsOne("public Object run() {"));
+		assertThat(code, containsOne("public String m0run() {"));
+	}
+}
diff --git a/jadx-core/src/test/smali/names/TestDuplicatedNames.smali b/jadx-core/src/test/smali/names/TestDuplicatedNames.smali
@@ -0,0 +1,40 @@
+.class public LTestDuplicatedNames;
+.super Ljava/lang/Object;
+.source "TestDuplicatedNames.java"
+
+
+# instance fields
+.field public fieldName:Ljava/lang/String;
+.field public fieldName:Ljava/lang/Object;
+
+
+# direct methods
+.method public constructor <init>()V
+    .registers 1
+
+    .prologue
+    .line 3
+    invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+
+    return-void
+.end method
+
+
+# virtual methods
+.method public run()Ljava/lang/String;
+    .registers 2
+
+    .prologue
+    iget-object v0, p0, LTestDuplicatedNames;->fieldName:Ljava/lang/String;
+
+    return-object v0
+.end method
+
+.method public run()Ljava/lang/Object;
+    .registers 2
+
+    .prologue
+    iget-object v0, p0, LTestDuplicatedNames;->fieldName:Ljava/lang/Object;
+
+    return-object v0
+.end method