From b57001d4a7c6b35b7342518323f0373813b0e2e8 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sun, 10 Apr 2022 15:39:16 +0100 Subject: [PATCH] fix: use correct reference for replaced bridge constructor (#1441) --- .../main/java/jadx/core/codegen/InsnGen.java | 17 +++++++--- .../java/jadx/core/dex/attributes/AType.java | 4 ++- .../attributes/nodes/MethodReplaceAttr.java | 31 +++++++++++++++++++ .../jadx/core/dex/visitors/ClassModifier.java | 13 ++++++-- .../dex/visitors/ProcessMethodsForInline.java | 7 +++-- .../dex/visitors/usage/UsageInfoVisitor.java | 11 +++++++ .../java/jadx/tests/api/IntegrationTest.java | 3 +- .../assertj/JadxClassNodeAssertions.java | 23 ++++++++++++++ .../inner/TestAnonymousClass14.java | 29 ++++++++++++----- .../inner/TestOuterConstructorCall.java | 20 ++++++------ 10 files changed, 128 insertions(+), 30 deletions(-) create mode 100644 jadx-core/src/main/java/jadx/core/dex/attributes/nodes/MethodReplaceAttr.java diff --git a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java index e65ed366..fa847dca 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java @@ -20,6 +20,7 @@ import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.nodes.FieldReplaceAttr; import jadx.core.dex.attributes.nodes.GenericInfoAttr; import jadx.core.dex.attributes.nodes.LoopLabelAttr; +import jadx.core.dex.attributes.nodes.MethodReplaceAttr; import jadx.core.dex.attributes.nodes.SkipMethodArgsAttr; import jadx.core.dex.info.ClassInfo; import jadx.core.dex.info.FieldInfo; @@ -694,19 +695,27 @@ public class InsnGen { throw new JadxRuntimeException("Constructor 'self' invoke must be removed!"); } MethodNode callMth = mth.root().resolveMethod(insn.getCallMth()); + MethodNode refMth = callMth; + if (callMth != null) { + MethodReplaceAttr replaceAttr = callMth.get(AType.METHOD_REPLACE); + if (replaceAttr != null) { + refMth = replaceAttr.getReplaceMth(); + } + } + if (insn.isSuper()) { - code.attachAnnotation(callMth); + code.attachAnnotation(refMth); code.add("super"); } else if (insn.isThis()) { - code.attachAnnotation(callMth); + code.attachAnnotation(refMth); code.add("this"); } else { code.add("new "); - if (callMth == null || callMth.contains(AFlag.DONT_GENERATE)) { + if (refMth == null || refMth.contains(AFlag.DONT_GENERATE)) { // use class reference if constructor method is missing (default constructor) code.attachAnnotation(mth.root().resolveClass(insn.getCallMth().getDeclClass())); } else { - code.attachAnnotation(callMth); + code.attachAnnotation(refMth); } mgen.getClassGen().addClsName(code, insn.getClassType()); GenericInfoAttr genericInfoAttr = insn.get(AType.GENERIC_INFO); diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/AType.java b/jadx-core/src/main/java/jadx/core/dex/attributes/AType.java index 25968061..85c20f61 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/AType.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/AType.java @@ -21,6 +21,7 @@ import jadx.core.dex.attributes.nodes.LoopLabelAttr; import jadx.core.dex.attributes.nodes.MethodBridgeAttr; import jadx.core.dex.attributes.nodes.MethodInlineAttr; import jadx.core.dex.attributes.nodes.MethodOverrideAttr; +import jadx.core.dex.attributes.nodes.MethodReplaceAttr; import jadx.core.dex.attributes.nodes.MethodTypeVarsAttr; import jadx.core.dex.attributes.nodes.PhiListAttr; import jadx.core.dex.attributes.nodes.RegDebugInfoAttr; @@ -65,11 +66,12 @@ public final class AType implements IJadxAttrType { // method public static final AType LOCAL_VARS_DEBUG_INFO = new AType<>(); public static final AType METHOD_INLINE = new AType<>(); + public static final AType METHOD_REPLACE = new AType<>(); + public static final AType BRIDGED_BY = new AType<>(); public static final AType SKIP_MTH_ARGS = new AType<>(); public static final AType METHOD_OVERRIDE = new AType<>(); public static final AType METHOD_TYPE_VARS = new AType<>(); public static final AType> TRY_BLOCKS_LIST = new AType<>(); - public static final AType BRIDGED_BY = new AType<>(); // region public static final AType DECLARE_VARIABLES = new AType<>(); diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/MethodReplaceAttr.java b/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/MethodReplaceAttr.java new file mode 100644 index 00000000..f3d6b8ab --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/MethodReplaceAttr.java @@ -0,0 +1,31 @@ +package jadx.core.dex.attributes.nodes; + +import jadx.api.plugins.input.data.attributes.PinnedAttribute; +import jadx.core.dex.attributes.AType; +import jadx.core.dex.nodes.MethodNode; + +/** + * Calls of method should be replaced by provided method (used for synthetic methods redirect) + */ +public class MethodReplaceAttr extends PinnedAttribute { + + private final MethodNode replaceMth; + + public MethodReplaceAttr(MethodNode replaceMth) { + this.replaceMth = replaceMth; + } + + public MethodNode getReplaceMth() { + return replaceMth; + } + + @Override + public AType getAttrType() { + return AType.METHOD_REPLACE; + } + + @Override + public String toString() { + return "REPLACED_BY: " + replaceMth; + } +} 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 09e213b3..f7e22800 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 @@ -11,6 +11,7 @@ import jadx.api.plugins.input.data.AccessFlags; import jadx.core.Consts; import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.nodes.FieldReplaceAttr; +import jadx.core.dex.attributes.nodes.MethodReplaceAttr; import jadx.core.dex.attributes.nodes.SkipMethodArgsAttr; import jadx.core.dex.info.AccessInfo; import jadx.core.dex.info.ClassInfo; @@ -31,6 +32,7 @@ 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.visitors.usage.UsageInfoVisitor; import jadx.core.utils.BlockUtils; import jadx.core.utils.InsnRemover; import jadx.core.utils.exceptions.JadxException; @@ -155,7 +157,7 @@ public class ClassModifier extends AbstractVisitor { return; } // remove synthetic constructor for inner classes - if (af.isConstructor()) { + if (mth.isConstructor() && mth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE)) { InsnNode insn = BlockUtils.getOnlyOneInsnFromMth(mth); if (insn != null) { List args = mth.getArgRegs(); @@ -210,7 +212,14 @@ public class ClassModifier extends AbstractVisitor { SkipMethodArgsAttr.skipArg(mth, i); } } - mth.add(AFlag.DONT_GENERATE); + MethodInfo callMth = constr.getCallMth(); + MethodNode callMthNode = cls.root().resolveMethod(callMth); + if (callMthNode != null) { + mth.addAttr(new MethodReplaceAttr(callMthNode)); + mth.add(AFlag.DONT_GENERATE); + // code generation order should be already fixed for marked methods + UsageInfoVisitor.replaceMethodUsage(callMthNode, mth); + } } } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ProcessMethodsForInline.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ProcessMethodsForInline.java index 884ea329..d0bbde5f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ProcessMethodsForInline.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ProcessMethodsForInline.java @@ -45,16 +45,17 @@ public class ProcessMethodsForInline extends AbstractVisitor { } AccessInfo accessFlags = mth.getAccessFlags(); boolean isSynthetic = accessFlags.isSynthetic() || mth.getName().contains("$"); - return isSynthetic && accessFlags.isStatic(); + return isSynthetic && (accessFlags.isStatic() || mth.isConstructor()); } private static void fixClassDependencies(MethodNode mth) { ClassNode parentClass = mth.getTopParentClass(); for (MethodNode useInMth : mth.getUseIn()) { - // remove possible cross dependency to force class with inline method to be processed before its - // usage + // remove possible cross dependency + // to force class with inline method to be processed before its usage ClassNode useTopCls = useInMth.getTopParentClass(); parentClass.setDependencies(ListUtils.safeRemoveAndTrim(parentClass.getDependencies(), useTopCls)); + useTopCls.addCodegenDep(parentClass); } } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/usage/UsageInfoVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/usage/UsageInfoVisitor.java index a868b2f7..0278129f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/usage/UsageInfoVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/usage/UsageInfoVisitor.java @@ -1,5 +1,8 @@ package jadx.core.dex.visitors.usage; +import java.util.Collections; +import java.util.List; + import jadx.api.plugins.input.data.ICallSite; import jadx.api.plugins.input.data.ICodeReader; import jadx.api.plugins.input.data.IMethodHandle; @@ -18,6 +21,7 @@ import jadx.core.dex.visitors.AbstractVisitor; import jadx.core.dex.visitors.JadxVisitor; import jadx.core.dex.visitors.OverrideMethodVisitor; import jadx.core.dex.visitors.rename.RenameVisitor; +import jadx.core.utils.ListUtils; import jadx.core.utils.input.InsnDataUtils; @JadxVisitor( @@ -134,4 +138,11 @@ public class UsageInfoVisitor extends AbstractVisitor { } } } + + public static void replaceMethodUsage(MethodNode mergeIntoMth, MethodNode sourceMth) { + List mergedUsage = ListUtils.distinctMergeSortedLists(mergeIntoMth.getUseIn(), sourceMth.getUseIn()); + mergedUsage.remove(sourceMth); + mergeIntoMth.setUseIn(mergedUsage); + sourceMth.setUseIn(Collections.emptyList()); + } } diff --git a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java index 995a2d2d..582b3b09 100644 --- a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java +++ b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java @@ -24,6 +24,7 @@ import java.util.jar.JarOutputStream; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assumptions; @@ -219,7 +220,7 @@ public abstract class IntegrationTest extends TestUtils { return sortedClsNodes; } - @Nullable + @NotNull public ClassNode searchCls(List list, String clsName) { for (ClassNode cls : list) { if (cls.getClassInfo().getFullName().equals(clsName)) { diff --git a/jadx-core/src/test/java/jadx/tests/api/utils/assertj/JadxClassNodeAssertions.java b/jadx-core/src/test/java/jadx/tests/api/utils/assertj/JadxClassNodeAssertions.java index 60ad1858..e1cec9ef 100644 --- a/jadx-core/src/test/java/jadx/tests/api/utils/assertj/JadxClassNodeAssertions.java +++ b/jadx-core/src/test/java/jadx/tests/api/utils/assertj/JadxClassNodeAssertions.java @@ -1,13 +1,18 @@ package jadx.tests.api.utils.assertj; +import java.util.Map; + import org.assertj.core.api.AbstractObjectAssert; import org.assertj.core.api.Assertions; +import jadx.api.CodePosition; import jadx.api.ICodeInfo; import jadx.core.dex.nodes.ClassNode; +import jadx.core.dex.nodes.ICodeNode; import jadx.tests.api.IntegrationTest; import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; +import static org.assertj.core.api.Assertions.fail; public class JadxClassNodeAssertions extends AbstractObjectAssert { public JadxClassNodeAssertions(ClassNode cls) { @@ -52,4 +57,22 @@ public class JadxClassNodeAssertions extends AbstractObjectAssert entry : code.getAnnotations().entrySet()) { + if (entry.getKey().getPos() == refPos) { + Assertions.assertThat(entry.getValue()).isEqualTo(node); + return; + } + } + fail("Annotation for reference string: '%s' at position %d not found", refStr, refPos); + } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass14.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass14.java index f13332a2..4b53a705 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass14.java +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass14.java @@ -2,12 +2,13 @@ package jadx.tests.integration.inner; import org.junit.jupiter.api.Test; +import jadx.api.CommentsLevel; import jadx.core.dex.nodes.ClassNode; +import jadx.core.dex.nodes.MethodNode; +import jadx.core.utils.ListUtils; import jadx.tests.api.SmaliTest; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; public class TestAnonymousClass14 extends SmaliTest { // @formatter:off @@ -44,11 +45,23 @@ public class TestAnonymousClass14 extends SmaliTest { @Test public void test() { - ClassNode clsNode = getClassNodeFromSmaliFiles("inner", "TestAnonymousClass14", "OuterCls"); - String code = clsNode.getCode().toString(); - code = code.replaceAll("/\\*.*?\\*/", ""); // remove block comments + getArgs().setCommentsLevel(CommentsLevel.WARN); + ClassNode outerCls = getClassNodeFromSmaliFiles("OuterCls"); + assertThat(outerCls).code() + .doesNotContain("synthetic", "AnonymousClass1") + .describedAs("only one constructor").containsOne("private TestCls(") + .describedAs("constructor without args").containsOne("private TestCls() {"); - assertThat(code, not(containsString("AnonymousClass1"))); - assertThat(code, not(containsString("synthetic"))); + MethodNode makeTestClsMth = outerCls.searchMethodByShortName("makeTestCls"); + assertThat(makeTestClsMth).isNotNull(); + + ClassNode testCls = searchCls(outerCls.getInnerClasses(), "TestCls"); + MethodNode ctrMth = ListUtils.filterOnlyOne(testCls.getMethods(), + m -> m.isConstructor() && !m.getAccessFlags().isSynthetic()); + assertThat(ctrMth).isNotNull(); + assertThat(ctrMth.getUseIn()).hasSize(1); + assertThat(ctrMth.getUseIn().get(0)).isEqualTo(makeTestClsMth); + + assertThat(outerCls).checkCodeAnnotationFor("new TestCls();", 4, ctrMth); } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestOuterConstructorCall.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestOuterConstructorCall.java index 40c52567..89c557c7 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/inner/TestOuterConstructorCall.java +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestOuterConstructorCall.java @@ -2,22 +2,20 @@ package jadx.tests.integration.inner; import org.junit.jupiter.api.Test; -import jadx.core.dex.nodes.ClassNode; +import jadx.api.CommentsLevel; import jadx.tests.api.IntegrationTest; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; public class TestOuterConstructorCall extends IntegrationTest { + @SuppressWarnings({ "InnerClassMayBeStatic", "unused" }) public static class TestCls { private TestCls(Inner inner) { System.out.println(inner); } private class Inner { - @SuppressWarnings("unused") private TestCls test() { return new TestCls(this); } @@ -26,11 +24,11 @@ public class TestOuterConstructorCall extends IntegrationTest { @Test public void test() { - ClassNode cls = getClassNode(TestCls.class); - String code = cls.getCode().toString(); - - assertThat(code, containsString("private class Inner {")); - assertThat(code, containsString("return new TestOuterConstructorCall$TestCls(this);")); - assertThat(code, not(containsString("synthetic"))); + getArgs().setCommentsLevel(CommentsLevel.WARN); + assertThat(getClassNode(TestCls.class)) + .code() + .containsOne("class Inner {") + .containsOne("return new TestOuterConstructorCall$TestCls(this);") + .doesNotContain("synthetic", "this$0"); } } -- GitLab