From eed65421ea597a0e75b45f8c616b048d31155ec6 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sat, 7 Mar 2015 20:09:51 +0300 Subject: [PATCH] core: fix incorrect argument removing in anonymous constructor, inline synthetic field increment method --- .../jadx/core/dex/visitors/ClassModifier.java | 5 +- .../dex/visitors/MethodInlineVisitor.java | 50 ++++++++--- .../inner/TestAnonymousClass5.java | 85 +++++++++++++++++++ 3 files changed, 127 insertions(+), 13 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass5.java 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 71e7873ce..291bcac9c 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 @@ -53,7 +53,8 @@ public class ClassModifier extends AbstractVisitor { ClassNode fieldsCls = cls.dex().resolveClass(ClassInfo.fromType(field.getType())); ClassInfo parentClass = cls.getClassInfo().getParentClass(); if (fieldsCls != null - && parentClass.equals(fieldsCls.getClassInfo())) { + && parentClass.equals(fieldsCls.getClassInfo()) + && field.getName().startsWith("this$") /* TODO: don't check name */) { int found = 0; for (MethodNode mth : cls.getMethods()) { if (removeFieldUsageFromConstructor(mth, field, fieldsCls)) { @@ -75,7 +76,7 @@ public class ClassModifier extends AbstractVisitor { return false; } List args = mth.getArguments(false); - if (args.isEmpty()) { + if (args.isEmpty() || mth.contains(AFlag.SKIP_FIRST_ARG)) { return false; } RegisterArg arg = args.get(0); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/MethodInlineVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/MethodInlineVisitor.java index 4b7e1d64d..7015488f1 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/MethodInlineVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/MethodInlineVisitor.java @@ -3,11 +3,17 @@ package jadx.core.dex.visitors; import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.nodes.MethodInlineAttr; import jadx.core.dex.info.AccessInfo; +import jadx.core.dex.instructions.InsnType; +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.BlockNode; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.utils.exceptions.JadxException; +import java.util.List; + /** * Inline synthetic methods. */ @@ -19,26 +25,48 @@ public class MethodInlineVisitor extends AbstractVisitor { if (accessFlags.isSynthetic() && accessFlags.isStatic() && mth.getBasicBlocks().size() == 2) { - BlockNode block = mth.getBasicBlocks().get(1); - if (block.getInstructions().isEmpty() - || block.contains(AFlag.RETURN)) { - inlineMth(mth); + BlockNode returnBlock = mth.getBasicBlocks().get(1); + if (returnBlock.contains(AFlag.RETURN) || returnBlock.getInstructions().isEmpty()) { + BlockNode firstBlock = mth.getBasicBlocks().get(0); + inlineMth(mth, firstBlock, returnBlock); } } } - private static void inlineMth(MethodNode mth) { - BlockNode firstBlock = mth.getBasicBlocks().get(0); - if (firstBlock.getInstructions().isEmpty()) { + private static void inlineMth(MethodNode mth, BlockNode firstBlock, BlockNode returnBlock) { + List insnList = firstBlock.getInstructions(); + if (insnList.isEmpty()) { // synthetic field getter BlockNode block = mth.getBasicBlocks().get(1); InsnNode insn = block.getInstructions().get(0); // set arg from 'return' instruction addInlineAttr(mth, InsnNode.wrapArg(insn.getArg(0))); - } else { - // synthetic field setter or method invoke - if (firstBlock.getInstructions().size() == 1) { - addInlineAttr(mth, firstBlock.getInstructions().get(0)); + return; + } + // synthetic field setter or method invoke + if (insnList.size() == 1) { + addInlineAttr(mth, insnList.get(0)); + return; + } + // other field operations + if (insnList.size() == 2 + && returnBlock.getInstructions().size() == 1 + && !mth.getReturnType().equals(ArgType.VOID)) { + InsnNode get = insnList.get(0); + InsnNode put = insnList.get(1); + InsnArg retArg = returnBlock.getInstructions().get(0).getArg(0); + if (get.getType() == InsnType.IGET + && put.getType() == InsnType.IPUT + && retArg.isRegister() + && get.getResult().equalRegisterAndType((RegisterArg) retArg)) { + RegisterArg retReg = (RegisterArg) retArg; + retReg.getSVar().removeUse(retReg); + CodeShrinker.shrinkMethod(mth); + + insnList = firstBlock.getInstructions(); + if (insnList.size() == 1) { + addInlineAttr(mth, insnList.get(0)); + } } } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass5.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass5.java new file mode 100644 index 000000000..e4d4ddcd3 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass5.java @@ -0,0 +1,85 @@ +package jadx.tests.integration.inner; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; + +import org.junit.Test; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.Assert.assertThat; + +public class TestAnonymousClass5 extends IntegrationTest { + + public static class TestCls { + private final Map map = new HashMap(); + private int a; + + public Iterable test(String name) { + final TestCls cls = map.get(name); + if (cls == null) { + return null; + } + final int listSize = cls.size(); + final Iterator iterator = new Iterator() { + int counter = 0; + + @Override + public TestCls next() { + cls.a++; + counter++; + return cls; + } + + @Override + public boolean hasNext() { + return counter < listSize; + } + + @Override + public void remove() { + throw new UnsupportedOperationException(); + } + }; + return new Iterable() { + @Override + public Iterator iterator() { + return iterator; + } + }; + } + + private int size() { + return 7; + } + + public void check() { + TestCls v = new TestCls(); + v.a = 3; + map.put("a", v); + Iterable it = test("a"); + TestCls next = it.iterator().next(); + assertThat(next, sameInstance(v)); + assertThat(next.a, is(4)); + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("map.get(name);")); + assertThat(code, not(containsString("access$008"))); + + // TODO +// assertThat(code, not(containsString("synthetic"))); + } +}