From 5f3c8816a3599d93de30bb4e9ad7fb8101cf23bd Mon Sep 17 00:00:00 2001 From: Skylot Date: Sat, 26 Feb 2022 15:42:09 +0000 Subject: [PATCH] fix: allow zero skips for restore new filled array --- .../jadx/core/dex/visitors/ReSugarCode.java | 109 ++++++++++-------- .../integration/arrays/TestArrayFill3.java | 23 ++-- 2 files changed, 69 insertions(+), 63 deletions(-) diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ReSugarCode.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ReSugarCode.java index e39de1b1..c88be6c9 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ReSugarCode.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ReSugarCode.java @@ -1,10 +1,10 @@ package jadx.core.dex.visitors; -import java.util.Comparator; import java.util.HashSet; import java.util.List; -import java.util.Objects; -import java.util.stream.Collectors; +import java.util.Map; +import java.util.SortedMap; +import java.util.TreeMap; import org.jetbrains.annotations.Nullable; @@ -34,7 +34,6 @@ import jadx.core.dex.visitors.shrink.CodeShrinkVisitor; import jadx.core.utils.InsnList; import jadx.core.utils.InsnRemover; import jadx.core.utils.InsnUtils; -import jadx.core.utils.Utils; import jadx.core.utils.exceptions.JadxException; @JadxVisitor( @@ -87,7 +86,7 @@ public class ReSugarCode extends AbstractVisitor { } switch (insn.getType()) { case NEW_ARRAY: - return processNewArray(mth, (NewArrayNode) insn, instructions, i, remover); + return processNewArray(mth, (NewArrayNode) insn, instructions, remover); case SWITCH: return processEnumSwitch(mth, (SwitchInsn) insn); @@ -100,8 +99,7 @@ public class ReSugarCode extends AbstractVisitor { /** * Replace new-array and sequence of array-put to new filled-array instruction. */ - private static boolean processNewArray(MethodNode mth, NewArrayNode newArrayInsn, - List instructions, int i, InsnRemover remover) { + private static boolean processNewArray(MethodNode mth, NewArrayNode newArrayInsn, List instructions, InsnRemover remover) { Object arrayLenConst = InsnUtils.getConstValueByArg(mth.root(), newArrayInsn.getArg(0)); if (!(arrayLenConst instanceof LiteralArg)) { return false; @@ -110,50 +108,81 @@ public class ReSugarCode extends AbstractVisitor { if (len == 0) { return false; } + ArgType arrType = newArrayInsn.getArrayType(); + ArgType elemType = arrType.getArrayElement(); + boolean allowMissingKeys = arrType.getArrayDimension() == 1 && elemType.isPrimitive(); + int minLen = allowMissingKeys ? len / 2 : len; + RegisterArg arrArg = newArrayInsn.getResult(); List useList = arrArg.getSVar().getUseList(); - if (useList.size() < len) { + if (useList.size() < minLen) { return false; } - List arrPuts = useList.stream() - .map(InsnArg::getParentInsn) - .filter(Objects::nonNull) - .filter(insn -> insn.getType() == InsnType.APUT) - .sorted(Comparator.comparingLong(insn -> { - Object constVal = InsnUtils.getConstValueByArg(mth.root(), insn.getArg(1)); - if (constVal instanceof LiteralArg) { - return ((LiteralArg) constVal).getLiteral(); - } - return -1; // bad value, put at top to fail fast next check - })) - .collect(Collectors.toList()); - if (arrPuts.size() != len) { - return false; + // quick check if APUT is used + boolean foundPut = false; + for (RegisterArg registerArg : useList) { + InsnNode parentInsn = registerArg.getParentInsn(); + if (parentInsn != null && parentInsn.getType() == InsnType.APUT) { + foundPut = true; + break; + } } - // expect all puts to be in same block - if (!new HashSet<>(instructions).containsAll(arrPuts)) { + if (!foundPut) { return false; } - - for (int j = 0; j < len; j++) { - InsnNode insn = arrPuts.get(j); - if (!checkPutInsn(mth, insn, arrArg, j)) { + // collect put instructions sorted by array index + SortedMap arrPuts = new TreeMap<>(); + for (RegisterArg registerArg : useList) { + InsnNode parentInsn = registerArg.getParentInsn(); + if (parentInsn == null || parentInsn.getType() != InsnType.APUT) { + continue; + } + if (!arrArg.sameRegAndSVar(parentInsn.getArg(0))) { return false; } + Object constVal = InsnUtils.getConstValueByArg(mth.root(), parentInsn.getArg(1)); + if (!(constVal instanceof LiteralArg)) { + return false; + } + long index = ((LiteralArg) constVal).getLiteral(); + if (index >= len) { + return false; + } + if (arrPuts.containsKey(index)) { + // stop on index rewrite + break; + } + arrPuts.put(index, parentInsn); + } + if (arrPuts.size() < minLen) { + return false; + } + // expect all puts to be in same block + if (!new HashSet<>(instructions).containsAll(arrPuts.values())) { + return false; } // checks complete, apply - ArgType arrType = newArrayInsn.getArrayType(); - InsnNode filledArr = new FilledNewArrayNode(arrType.getArrayElement(), len); + InsnNode filledArr = new FilledNewArrayNode(elemType, len); filledArr.setResult(arrArg.duplicate()); - for (InsnNode put : arrPuts) { + long prevIndex = -1; + for (Map.Entry entry : arrPuts.entrySet()) { + long index = entry.getKey(); + if (index != prevIndex) { + // use zero for missing keys + for (long i = prevIndex + 1; i < index; i++) { + filledArr.addArg(InsnArg.lit(0, elemType)); + } + } + InsnNode put = entry.getValue(); filledArr.addArg(replaceConstInArg(mth, put.getArg(2))); remover.addAndUnbind(put); + prevIndex = index; } remover.addAndUnbind(newArrayInsn); - InsnNode lastPut = Utils.last(arrPuts); + InsnNode lastPut = arrPuts.get(arrPuts.lastKey()); int replaceIndex = InsnList.getIndex(instructions, lastPut); instructions.set(replaceIndex, filledArr); return true; @@ -172,22 +201,6 @@ public class ReSugarCode extends AbstractVisitor { return valueArg.duplicate(); } - private static boolean checkPutInsn(MethodNode mth, InsnNode insn, RegisterArg arrArg, int putIndex) { - if (insn == null || insn.getType() != InsnType.APUT) { - return false; - } - if (!arrArg.sameRegAndSVar(insn.getArg(0))) { - return false; - } - InsnArg indexArg = insn.getArg(1); - Object value = InsnUtils.getConstValueByArg(mth.root(), indexArg); - if (value instanceof LiteralArg) { - int index = (int) ((LiteralArg) value).getLiteral(); - return index == putIndex; - } - return false; - } - private static boolean processEnumSwitch(MethodNode mth, SwitchInsn insn) { InsnArg arg = insn.getArg(0); if (!arg.isInsnWrap()) { diff --git a/jadx-core/src/test/java/jadx/tests/integration/arrays/TestArrayFill3.java b/jadx-core/src/test/java/jadx/tests/integration/arrays/TestArrayFill3.java index e2b9791a..3defe76c 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/arrays/TestArrayFill3.java +++ b/jadx-core/src/test/java/jadx/tests/integration/arrays/TestArrayFill3.java @@ -1,30 +1,23 @@ package jadx.tests.integration.arrays; -import org.junit.jupiter.api.Test; - -import jadx.NotYetImplemented; -import jadx.core.dex.nodes.ClassNode; import jadx.tests.api.IntegrationTest; +import jadx.tests.api.extensions.profiles.TestProfile; +import jadx.tests.api.extensions.profiles.TestWithProfiles; -import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.MatcherAssert.assertThat; +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; public class TestArrayFill3 extends IntegrationTest { public static class TestCls { - - public byte[] test(int a) { + public byte[] test() { return new byte[] { 0, 1, 2 }; } } - @Test - @NotYetImplemented + @TestWithProfiles({ TestProfile.ECJ_J8, TestProfile.ECJ_DX_J8 }) public void test() { - useEclipseCompiler(); - ClassNode cls = getClassNode(TestCls.class); - String code = cls.getCode().toString(); - - assertThat(code, containsString("return new byte[]{0, 1, 2}")); + assertThat(getClassNode(TestCls.class)) + .code() + .containsOne("return new byte[]{0, 1, 2}"); } } -- GitLab