From 3119e8c893f71d8da2eeaf9766996539dbe12a46 Mon Sep 17 00:00:00 2001 From: Skylot <118523+skylot@users.noreply.github.com> Date: Fri, 10 Apr 2026 21:56:11 +0100 Subject: [PATCH] fix: disable ternary mod for duplicated blocks (#2844) --- .../java/jadx/core/dex/attributes/AFlag.java | 6 +- .../core/dex/visitors/MoveInlineVisitor.java | 2 - .../core/dex/visitors/regions/TernaryMod.java | 3 + .../visitors/regions/maker/RegionMaker.java | 18 ++--- .../java/jadx/core/utils/InsnRemover.java | 3 + .../java/jadx/tests/api/IntegrationTest.java | 4 +- .../test/java/jadx/tests/api/RaungTest.java | 2 +- .../test/java/jadx/tests/api/SmaliTest.java | 1 + .../jadx/tests/api/compiler/TestCompiler.java | 30 ++++---- .../api/extensions/profiles/TestProfile.java | 8 ++- .../conditions/TestConditions22.java | 32 ++++++--- .../smali/conditions/TestConditions22.smali | 72 +++++++++++++++++++ 12 files changed, 140 insertions(+), 41 deletions(-) create mode 100644 jadx-core/src/test/smali/conditions/TestConditions22.smali 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 e06aceedf..ae47bf15c 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 @@ -31,6 +31,7 @@ public enum AFlag { FORCE_RAW_NAME, // force use of raw name instead alias ADDED_TO_REGION, + DUPLICATED, // this loop condition has been merged or otherwise shouldn't be subject to the 1 instruction limit ALLOW_MULTIPLE_INSNS_LOOP_COND, @@ -72,11 +73,6 @@ public enum AFlag { */ FORCE_ASSIGN_INLINE, - /** - * A MOVE instruction has been inlined - */ - MOVE_INLINED, - CUSTOM_DECLARE, // variable for this register don't need declaration DECLARE_VAR, diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java index 740fd4387..c06dbd21a 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java @@ -2,7 +2,6 @@ package jadx.core.dex.visitors; import java.util.ArrayList; -import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.nodes.RegDebugInfoAttr; import jadx.core.dex.instructions.InsnType; @@ -40,7 +39,6 @@ public class MoveInlineVisitor extends AbstractVisitor { continue; } if (processMove(mth, insn)) { - block.add(AFlag.MOVE_INLINED); remover.addAndUnbind(insn); } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java index a89638126..a66a19592 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java @@ -86,6 +86,9 @@ public class TernaryMod extends AbstractRegionVisitor implements IRegionIterativ if (tb == null || eb == null) { return false; } + if (tb.contains(AFlag.DUPLICATED) || eb.contains(AFlag.DUPLICATED)) { + return false; + } List conditionBlocks = ifRegion.getConditionBlocks(); if (conditionBlocks.isEmpty()) { return false; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/RegionMaker.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/RegionMaker.java index bf9e00c24..3f8ad9c0f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/RegionMaker.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/maker/RegionMaker.java @@ -4,6 +4,8 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import org.jetbrains.annotations.Nullable; + import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.nodes.EdgeInsnAttr; @@ -55,20 +57,20 @@ public class RegionMaker { insertEdgeInsns(region, startBlock); return region; } - if (processedBlocks.addChecked(startBlock)) { - mth.addWarnComment("Found duplicated region for block: " + startBlock + ' ' + startBlock.getAttributesString()); - // Add block to multiple regions (duplicate the instructions in decompiled code) and allow - // processing to continue + // Add block to multiple regions (duplicate the instructions in decompiled code) + // and allow processing to continue + if (!startBlock.contains(AFlag.DUPLICATED)) { + mth.addWarnComment("Code duplicated, block: " + startBlock + ' ' + startBlock.getAttributesString()); + startBlock.add(AFlag.DUPLICATED); + } } - BlockNode next = startBlock; - while (next != null) { next = traverse(region, next); regionsCount++; if (regionsCount > regionsLimit) { - throw new JadxOverflowException("Regions count limit reached at block " + startBlock.toString()); + throw new JadxOverflowException("Regions count limit reached at block " + startBlock); } } return region; @@ -77,7 +79,7 @@ public class RegionMaker { /** * Recursively traverse all blocks from 'block' until block from 'exits' */ - private BlockNode traverse(IRegion r, BlockNode block) { + private @Nullable BlockNode traverse(IRegion r, BlockNode block) { if (block.contains(AFlag.MTH_EXIT_BLOCK)) { return null; } diff --git a/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java b/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java index 321141967..729e05df4 100644 --- a/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java +++ b/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java @@ -220,6 +220,9 @@ public class InsnRemover { } public static void remove(MethodNode mth, BlockNode block, InsnNode insn) { + if (block.contains(AFlag.DUPLICATED)) { + mth.addWarnComment("Instruction removed from duplicated block: " + block + ", please report this as an issue"); + } unbindInsn(mth, insn); removeWithoutUnbind(mth, block, insn); } 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 dd23a7fb9..0ab59fb66 100644 --- a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java +++ b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java @@ -125,7 +125,7 @@ public abstract class IntegrationTest extends TestUtils { /** * Run check method on decompiled code even if source check method not found. - * Useful for smali test if check method added to smali code + * Useful for Smali tests if check method added into Smali code */ private boolean forceDecompiledCheck = false; @@ -263,7 +263,7 @@ public abstract class IntegrationTest extends TestUtils { protected JadxDecompiler loadFiles(List inputFiles) { args.setInputFiles(inputFiles); boolean useDx = !isJavaInput(); - LOG.info(useDx ? "Using dex input" : "Using java input"); + LOG.info(useDx ? "Using dex input" : "Using java input, current java version: " + JavaUtils.JAVA_VERSION_INT); args.setUseDxInput(useDx); JadxDecompiler d = new JadxDecompiler(args); diff --git a/jadx-core/src/test/java/jadx/tests/api/RaungTest.java b/jadx-core/src/test/java/jadx/tests/api/RaungTest.java index 30ed06376..37e448033 100644 --- a/jadx-core/src/test/java/jadx/tests/api/RaungTest.java +++ b/jadx-core/src/test/java/jadx/tests/api/RaungTest.java @@ -13,7 +13,6 @@ import jadx.core.dex.nodes.ClassNode; import jadx.core.dex.nodes.RootNode; import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; -import static org.assertj.core.api.Assertions.assertThat; public abstract class RaungTest extends IntegrationTest { @@ -22,6 +21,7 @@ public abstract class RaungTest extends IntegrationTest { private static final String RAUNG_TESTS_EXT = ".raung"; @BeforeEach + @Override public void init() { super.init(); this.useJavaInput(); diff --git a/jadx-core/src/test/java/jadx/tests/api/SmaliTest.java b/jadx-core/src/test/java/jadx/tests/api/SmaliTest.java index 64fa8a1a0..d51297430 100644 --- a/jadx-core/src/test/java/jadx/tests/api/SmaliTest.java +++ b/jadx-core/src/test/java/jadx/tests/api/SmaliTest.java @@ -28,6 +28,7 @@ public abstract class SmaliTest extends IntegrationTest { } @BeforeEach + @Override public void init() { Assumptions.assumeFalse(USE_JAVA_INPUT, "skip smali test for java input tests"); super.init(); diff --git a/jadx-core/src/test/java/jadx/tests/api/compiler/TestCompiler.java b/jadx-core/src/test/java/jadx/tests/api/compiler/TestCompiler.java index 32092a63f..8ae1908a2 100644 --- a/jadx-core/src/test/java/jadx/tests/api/compiler/TestCompiler.java +++ b/jadx-core/src/test/java/jadx/tests/api/compiler/TestCompiler.java @@ -6,11 +6,13 @@ import java.io.IOException; import java.io.PrintWriter; import java.io.Writer; import java.lang.reflect.Method; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Locale; import javax.tools.DiagnosticListener; import javax.tools.JavaCompiler; @@ -19,8 +21,8 @@ import javax.tools.JavaFileObject; import javax.tools.ToolProvider; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; -import jadx.api.impl.SimpleCodeWriter; import jadx.core.dex.nodes.ClassNode; import jadx.core.utils.files.FileUtils; import jadx.tests.api.IntegrationTest; @@ -47,7 +49,8 @@ public class TestCompiler implements Closeable { throw new IllegalStateException("Can not find compiler, please use JDK instead"); } } - fileManager = new ClassFileManager(compiler.getStandardFileManager(null, null, null)); + fileManager = new ClassFileManager(compiler.getStandardFileManager( + System.out::println, Locale.ENGLISH, StandardCharsets.UTF_8)); } public List compileFiles(List sourceFiles, Path outTmp) throws IOException { @@ -74,23 +77,25 @@ public class TestCompiler implements Closeable { List arguments = new ArrayList<>(); arguments.add(options.isIncludeDebugInfo() ? "-g" : "-g:none"); int javaVersion = options.getJavaVersion(); - String javaVerStr = javaVersion <= 8 ? "1." + javaVersion : Integer.toString(javaVersion); - arguments.add("-source"); - arguments.add(javaVerStr); - arguments.add("-target"); - arguments.add(javaVerStr); + if (javaVersion != 0) { + String javaVerStr = javaVersion <= 8 ? "1." + javaVersion : Integer.toString(javaVersion); + arguments.add("-source"); + arguments.add(javaVerStr); + arguments.add("-target"); + arguments.add(javaVerStr); + } arguments.addAll(options.getArguments()); - SimpleCodeWriter output = new SimpleCodeWriter(); + StringBuilder sb = new StringBuilder(); DiagnosticListener diagnostic = diagObj -> { String msg = "Compiler diagnostic: " + diagObj; - output.startLine(msg); + sb.append('\n').append(msg); System.out.println(msg); }; Writer out = new PrintWriter(System.out); CompilationTask compilerTask = compiler.getTask(out, fileManager, diagnostic, arguments, null, jfObjects); if (Boolean.FALSE.equals(compilerTask.call())) { - throw new RuntimeException("Compilation failed: " + output); + throw new RuntimeException("Compilation failed: " + sb); } } @@ -102,12 +107,11 @@ public class TestCompiler implements Closeable { return getClassLoader().loadClass(clsFullName); } - @NotNull - public Method getMethod(Class cls, String methodName, Class[] types) throws NoSuchMethodException { + public @NotNull Method getMethod(Class cls, String methodName, Class[] types) throws NoSuchMethodException { return cls.getMethod(methodName, types); } - public Object invoke(String clsFullName, String methodName, Class[] types, Object[] args) { + public @Nullable Object invoke(String clsFullName, String methodName, Class[] types, Object[] args) { try { for (Class type : types) { checkType(type); diff --git a/jadx-core/src/test/java/jadx/tests/api/extensions/profiles/TestProfile.java b/jadx-core/src/test/java/jadx/tests/api/extensions/profiles/TestProfile.java index bdb965bb2..5a08f9588 100644 --- a/jadx-core/src/test/java/jadx/tests/api/extensions/profiles/TestProfile.java +++ b/jadx-core/src/test/java/jadx/tests/api/extensions/profiles/TestProfile.java @@ -23,6 +23,11 @@ public enum TestProfile implements Consumer { test.keepParentClassOnInput(); test.getArgs().getPluginOptions().put("java-convert.d8-desugar", "yes"); }), + JAVA("java", test -> { + // do not set java version, use default for current compiler + test.useTargetJavaVersion(0); + test.useJavaInput(); + }), JAVA8("java-8", test -> { test.useTargetJavaVersion(8); test.useJavaInput(); @@ -45,7 +50,8 @@ public enum TestProfile implements Consumer { test.useTargetJavaVersion(8); test.useJavaInput(); }), - ALL("all", null); + ALL("all", test -> { + }); private final String description; private final Consumer setup; diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions22.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions22.java index adb0bc118..83ece0383 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions22.java +++ b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions22.java @@ -1,12 +1,14 @@ package jadx.tests.integration.conditions; -import jadx.tests.api.IntegrationTest; +import org.junit.jupiter.api.Test; + +import jadx.tests.api.SmaliTest; import jadx.tests.api.extensions.profiles.TestProfile; import jadx.tests.api.extensions.profiles.TestWithProfiles; import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; -public class TestConditions22 extends IntegrationTest { +public class TestConditions22 extends SmaliTest { public static class TestCls { @@ -38,13 +40,17 @@ public class TestConditions22 extends IntegrationTest { } public void check() { - assertThat(test(1, 2)).isEqualTo(3); - assertThat(test(1, 1)).isEqualTo(0); - assertThat(test(2, 3)).isEqualTo(4); - assertThat(test(2, 2)).isEqualTo(0); - assertThat(test(3, 4)).isEqualTo(5); - assertThat(test(3, 3)).isEqualTo(0); - assertThat(test(4, 4)).isEqualTo(0); + verify(1, 2, 3); + verify(1, 1, 0); + verify(2, 3, 4); + verify(2, 2, 0); + verify(3, 4, 5); + verify(3, 3, 0); + verify(4, 4, 0); + } + + private static void verify(int a, int b, int result) { + assertThat(test(a, b)).isEqualTo(result); } } @@ -57,4 +63,12 @@ public class TestConditions22 extends IntegrationTest { .containsOne(indent(2) + "} else if (") .containsOne(indent(2) + "} else {"); } + + @Test + public void testSmali() { + allowWarnInCode(); // TODO: don't add 'duplicated region' warning for small and/or constant code + forceDecompiledCheck(); + assertThat(getClassNodeFromSmali()) + .code(); + } } diff --git a/jadx-core/src/test/smali/conditions/TestConditions22.smali b/jadx-core/src/test/smali/conditions/TestConditions22.smali new file mode 100644 index 000000000..8cc3012f1 --- /dev/null +++ b/jadx-core/src/test/smali/conditions/TestConditions22.smali @@ -0,0 +1,72 @@ +.class public Lconditions/TestConditions22; +.super Ljava/lang/Object; + +.method public static test(II)I + .registers 6 + + const/4 v0, 0x1 + const/4 v1, 0x2 + const/4 v2, 0x3 + const/4 v3, 0x0 + + if-ne p0, v0, :cond_b + if-ne p1, v1, :cond_9 + goto :goto_17 + + :cond_9 + move v2, v3 + goto :goto_17 + + :cond_b + const/4 v0, 0x4 + if-ne p0, v1, :cond_12 + if-ne p1, v2, :cond_9 + move v2, v0 + goto :goto_17 + + :cond_12 + if-ne p0, v2, :cond_9 + if-ne p1, v0, :cond_9 + const/4 v2, 0x5 + + .line 32 + :goto_17 + sget-object p0, Ljava/lang/System;->out:Ljava/io/PrintStream; + new-instance p1, Ljava/lang/StringBuilder; + const-string v0, "k = " + invoke-direct {p1, v0}, Ljava/lang/StringBuilder;->(Ljava/lang/String;)V + invoke-virtual {p1, v2}, Ljava/lang/StringBuilder;->append(I)Ljava/lang/StringBuilder; + move-result-object p1 + invoke-virtual {p1}, Ljava/lang/StringBuilder;->toString()Ljava/lang/String; + move-result-object p1 + invoke-virtual {p0, p1}, Ljava/io/PrintStream;->println(Ljava/lang/String;)V + return v2 +.end method + +.method private static verify(III)V + .registers 3 + invoke-static {p0, p1}, Lconditions/TestConditions22;->test(II)I + move-result p0 + invoke-static {p0}, Ljadx/tests/api/utils/assertj/JadxAssertions;->assertThat(I)Lorg/assertj/core/api/AbstractIntegerAssert; + move-result-object p0 + invoke-virtual {p0, p2}, Lorg/assertj/core/api/AbstractIntegerAssert;->isEqualTo(I)Lorg/assertj/core/api/AbstractIntegerAssert; + return-void +.end method + +.method public check()V + .registers 5 + const/4 v0, 0x1 + const/4 v1, 0x2 + const/4 v2, 0x3 + invoke-static {v0, v1, v2}, Lconditions/TestConditions22;->verify(III)V + const/4 v3, 0x0 + invoke-static {v0, v0, v3}, Lconditions/TestConditions22;->verify(III)V + const/4 v0, 0x4 + invoke-static {v1, v2, v0}, Lconditions/TestConditions22;->verify(III)V + invoke-static {v1, v1, v3}, Lconditions/TestConditions22;->verify(III)V + const/4 v1, 0x5 + invoke-static {v2, v0, v1}, Lconditions/TestConditions22;->verify(III)V + invoke-static {v2, v2, v3}, Lconditions/TestConditions22;->verify(III)V + invoke-static {v0, v0, v3}, Lconditions/TestConditions22;->verify(III)V + return-void +.end method