fix: additional checks for synthetic methods remove, rename and inline (

#452)

jadx-core/src/main/java/jadx/core/Jadx.java

@@ -95,10 +95,10 @@ public static List<IDexTreeVisitor> getPassesList(JadxArgs args) {
 			passes.add(new SimplifyVisitor());
 			passes.add(new CheckRegions());
 
-			passes.add(new MethodInlineVisitor());
 			passes.add(new ExtractFieldInit());
 			passes.add(new FixAccessModifiers());
 			passes.add(new ClassModifier());
+			passes.add(new MethodInlineVisitor());
 			passes.add(new EnumVisitor());
 			passes.add(new PrepareForCodeGen());
 			passes.add(new LoopRegionVisitor());

jadx-core/src/main/java/jadx/core/clsp/ClsSet.java

@@ -97,11 +97,11 @@ public static NClass[] makeParentsArray(ClassNode cls, Map<String, NClass> names
 	}
 
 	private static NClass getCls(String fullName, Map<String, NClass> names) {
-		NClass id = names.get(fullName);
-		if (id == null && !names.containsKey(fullName)) {
+		NClass cls = names.get(fullName);
+		if (cls == null) {
 			LOG.debug("Class not found: {}", fullName);
 		}
-		return id;
+		return cls;
 	}
 
 	void save(File output) throws IOException {

jadx-core/src/main/java/jadx/core/clsp/NClass.java

@@ -7,7 +7,7 @@ public class NClass {
 
 	private final String name;
 	private NClass[] parents;
-	private int id;
+	private final int id;
 
 	public NClass(String name, int id) {
 		this.name = name;
@@ -22,10 +22,6 @@ public int getId() {
 		return id;
 	}
 
-	public void setId(int id) {
-		this.id = id;
-	}
-
 	public NClass[] getParents() {
 		return parents;
 	}

jadx-core/src/main/java/jadx/core/codegen/AnnotationGen.java

@@ -63,12 +63,7 @@ private void add(IAttributeNode node, CodeWriter code) {
 		}
 		for (Annotation a : aList.getAll()) {
 			String aCls = a.getAnnotationClass();
-			if (aCls.startsWith(Consts.DALVIK_ANNOTATION_PKG)) {
-				// skip
-				if (Consts.DEBUG) {
-					code.startLine("// " + a);
-				}
-			} else {
+			if (!aCls.startsWith(Consts.DALVIK_ANNOTATION_PKG)) {
 				code.startLine();
 				formatAnnotation(code, a);
 			}

jadx-core/src/main/java/jadx/core/codegen/InsnGen.java

@@ -11,6 +11,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import jadx.core.Consts;
 import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.attributes.AType;
 import jadx.core.dex.attributes.nodes.FieldReplaceAttr;
@@ -637,6 +638,11 @@ private void makeInvoke(InvokeNode insn, CodeWriter code) throws CodegenExceptio
 				break;
 
 			case SUPER:
+				ClassInfo superCallCls = getClassForSuperCall(code, callMth);
+				if (superCallCls != null) {
+					useClass(code, superCallCls);
+					code.add('.');
+				}
 				// use 'super' instead 'this' in 0 arg
 				code.add("super").add('.');
 				k++;
@@ -658,6 +664,36 @@ private void makeInvoke(InvokeNode insn, CodeWriter code) throws CodegenExceptio
 		generateMethodArguments(code, insn, k, callMthNode);
 	}
 
+	@Nullable
+	private ClassInfo getClassForSuperCall(CodeWriter code, MethodInfo callMth) {
+		ClassNode useCls = mth.getParentClass();
+		ClassInfo insnCls = useCls.getAlias();
+		ClassInfo declClass = callMth.getDeclClass();
+		if (insnCls.equals(declClass)) {
+			return null;
+		}
+		ClassNode topClass = useCls.getTopParentClass();
+		if (topClass.getClassInfo().equals(declClass)) {
+			return declClass;
+		}
+		// search call class
+		ClassNode nextParent = useCls;
+		do {
+			ClassInfo nextClsInfo = nextParent.getClassInfo();
+			if (nextClsInfo.equals(declClass)
+					|| ArgType.isInstanceOf(mth.dex(), nextClsInfo.getType(), declClass.getType())) {
+				if (nextParent == useCls) {
+					return null;
+				}
+				return nextClsInfo;
+			}
+			nextParent = nextParent.getParentClass();
+		} while (nextParent != null && nextParent != topClass);
+
+		// search failed, just return parent class
+		return useCls.getParentClass().getClassInfo();
+	}
+
 	void generateMethodArguments(CodeWriter code, InsnNode insn, int startArgNum,
 	                             @Nullable MethodNode callMth) throws CodegenException {
 		int k = startArgNum;
@@ -748,6 +784,9 @@ private boolean inlineMethod(MethodNode callMthNode, InvokeNode insn, CodeWriter
 			return false;
 		}
 		InsnNode inl = mia.getInsn();
+		if (Consts.DEBUG) {
+			code.add("/* inline method: ").add(callMthNode.toString()).add("*/").startLine();
+		}
 		if (callMthNode.getMethodInfo().getArgumentsTypes().isEmpty()) {
 			makeInsn(inl, code, Flags.BODY_ONLY);
 		} else {

jadx-core/src/main/java/jadx/core/dex/info/MethodInfo.java

@@ -43,8 +43,12 @@ public static MethodInfo fromDex(DexNode dex, int mthIndex) {
 	}
 
 	public String makeSignature(boolean includeRetType) {
+		return makeSignature(false, includeRetType);
+	}
+
+	public String makeSignature(boolean useAlias, boolean includeRetType) {
 		StringBuilder signature = new StringBuilder();
-		signature.append(name);
+		signature.append(useAlias ? alias : name);
 		signature.append('(');
 		for (ArgType arg : args) {
 			signature.append(TypeGen.signature(arg));

jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java

@@ -38,12 +38,12 @@
 
 import static jadx.core.dex.nodes.ProcessState.UNLOADED;
 
-public class ClassNode extends LineAttrNode implements ILoadable, IDexNode {
+public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode {
 	private static final Logger LOG = LoggerFactory.getLogger(ClassNode.class);
 
 	private final DexNode dex;
 	private final ClassInfo clsInfo;
-	private final AccessInfo accessFlags;
+	private AccessInfo accessFlags;
 	private ArgType superClass;
 	private List<ArgType> interfaces;
 	private Map<ArgType, List<ArgType>> genericMap;
@@ -409,10 +409,16 @@ public MethodNode getDefaultConstructor() {
 		return null;
 	}
 
+	@Override
 	public AccessInfo getAccessFlags() {
 		return accessFlags;
 	}
 
+	@Override
+	public void setAccessFlags(AccessInfo accessFlags) {
+		this.accessFlags = accessFlags;
+	}
+
 	@Override
 	public DexNode dex() {
 		return dex;

jadx-core/src/main/java/jadx/core/dex/nodes/FieldNode.java

@@ -8,11 +8,11 @@
 import jadx.core.dex.info.FieldInfo;
 import jadx.core.dex.instructions.args.ArgType;
 
-public class FieldNode extends LineAttrNode {
+public class FieldNode extends LineAttrNode implements ICodeNode {
 
 	private final ClassNode parent;
 	private final FieldInfo fieldInfo;
-	private final AccessInfo accFlags;
+	private AccessInfo accFlags;
 
 	private ArgType type; // store signature
 
@@ -32,10 +32,16 @@ public FieldInfo getFieldInfo() {
 		return fieldInfo;
 	}
 
+	@Override
 	public AccessInfo getAccessFlags() {
 		return accFlags;
 	}
 
+	@Override
+	public void setAccessFlags(AccessInfo accFlags) {
+		this.accFlags = accFlags;
+	}
+
 	public String getName() {
 		return fieldInfo.getName();
 	}
@@ -56,6 +62,21 @@ public ClassNode getParentClass() {
 		return parent;
 	}
 
+	@Override
+	public String typeName() {
+		return "field";
+	}
+
+	@Override
+	public DexNode dex() {
+		return parent.dex();
+	}
+
+	@Override
+	public RootNode root() {
+		return parent.root();
+	}
+
 	@Override
 	public int hashCode() {
 		return fieldInfo.hashCode();

jadx-core/src/main/java/jadx/core/dex/nodes/ICodeNode.java

@@ -0,0 +1,10 @@
+package jadx.core.dex.nodes;
+
+import jadx.core.dex.attributes.IAttributeNode;
+import jadx.core.dex.info.AccessInfo;
+
+public interface ICodeNode extends IDexNode, IAttributeNode {
+	AccessInfo getAccessFlags();
+
+	void setAccessFlags(AccessInfo newAccessFlags);
+}

jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java

@@ -46,7 +46,7 @@
 
 import static jadx.core.utils.Utils.lockList;
 
-public class MethodNode extends LineAttrNode implements ILoadable, IDexNode {
+public class MethodNode extends LineAttrNode implements ILoadable, ICodeNode {
 	private static final Logger LOG = LoggerFactory.getLogger(MethodNode.class);
 
 	private final MethodInfo mthInfo;
@@ -595,12 +595,14 @@ public List<SSAVar> getSVars() {
 		return sVars;
 	}
 
+	@Override
 	public AccessInfo getAccessFlags() {
 		return accFlags;
 	}
 
-	public void setAccFlags(AccessInfo accFlags) {
-		this.accFlags = accFlags;
+	@Override
+	public void setAccessFlags(AccessInfo newAccessFlags) {
+		this.accFlags = newAccessFlags;
 	}
 
 	public Region getRegion() {

jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java

@@ -5,6 +5,7 @@
 
 import com.android.dx.rop.code.AccessFlags;
 
+import jadx.core.Consts;
 import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.attributes.AType;
 import jadx.core.dex.attributes.nodes.FieldReplaceAttr;
@@ -15,6 +16,7 @@
 import jadx.core.dex.instructions.IndexInsnNode;
 import jadx.core.dex.instructions.InsnType;
 import jadx.core.dex.instructions.InvokeNode;
+import jadx.core.dex.instructions.InvokeType;
 import jadx.core.dex.instructions.args.ArgType;
 import jadx.core.dex.instructions.args.InsnArg;
 import jadx.core.dex.instructions.args.InsnWrapArg;
@@ -140,7 +142,11 @@ private static void removeSyntheticMethods(ClassNode cls, MethodNode mth) {
 			return;
 		}
 		if (removeBridgeMethod(cls, mth)) {
-			mth.add(AFlag.DONT_GENERATE);
+			if (Consts.DEBUG) {
+				mth.addAttr(AType.COMMENTS, "Removed as synthetic bridge method");
+			} else {
+				mth.add(AFlag.DONT_GENERATE);
+			}
 			return;
 		}
 		// remove synthetic constructor for inner classes
@@ -219,40 +225,45 @@ private static boolean removeBridgeMethod(ClassNode cls, MethodNode mth) {
 
 	private static boolean checkSyntheticWrapper(MethodNode mth, InsnNode insn) {
 		InsnType insnType = insn.getType();
-		if (insnType == InsnType.INVOKE) {
-			MethodInfo callMth = ((InvokeNode) insn).getCallMth();
-			MethodNode wrappedMth = mth.root().deepResolveMethod(callMth);
-			if (wrappedMth != null) {
-				AccessInfo wrappedAccFlags = wrappedMth.getAccessFlags();
-				if (wrappedAccFlags.isStatic()) {
-					return false;
-				}
-				if (callMth.getArgsCount() != mth.getMethodInfo().getArgsCount()) {
-					return false;
-				}
-				// rename method only from current class
-				if (!mth.getParentClass().equals(wrappedMth.getParentClass())) {
-					return false;
-				}
-				// all args must be registers passed from method args (allow only casts insns)
-				for (InsnArg arg : insn.getArguments()) {
-					if (!registersAndCastsOnly(arg)) {
-						return false;
-					}
-				}
-				String alias = mth.getAlias();
-				if (Objects.equals(wrappedMth.getAlias(), alias)) {
-					return true;
-				}
-				if (!wrappedAccFlags.isPublic()) {
-					// must be public
-					FixAccessModifiers.changeVisibility(wrappedMth, AccessFlags.ACC_PUBLIC);
-				}
-				wrappedMth.getMethodInfo().setAlias(alias);
-				return true;
+		if (insnType != InsnType.INVOKE) {
+			return false;
+		}
+		InvokeNode invokeInsn = (InvokeNode) insn;
+		if (invokeInsn.getInvokeType() == InvokeType.SUPER) {
+			return false;
+		}
+		MethodInfo callMth = invokeInsn.getCallMth();
+		MethodNode wrappedMth = mth.root().deepResolveMethod(callMth);
+		if (wrappedMth == null) {
+			return false;
+		}
+		AccessInfo wrappedAccFlags = wrappedMth.getAccessFlags();
+		if (wrappedAccFlags.isStatic()) {
+			return false;
+		}
+		if (callMth.getArgsCount() != mth.getMethodInfo().getArgsCount()) {
+			return false;
+		}
+		// rename method only from current class
+		if (!mth.getParentClass().equals(wrappedMth.getParentClass())) {
+			return false;
+		}
+		// all args must be registers passed from method args (allow only casts insns)
+		for (InsnArg arg : insn.getArguments()) {
+			if (!registersAndCastsOnly(arg)) {
+				return false;
 			}
 		}
-		return false;
+		// remove confirmed, change visibility and name if needed
+		if (!wrappedAccFlags.isPublic()) {
+			// must be public
+			FixAccessModifiers.changeVisibility(wrappedMth, AccessFlags.ACC_PUBLIC);
+		}
+		String alias = mth.getAlias();
+		if (!Objects.equals(wrappedMth.getAlias(), alias)) {
+			wrappedMth.getMethodInfo().setAlias(alias);
+		}
+		return true;
 	}
 
 	private static boolean registersAndCastsOnly(InsnArg arg) {
@@ -274,8 +285,7 @@ private static boolean isMethodUnique(ClassNode cls, MethodNode mth) {
 			if (otherMth != mth) {
 				MethodInfo omi = otherMth.getMethodInfo();
 				if (omi.getName().equals(mi.getName())
-						&& omi.getArgumentsTypes().size() == mi.getArgumentsTypes().size()) {
-					// TODO: check objects types
+						&& Objects.equals(omi.getArgumentsTypes(), mi.getArgumentsTypes())) {
 					return false;
 				}
 			}