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 380fefba3..fd4791fd3 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 @@ -185,6 +185,10 @@ public class RegisterArg extends InsnArg implements Named { return regNum == ((RegisterArg) arg).getRegNum(); } + public boolean sameType(InsnArg arg) { + return this.getType().equals(arg.getType()); + } + public boolean sameCodeVar(RegisterArg arg) { return this.getSVar().getCodeVar() == arg.getSVar().getCodeVar(); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java index f26fb5025..3d82cf408 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java @@ -1,10 +1,7 @@ package jadx.core.dex.visitors; import java.util.ArrayList; -import java.util.Collections; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import jadx.api.plugins.input.data.AccessFlags; @@ -63,7 +60,6 @@ public class ClassModifier extends AbstractVisitor { removeSyntheticFields(cls); cls.getMethods().forEach(ClassModifier::removeSyntheticMethods); cls.getMethods().forEach(ClassModifier::removeEmptyMethods); - cls.getMethods().forEach(ClassModifier::processAnonymousConstructor); return false; } @@ -161,7 +157,8 @@ public class ClassModifier extends AbstractVisitor { return; } // remove synthetic constructor for inner classes - if (mth.isConstructor() && mth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE)) { + if (mth.isConstructor() + && (mth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE) || mth.contains(AFlag.ANONYMOUS_CONSTRUCTOR))) { InsnNode insn = BlockUtils.getOnlyOneInsnFromMth(mth); if (insn != null) { List args = mth.getArgRegs(); @@ -173,26 +170,32 @@ public class ClassModifier extends AbstractVisitor { } private static boolean isRemovedClassInArgs(ClassNode cls, List mthArgs) { + boolean removedFound = false; for (RegisterArg arg : mthArgs) { ArgType argType = arg.getType(); if (!argType.isObject()) { continue; } + boolean remove = false; ClassNode argCls = cls.root().resolveClass(argType); if (argCls == null) { // check if missing class from current top class ClassInfo argClsInfo = ClassInfo.fromType(cls.root(), argType); - if (argClsInfo.isInner() + if (argClsInfo.getParentClass() != null && cls.getFullName().startsWith(argClsInfo.getParentClass().getFullName())) { - return true; + remove = true; } } else { if (argCls.contains(AFlag.DONT_GENERATE) || isEmptySyntheticClass(argCls)) { - return true; + remove = true; } } + if (remove) { + arg.add(AFlag.REMOVE); + removedFound = true; + } } - return false; + return removedFound; } /** @@ -332,91 +335,6 @@ public class ClassModifier extends AbstractVisitor { } } - /** - * Remove super call and put into removed fields from anonymous constructor - */ - private static void processAnonymousConstructor(MethodNode mth) { - if (!mth.contains(AFlag.ANONYMOUS_CONSTRUCTOR)) { - return; - } - List usedInsns = new ArrayList<>(); - Map argsMap = getArgsToFieldsMapping(mth, usedInsns); - for (Map.Entry entry : argsMap.entrySet()) { - FieldNode field = entry.getValue(); - if (field == null) { - continue; - } - InsnArg arg = entry.getKey(); - field.addAttr(new FieldReplaceAttr(arg)); - field.add(AFlag.DONT_GENERATE); - if (arg.isRegister()) { - arg.add(AFlag.SKIP_ARG); - SkipMethodArgsAttr.skipArg(mth, ((RegisterArg) arg)); - } - } - for (InsnNode usedInsn : usedInsns) { - usedInsn.add(AFlag.DONT_GENERATE); - } - } - - private static Map getArgsToFieldsMapping(MethodNode mth, List usedInsns) { - MethodInfo callMth = mth.getMethodInfo(); - ClassNode cls = mth.getParentClass(); - List argList = mth.getArgRegs(); - ClassNode outerCls = mth.getUseIn().get(0).getParentClass(); - int startArg = 0; - if (callMth.getArgsCount() != 0 && callMth.getArgumentsTypes().get(0).equals(outerCls.getClassInfo().getType())) { - startArg = 1; - } - Map map = new LinkedHashMap<>(); - int argsCount = argList.size(); - for (int i = startArg; i < argsCount; i++) { - RegisterArg arg = argList.get(i); - InsnNode useInsn = getParentInsnSkipMove(arg); - if (useInsn == null) { - return Collections.emptyMap(); - } - switch (useInsn.getType()) { - case IPUT: - FieldNode fieldNode = cls.searchField((FieldInfo) ((IndexInsnNode) useInsn).getIndex()); - if (fieldNode == null || !fieldNode.getAccessFlags().isSynthetic()) { - return Collections.emptyMap(); - } - map.put(arg, fieldNode); - usedInsns.add(useInsn); - break; - - case CONSTRUCTOR: - ConstructorInsn superConstr = (ConstructorInsn) useInsn; - if (!superConstr.isSuper()) { - return Collections.emptyMap(); - } - usedInsns.add(useInsn); - break; - - default: - return Collections.emptyMap(); - } - } - return map; - } - - private static InsnNode getParentInsnSkipMove(RegisterArg arg) { - SSAVar sVar = arg.getSVar(); - if (sVar.getUseCount() != 1) { - return null; - } - RegisterArg useArg = sVar.getUseList().get(0); - InsnNode parentInsn = useArg.getParentInsn(); - if (parentInsn == null) { - return null; - } - if (parentInsn.getType() == InsnType.MOVE) { - return getParentInsnSkipMove(parentInsn.getResult()); - } - return parentInsn; - } - private static boolean isNonDefaultConstructorExists(MethodNode defCtor) { ClassNode parentClass = defCtor.getParentClass(); for (MethodNode mth : parentClass.getMethods()) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java index 9e0a8ffef..000e1846a 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java @@ -222,9 +222,9 @@ public class ConstInlineVisitor extends AbstractVisitor { } if (parentInsn.getType() == InsnType.CONSTRUCTOR) { // don't inline into anonymous call if it can be inlined later - ConstructorInsn ctrInsn = (ConstructorInsn) parentInsn; - MethodNode ctrMth = mth.root().getMethodUtils().resolveMethod(ctrInsn); - if (ctrMth != null && ctrMth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE)) { + MethodNode ctrMth = mth.root().getMethodUtils().resolveMethod((ConstructorInsn) parentInsn); + if (ctrMth != null + && (ctrMth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE) || ctrMth.contains(AFlag.ANONYMOUS_CONSTRUCTOR))) { return false; } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/variables/ProcessVariables.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/variables/ProcessVariables.java index b4fdd868a..586440417 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/variables/ProcessVariables.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/variables/ProcessVariables.java @@ -15,12 +15,12 @@ import org.slf4j.LoggerFactory; import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.nodes.DeclareVariablesAttr; -import jadx.core.dex.instructions.BaseInvokeNode; import jadx.core.dex.instructions.InsnType; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.CodeVar; import jadx.core.dex.instructions.args.RegisterArg; import jadx.core.dex.instructions.args.SSAVar; +import jadx.core.dex.instructions.mods.ConstructorInsn; import jadx.core.dex.nodes.IBlock; import jadx.core.dex.nodes.IContainer; import jadx.core.dex.nodes.IRegion; @@ -116,15 +116,26 @@ public class ProcessVariables extends AbstractVisitor { } private boolean isArgUnused(MethodNode mth, RegisterArg arg) { - if (arg.contains(AFlag.REMOVE) || arg.contains(AFlag.SKIP_ARG)) { + if (arg.contains(AFlag.REMOVE)) { return true; } + // check constructors for removed args InsnNode parentInsn = arg.getParentInsn(); - if (parentInsn instanceof BaseInvokeNode - && mth.root().getMethodUtils().isSkipArg(((BaseInvokeNode) parentInsn), arg)) { - arg.add(AFlag.DONT_GENERATE); - arg.add(AFlag.REMOVE); - return true; + if (parentInsn != null + && parentInsn.getType() == InsnType.CONSTRUCTOR + && parentInsn.contains(AType.METHOD_DETAILS)) { + MethodNode resolveMth = mth.root().getMethodUtils().resolveMethod(((ConstructorInsn) parentInsn)); + if (resolveMth != null && resolveMth.contains(AType.SKIP_MTH_ARGS)) { + int insnPos = parentInsn.getArgIndex(arg); + List mthArgs = resolveMth.getArgRegs(); + if (0 <= insnPos && insnPos < mthArgs.size()) { + RegisterArg mthArg = mthArgs.get(insnPos); + if (mthArg.contains(AFlag.REMOVE) && arg.sameType(mthArg)) { + arg.add(AFlag.DONT_GENERATE); + return true; + } + } + } } return false; } diff --git a/jadx-core/src/test/java/jadx/tests/external/BaseExternalTest.java b/jadx-core/src/test/java/jadx/tests/external/BaseExternalTest.java index d45a23481..54d35dd3d 100644 --- a/jadx-core/src/test/java/jadx/tests/external/BaseExternalTest.java +++ b/jadx-core/src/test/java/jadx/tests/external/BaseExternalTest.java @@ -1,6 +1,7 @@ package jadx.tests.external; import java.io.File; +import java.util.function.BiFunction; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -15,6 +16,7 @@ import jadx.api.JadxDecompiler; import jadx.api.JadxInternalAccess; import jadx.api.metadata.ICodeAnnotation; import jadx.api.metadata.ICodeNodeRef; +import jadx.api.metadata.annotations.NodeDeclareRef; import jadx.core.dex.nodes.ClassNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.nodes.RootNode; @@ -148,26 +150,39 @@ public abstract class BaseExternalTest extends TestUtils { private String cutMethodCode(ICodeInfo codeInfo, MethodNode mth) { int startPos = getCommentStartPos(codeInfo, mth.getDefPosition()); - int stopPos = getNextNodePos(mth, codeInfo); + int stopPos = getMethodEnd(mth, codeInfo); return codeInfo.getCodeStr().substring(startPos, stopPos); } - private int getNextNodePos(MethodNode mth, ICodeInfo codeInfo) { - int pos = mth.getDefPosition() + 1; - while (true) { - ICodeNodeRef nodeBelow = codeInfo.getCodeMetadata().getNodeBelow(pos); - if (nodeBelow == null) { - return codeInfo.getCodeStr().length(); + private int getMethodEnd(MethodNode mth, ICodeInfo codeInfo) { + // skip nested nodes DEF/END until first unpaired END annotation (end of this method) + Integer end = codeInfo.getCodeMetadata().searchDown(mth.getDefPosition() + 1, new BiFunction<>() { + int nested = 0; + + @Override + public Integer apply(Integer pos, ICodeAnnotation ann) { + switch (ann.getAnnType()) { + case DECLARATION: + ICodeNodeRef node = ((NodeDeclareRef) ann).getNode(); + switch (node.getAnnType()) { + case CLASS: + case METHOD: + nested++; + break; + } + break; + + case END: + if (nested == 0) { + return pos; + } + nested--; + break; + } + return null; } - if (nodeBelow.getAnnType() != ICodeAnnotation.AnnType.METHOD) { - return nodeBelow.getDefPosition(); - } - MethodNode nodeMth = (MethodNode) nodeBelow; - if (nodeMth.getParentClass().equals(mth.getParentClass())) { // skip methods from anonymous classes - return getCommentStartPos(codeInfo, nodeMth.getDefPosition()); - } - pos = nodeMth.getDefPosition() + 1; - } + }); + return end != null ? end : codeInfo.getCodeStr().length(); } protected int getCommentStartPos(ICodeInfo codeInfo, int pos) { diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions3.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions3.java index 71f589188..f49315025 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions3.java +++ b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions3.java @@ -5,12 +5,9 @@ import java.util.regex.Pattern; import org.junit.jupiter.api.Test; -import jadx.core.dex.nodes.ClassNode; import jadx.tests.api.IntegrationTest; -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.not; -import static org.hamcrest.MatcherAssert.assertThat; +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; public class TestConditions3 extends IntegrationTest { @@ -59,12 +56,10 @@ public class TestConditions3 extends IntegrationTest { @Test public void test() { - ClassNode cls = getClassNode(TestCls.class); - String code = cls.getCode().toString(); - - assertThat(code, containsString("return null;")); - assertThat(code, not(containsString("else"))); - - assertThat(code, not(containsString("AnonymousClass_1"))); + assertThat(getClassNode(TestCls.class)) + .code() + .contains("return null;") + .doesNotContain("else") + .doesNotContain("AnonymousClass_1"); } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass21.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass21.java new file mode 100644 index 000000000..7a956eb25 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass21.java @@ -0,0 +1,35 @@ +package jadx.tests.integration.inner; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +/** + * Don't inline const string into anonymous class constructor + */ +public class TestAnonymousClass21 extends IntegrationTest { + + @SuppressWarnings("Convert2Lambda") + public static class TestCls { + public void test() { + String str = "str"; + new Thread(new Runnable() { + @Override + public void run() { + System.out.println(str); + } + }).start(); + } + } + + @Test + public void test() { + noDebugInfo(); + assertThat(getClassNode(TestCls.class)) + .code() + .containsOne("String str = \"str\";") + .containsOne("System.out.println(str);"); + } +}