From 3c7be5e9be814cd566bb4fc931f20b36325e93a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bendeg=C3=BAz=20=C3=81cs?= <30595431+acsbendi@users.noreply.github.com> Date: Sat, 7 Mar 2020 17:52:21 +0100 Subject: [PATCH] fix: use `super` instead `this` when `super` member is shadowed (PR #878) * Added failing test for super member shadowing. * Fixed new test containing incorrect variable names. * Implemented marking super fields used in a subclass with super keyword. * Renamed member variable in the example to reflect smali and test. * Fixed formatting and imports. --- jadx-core/src/main/java/jadx/core/Jadx.java | 24 +-------- .../java/jadx/core/dex/attributes/AFlag.java | 1 + .../dex/instructions/args/RegisterArg.java | 8 +++ .../dex/visitors/FindSuperUsageVisitor.java | 49 +++++++++++++++++++ .../others/TestShadowingSuperMember.java | 47 ++++++++++++++++++ .../others/TestShadowingSuperMember/A.smali | 16 ++++++ .../others/TestShadowingSuperMember/B.smali | 25 ++++++++++ .../others/TestShadowingSuperMember/C.smali | 12 +++++ 8 files changed, 160 insertions(+), 22 deletions(-) create mode 100644 jadx-core/src/main/java/jadx/core/dex/visitors/FindSuperUsageVisitor.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/others/TestShadowingSuperMember.java create mode 100644 jadx-core/src/test/smali/others/TestShadowingSuperMember/A.smali create mode 100644 jadx-core/src/test/smali/others/TestShadowingSuperMember/B.smali create mode 100644 jadx-core/src/test/smali/others/TestShadowingSuperMember/C.smali diff --git a/jadx-core/src/main/java/jadx/core/Jadx.java b/jadx-core/src/main/java/jadx/core/Jadx.java index bf0d8a095..fd8162f90 100644 --- a/jadx-core/src/main/java/jadx/core/Jadx.java +++ b/jadx-core/src/main/java/jadx/core/Jadx.java @@ -11,28 +11,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import jadx.api.JadxArgs; -import jadx.core.dex.visitors.AttachMethodDetails; -import jadx.core.dex.visitors.ClassModifier; -import jadx.core.dex.visitors.ConstInlineVisitor; -import jadx.core.dex.visitors.ConstructorVisitor; -import jadx.core.dex.visitors.DeboxingVisitor; -import jadx.core.dex.visitors.DependencyCollector; -import jadx.core.dex.visitors.DotGraphVisitor; -import jadx.core.dex.visitors.EnumVisitor; -import jadx.core.dex.visitors.ExtractFieldInit; -import jadx.core.dex.visitors.FallbackModeVisitor; -import jadx.core.dex.visitors.FixAccessModifiers; -import jadx.core.dex.visitors.IDexTreeVisitor; -import jadx.core.dex.visitors.InitCodeVariables; -import jadx.core.dex.visitors.MarkFinallyVisitor; -import jadx.core.dex.visitors.MethodInlineVisitor; -import jadx.core.dex.visitors.MethodInvokeVisitor; -import jadx.core.dex.visitors.ModVisitor; -import jadx.core.dex.visitors.PrepareForCodeGen; -import jadx.core.dex.visitors.ProcessAnonymous; -import jadx.core.dex.visitors.ReSugarCode; -import jadx.core.dex.visitors.RenameVisitor; -import jadx.core.dex.visitors.SimplifyVisitor; +import jadx.core.dex.visitors.*; import jadx.core.dex.visitors.blocksmaker.BlockExceptionHandler; import jadx.core.dex.visitors.blocksmaker.BlockFinish; import jadx.core.dex.visitors.blocksmaker.BlockProcessor; @@ -71,6 +50,7 @@ public class Jadx { passes.add(new DebugInfoParseVisitor()); } + passes.add(new FindSuperUsageVisitor()); passes.add(new BlockSplitter()); if (args.isRawCFGOutput()) { passes.add(DotGraphVisitor.dumpRaw()); diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java b/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java index c4f862442..85d54f45c 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java @@ -33,6 +33,7 @@ public enum AFlag { ANONYMOUS_CLASS, THIS, + SUPER, /** * RegisterArg attribute for method arguments diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java b/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java index 6434a3f71..6b8ada9d4 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java @@ -11,6 +11,7 @@ import jadx.core.utils.exceptions.JadxRuntimeException; public class RegisterArg extends InsnArg implements Named { public static final String THIS_ARG_NAME = "this"; + public static final String SUPER_ARG_NAME = "super"; protected final int regNum; // not null after SSATransform pass @@ -87,6 +88,9 @@ public class RegisterArg extends InsnArg implements Named { @Override public String getName() { + if (isSuper()) { + return SUPER_ARG_NAME; + } if (isThis()) { return THIS_ARG_NAME; } @@ -96,6 +100,10 @@ public class RegisterArg extends InsnArg implements Named { return sVar.getName(); } + private boolean isSuper() { + return contains(AFlag.SUPER); + } + @Override public void setName(String name) { if (sVar != null && name != null) { 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 new file mode 100644 index 000000000..fe8a63220 --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/FindSuperUsageVisitor.java @@ -0,0 +1,49 @@ +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/test/java/jadx/tests/integration/others/TestShadowingSuperMember.java b/jadx-core/src/test/java/jadx/tests/integration/others/TestShadowingSuperMember.java new file mode 100644 index 000000000..22aaf374d --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/others/TestShadowingSuperMember.java @@ -0,0 +1,47 @@ +package jadx.tests.integration.others; + +import org.junit.jupiter.api.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.hamcrest.MatcherAssert.assertThat; + +public class TestShadowingSuperMember extends SmaliTest { + // @formatter:off + /* + + public class C { + public C(String s) { + } + } + + public class A { + public int A00; + public A(String s) { + } + } + + public class B extends A { + public C A00; + public B(String str) { + super(str); + } + + public int add(int b) { + return super.A00 + b; + } + } + */ + // @formatter:on + + @Test + public void test() { + allowWarnInCode(); + ClassNode cls = getClassNodeFromSmaliFiles("B"); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("return super.A00 + ")); + } +} diff --git a/jadx-core/src/test/smali/others/TestShadowingSuperMember/A.smali b/jadx-core/src/test/smali/others/TestShadowingSuperMember/A.smali new file mode 100644 index 000000000..dee54405e --- /dev/null +++ b/jadx-core/src/test/smali/others/TestShadowingSuperMember/A.smali @@ -0,0 +1,16 @@ +.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 new file mode 100644 index 000000000..ddcaa6a6c --- /dev/null +++ b/jadx-core/src/test/smali/others/TestShadowingSuperMember/B.smali @@ -0,0 +1,25 @@ +.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 new file mode 100644 index 000000000..98f21ed6e --- /dev/null +++ b/jadx-core/src/test/smali/others/TestShadowingSuperMember/C.smali @@ -0,0 +1,12 @@ +.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