diff --git a/jadx-core/src/main/java/jadx/core/Jadx.java b/jadx-core/src/main/java/jadx/core/Jadx.java index 1a05c5986..068436ea6 100644 --- a/jadx-core/src/main/java/jadx/core/Jadx.java +++ b/jadx-core/src/main/java/jadx/core/Jadx.java @@ -49,13 +49,10 @@ public class Jadx { if (args.isDebugInfo()) { passes.add(new DebugInfoParseVisitor()); } - - passes.add(new FindSuperUsageVisitor()); passes.add(new BlockSplitter()); if (args.isRawCFGOutput()) { passes.add(DotGraphVisitor.dumpRaw()); } - passes.add(new BlockProcessor()); passes.add(new BlockExceptionHandler()); passes.add(new BlockFinish()); @@ -72,6 +69,7 @@ public class Jadx { } passes.add(new GenericTypesVisitor()); + passes.add(new ShadowFieldVisitor()); passes.add(new DeboxingVisitor()); passes.add(new ModVisitor()); passes.add(new CodeShrinkVisitor()); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/FindSuperUsageVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/FindSuperUsageVisitor.java deleted file mode 100644 index fe8a63220..000000000 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/FindSuperUsageVisitor.java +++ /dev/null @@ -1,49 +0,0 @@ -package jadx.core.dex.visitors; - -import jadx.core.dex.attributes.AFlag; -import jadx.core.dex.instructions.args.ArgType; -import jadx.core.dex.instructions.args.InsnArg; -import jadx.core.dex.instructions.args.RegisterArg; -import jadx.core.dex.nodes.InsnNode; -import jadx.core.dex.nodes.MethodNode; -import jadx.core.dex.visitors.blocksmaker.BlockSplitter; -import jadx.core.utils.exceptions.JadxException; - -@JadxVisitor( - name = "FindSuperUsageVisitor", - desc = "Finds variables where a member of the super class is used and marks them.", - runBefore = BlockSplitter.class -) -public class FindSuperUsageVisitor extends AbstractVisitor { - - @Override - public void visit(MethodNode mth) throws JadxException { - if (mth.isNoCode()) { - return; - } - process(mth); - } - - private static void process(MethodNode methodNode) { - ArgType superClass = methodNode.getParentClass().getSuperClass(); - if (superClass == null) { - return; - } - String superClassName = superClass.getObject(); - if (superClassName.equals("java.lang.Object")) { - return; - } - for (InsnNode instruction : methodNode.getInstructions()) { - if (instruction != null) { - for (InsnArg argument : instruction.getArguments()) { - if (argument.isRegister()) { - ArgType argumentType = ((RegisterArg) argument).getInitType(); - if (argumentType.isObject() && argumentType.getObject().equals(superClassName)) { - argument.add(AFlag.SUPER); - } - } - } - } - } - } -} diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ShadowFieldVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ShadowFieldVisitor.java new file mode 100644 index 000000000..d4d4e0e67 --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ShadowFieldVisitor.java @@ -0,0 +1,181 @@ +package jadx.core.dex.visitors; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.jetbrains.annotations.Nullable; + +import jadx.core.dex.attributes.AFlag; +import jadx.core.dex.info.FieldInfo; +import jadx.core.dex.instructions.IndexInsnNode; +import jadx.core.dex.instructions.InsnType; +import jadx.core.dex.instructions.args.ArgType; +import jadx.core.dex.instructions.args.InsnArg; +import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.ClassNode; +import jadx.core.dex.nodes.FieldNode; +import jadx.core.dex.nodes.InsnNode; +import jadx.core.dex.nodes.MethodNode; +import jadx.core.dex.nodes.RootNode; +import jadx.core.dex.visitors.shrink.CodeShrinkVisitor; +import jadx.core.dex.visitors.typeinference.TypeInferenceVisitor; +import jadx.core.utils.exceptions.JadxException; + +@JadxVisitor( + name = "ShadowFieldVisitor", + desc = "Fix shadowed field access", + runAfter = TypeInferenceVisitor.class, + runBefore = CodeShrinkVisitor.class +) +public class ShadowFieldVisitor extends AbstractVisitor { + private Map fixInfoMap; + + @Override + public void init(RootNode root) { + Map map = new HashMap<>(); + for (ClassNode cls : root.getClasses(true)) { + Map fieldFixMap = searchShadowedFields(cls); + if (!fieldFixMap.isEmpty()) { + FieldFixInfo fixInfo = new FieldFixInfo(); + fixInfo.fieldFixMap = fieldFixMap; + map.put(cls.getRawName(), fixInfo); + } + } + this.fixInfoMap = map; + } + + @Override + public void visit(MethodNode mth) throws JadxException { + if (mth.isNoCode()) { + return; + } + fixShadowFieldAccess(mth, fixInfoMap); + } + + private static class FieldFixInfo { + Map fieldFixMap; + } + + private enum FieldFixType { + SUPER, + CAST + } + + private static Map searchShadowedFields(ClassNode thisCls) { + List allFields = collectAllInstanceFields(thisCls); + if (allFields.isEmpty()) { + return Collections.emptyMap(); + } + Map> mapByName = groupByName(allFields); + mapByName.entrySet().removeIf(entry -> entry.getValue().size() == 1); + if (mapByName.isEmpty()) { + return Collections.emptyMap(); + } + Map fixMap = new HashMap<>(); + for (List fields : mapByName.values()) { + boolean fromThisCls = fields.get(0).getParentClass() == thisCls; + if (fromThisCls && fields.size() == 2) { + // only one super class contains same field => can use super + FieldNode otherField = fields.get(1); + if (otherField.getParentClass() != thisCls) { + fixMap.put(otherField.getFieldInfo(), FieldFixType.SUPER); + } + } else { + // several super classes contains same field => can't use super, need cast to exact class + for (FieldNode field : fields) { + if (field.getParentClass() != thisCls) { + fixMap.put(field.getFieldInfo(), FieldFixType.CAST); + } + } + } + } + return fixMap; + } + + private static Map> groupByName(List allFields) { + Map> groupByName = new HashMap<>(allFields.size()); + for (FieldNode field : allFields) { + groupByName + .computeIfAbsent(field.getName(), k -> new ArrayList<>()) + .add(field); + } + return groupByName; + } + + private static List collectAllInstanceFields(ClassNode cls) { + List fieldsList = new ArrayList<>(); + ClassNode currentClass = cls; + while (currentClass != null) { + for (FieldNode field : currentClass.getFields()) { + if (!field.getAccessFlags().isStatic()) { + fieldsList.add(field); + } + } + ArgType superClass = currentClass.getSuperClass(); + if (superClass == null) { + break; + } + currentClass = cls.root().resolveClass(superClass); + } + return fieldsList; + } + + private static void fixShadowFieldAccess(MethodNode mth, Map fixInfoMap) { + for (BlockNode block : mth.getBasicBlocks()) { + for (InsnNode insn : block.getInstructions()) { + processInsn(mth, insn, fixInfoMap); + } + } + } + + private static void processInsn(MethodNode mth, InsnNode insn, Map fixInfoMap) { + FieldInfo fieldInfo = getFieldInfo(insn); + if (fieldInfo == null) { + return; + } + InsnArg arg = insn.getArg(insn.getArgsCount() - 1); + ArgType type = arg.getType(); + if (!type.isTypeKnown() || !type.isObject()) { + return; + } + FieldFixInfo fieldFixInfo = fixInfoMap.get(type.getObject()); + if (fieldFixInfo == null) { + return; + } + FieldFixType fieldFixType = fieldFixInfo.fieldFixMap.get(fieldInfo); + if (fieldFixType == null) { + return; + } + fixFieldAccess(mth, fieldInfo, fieldFixType, arg); + } + + @Nullable + private static FieldInfo getFieldInfo(InsnNode insn) { + switch (insn.getType()) { + case IPUT: + case IGET: + return ((FieldInfo) ((IndexInsnNode) insn).getIndex()); + default: + return null; + } + } + + private static void fixFieldAccess(MethodNode mth, FieldInfo fieldInfo, FieldFixType fieldFixType, InsnArg arg) { + if (fieldFixType == FieldFixType.SUPER) { + if (arg.isThis()) { + // convert 'this' to 'super' + arg.add(AFlag.SUPER); + return; + } + } + // apply cast + InsnNode castInsn = new IndexInsnNode(InsnType.CAST, fieldInfo.getDeclClass().getType(), 1); + castInsn.addArg(arg.duplicate()); + castInsn.add(AFlag.SYNTHETIC); + castInsn.add(AFlag.EXPLICIT_CAST); + arg.wrapInstruction(mth, castInsn, false); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/others/TestShadowingSuperMember.java b/jadx-core/src/test/java/jadx/tests/integration/others/TestShadowingSuperMember.java index 22aaf374d..f09fcd46c 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/others/TestShadowingSuperMember.java +++ b/jadx-core/src/test/java/jadx/tests/integration/others/TestShadowingSuperMember.java @@ -2,46 +2,54 @@ package jadx.tests.integration.others; import org.junit.jupiter.api.Test; -import jadx.core.dex.nodes.ClassNode; -import jadx.tests.api.SmaliTest; +import jadx.tests.api.IntegrationTest; -import static jadx.tests.api.utils.JadxMatchers.containsOne; -import static org.hamcrest.MatcherAssert.assertThat; +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; -public class TestShadowingSuperMember extends SmaliTest { - // @formatter:off - /* - - public class C { +public class TestShadowingSuperMember extends IntegrationTest { + public static class TestCls { + public static class C { public C(String s) { } } - public class A { - public int A00; + public static class A { + public int a00; + public A(String s) { } } - public class B extends A { - public C A00; + public static class B extends A { + public C a00; + public B(String str) { super(str); } public int add(int b) { - return super.A00 + b; + return super.a00 + b; + } + + public int sub(int b) { + return ((A) this).a00 - b; } } - */ - // @formatter:on + + public void check() { + B b = new B(""); + ((A) b).a00 = 2; + assertThat(b.add(3)).isEqualTo(5); + assertThat(b.sub(3)).isEqualTo(-1); + } + } @Test public void test() { - allowWarnInCode(); - ClassNode cls = getClassNodeFromSmaliFiles("B"); - String code = cls.getCode().toString(); - - assertThat(code, containsOne("return super.A00 + ")); + assertThat(getClassNode(TestCls.class)) + .code() + .containsOne("return super.a00 + b;") + .containsOne("return super.a00 - b;") + .containsOne("((A) b).a00 = 2;"); } } diff --git a/jadx-core/src/test/smali/others/TestShadowingSuperMember/A.smali b/jadx-core/src/test/smali/others/TestShadowingSuperMember/A.smali deleted file mode 100644 index dee54405e..000000000 --- a/jadx-core/src/test/smali/others/TestShadowingSuperMember/A.smali +++ /dev/null @@ -1,16 +0,0 @@ -.class public Lothers/A; -.super Ljava/lang/Object; - -# instance fields -.field public A00:I - -# direct methods -.method public constructor (Ljava/lang/String;)V - .registers 3 - - .prologue - - invoke-direct {p0}, Ljava/lang/Object;->()V - - return-void -.end method diff --git a/jadx-core/src/test/smali/others/TestShadowingSuperMember/B.smali b/jadx-core/src/test/smali/others/TestShadowingSuperMember/B.smali deleted file mode 100644 index ddcaa6a6c..000000000 --- a/jadx-core/src/test/smali/others/TestShadowingSuperMember/B.smali +++ /dev/null @@ -1,25 +0,0 @@ -.class public Lothers/B; -.super Lothers/A; - -.field public A00:Lothers/C; - -# direct methods -.method public constructor (Ljava/lang/String;)V - .registers 3 - - .prologue - invoke-direct {p0, p1}, Lothers/A;->(Ljava/lang/String;)V - - return-void -.end method - - -.method public add(I)I - .registers 3 - - iget v1, p0, Lothers/A;->A00:I - - add-int/2addr v1, p1 - - return v1 -.end method diff --git a/jadx-core/src/test/smali/others/TestShadowingSuperMember/C.smali b/jadx-core/src/test/smali/others/TestShadowingSuperMember/C.smali deleted file mode 100644 index 98f21ed6e..000000000 --- a/jadx-core/src/test/smali/others/TestShadowingSuperMember/C.smali +++ /dev/null @@ -1,12 +0,0 @@ -.class public Lothers/C; -.super Ljava/lang/Object; - -.method public constructor (Ljava/lang/String;)V - .registers 3 - - .prologue - - invoke-direct {p0}, Ljava/lang/Object;->()V - - return-void -.end method