fix: disable ternary mod for duplicated blocks (#2844)

This commit is contained in:
Skylot
2026-04-10 21:56:11 +01:00
parent f95c5e8f39
commit 3119e8c893
12 changed files with 140 additions and 41 deletions
@@ -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,
@@ -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);
}
}
@@ -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<BlockNode> conditionBlocks = ifRegion.getConditionBlocks();
if (conditionBlocks.isEmpty()) {
return false;
@@ -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;
}
@@ -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);
}
@@ -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<File> 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);
@@ -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();
@@ -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();
@@ -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<File> compileFiles(List<File> sourceFiles, Path outTmp) throws IOException {
@@ -74,23 +77,25 @@ public class TestCompiler implements Closeable {
List<String> 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<JavaFileObject> 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);
@@ -23,6 +23,11 @@ public enum TestProfile implements Consumer<IntegrationTest> {
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<IntegrationTest> {
test.useTargetJavaVersion(8);
test.useJavaInput();
}),
ALL("all", null);
ALL("all", test -> {
});
private final String description;
private final Consumer<IntegrationTest> setup;
@@ -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();
}
}
@@ -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;-><init>(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