fix: improve const inlining in finally blocks (#917)
This commit is contained in:
@@ -99,35 +99,11 @@ public class ConstInlineVisitor extends AbstractVisitor {
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
if (checkForFinallyBlock(sVar)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// all check passed, run replace
|
||||
replaceConst(mth, insn, constArg, toRemove);
|
||||
}
|
||||
|
||||
private static boolean checkForFinallyBlock(SSAVar sVar) {
|
||||
List<SSAVar> ssaVars = sVar.getCodeVar().getSsaVars();
|
||||
if (ssaVars.size() <= 1) {
|
||||
return false;
|
||||
}
|
||||
int countInsns = 0;
|
||||
int countFinallyInsns = 0;
|
||||
for (SSAVar ssaVar : ssaVars) {
|
||||
for (RegisterArg reg : ssaVar.getUseList()) {
|
||||
InsnNode parentInsn = reg.getParentInsn();
|
||||
if (parentInsn != null) {
|
||||
countInsns++;
|
||||
if (parentInsn.contains(AFlag.FINALLY_INSNS)) {
|
||||
countFinallyInsns++;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return countFinallyInsns != 0 && countFinallyInsns != countInsns;
|
||||
}
|
||||
|
||||
/**
|
||||
* Don't inline null object
|
||||
*/
|
||||
@@ -180,10 +156,7 @@ public class ConstInlineVisitor extends AbstractVisitor {
|
||||
List<RegisterArg> useList = new ArrayList<>(ssaVar.getUseList());
|
||||
int replaceCount = 0;
|
||||
for (RegisterArg arg : useList) {
|
||||
if (arg.contains(AFlag.DONT_INLINE_CONST)) {
|
||||
continue;
|
||||
}
|
||||
if (replaceArg(mth, arg, constArg, constInsn, toRemove)) {
|
||||
if (canInline(arg) && replaceArg(mth, arg, constArg, constInsn, toRemove)) {
|
||||
replaceCount++;
|
||||
}
|
||||
}
|
||||
@@ -192,6 +165,20 @@ public class ConstInlineVisitor extends AbstractVisitor {
|
||||
}
|
||||
}
|
||||
|
||||
private static boolean canInline(RegisterArg arg) {
|
||||
if (arg.contains(AFlag.DONT_INLINE_CONST)) {
|
||||
return false;
|
||||
}
|
||||
InsnNode parentInsn = arg.getParentInsn();
|
||||
if (parentInsn == null) {
|
||||
return false;
|
||||
}
|
||||
if (parentInsn.contains(AFlag.DONT_GENERATE) || parentInsn.contains(AFlag.FINALLY_INSNS)) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
private static boolean replaceArg(MethodNode mth, RegisterArg arg, InsnArg constArg, InsnNode constInsn, List<InsnNode> toRemove) {
|
||||
InsnNode useInsn = arg.getParentInsn();
|
||||
if (useInsn == null) {
|
||||
|
||||
@@ -6,6 +6,7 @@ import java.util.LinkedHashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.function.Predicate;
|
||||
import java.util.stream.Collectors;
|
||||
import java.util.stream.Stream;
|
||||
|
||||
@@ -59,6 +60,17 @@ public class DebugUtils {
|
||||
};
|
||||
}
|
||||
|
||||
public static IDexTreeVisitor dumpRawVisitor(String desc, Predicate<MethodNode> filter) {
|
||||
return new AbstractVisitor() {
|
||||
@Override
|
||||
public void visit(MethodNode mth) {
|
||||
if (filter.test(mth)) {
|
||||
dumpRaw(mth, desc);
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
public static void dump(MethodNode mth, String desc) {
|
||||
File out = new File("test-graph-" + desc + "-tmp");
|
||||
DotGraphVisitor.dump().save(out, mth);
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
package jadx.tests.integration.trycatch;
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.io.InputStream;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import jadx.NotYetImplemented;
|
||||
import jadx.tests.api.SmaliTest;
|
||||
|
||||
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
|
||||
|
||||
public class TestFinally3 extends SmaliTest {
|
||||
|
||||
@SuppressWarnings({ "RedundantThrows", "unused" })
|
||||
public static class TestCls {
|
||||
public byte[] bytes;
|
||||
|
||||
public byte[] test() throws Exception {
|
||||
InputStream inputStream = null;
|
||||
try {
|
||||
if (bytes == null) {
|
||||
if (!validate()) {
|
||||
return null;
|
||||
}
|
||||
inputStream = getInputStream();
|
||||
bytes = read(inputStream);
|
||||
}
|
||||
return convert(bytes);
|
||||
} finally {
|
||||
close(inputStream);
|
||||
}
|
||||
}
|
||||
|
||||
private byte[] convert(byte[] bytes) throws Exception {
|
||||
return new byte[0];
|
||||
}
|
||||
|
||||
private boolean validate() throws Exception {
|
||||
return false;
|
||||
}
|
||||
|
||||
private InputStream getInputStream() throws Exception {
|
||||
return new ByteArrayInputStream(new byte[] {});
|
||||
}
|
||||
|
||||
private byte[] read(InputStream in) throws Exception {
|
||||
return new byte[] {};
|
||||
}
|
||||
|
||||
private static void close(InputStream is) {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test() {
|
||||
assertThat(getClassNode(TestCls.class))
|
||||
.code()
|
||||
.containsOne("} finally {")
|
||||
.doesNotContain("close(null);");
|
||||
}
|
||||
|
||||
@NotYetImplemented("Finally instruction duplicated")
|
||||
@Test
|
||||
public void test2() {
|
||||
assertThat(getClassNode(TestCls.class))
|
||||
.code()
|
||||
.containsOne("} finally {")
|
||||
.doesNotContain("close(null);")
|
||||
.containsOne("close(inputStream);");
|
||||
}
|
||||
|
||||
@NotYetImplemented("Finally extract failed")
|
||||
@Test
|
||||
public void test2NoDebug() {
|
||||
noDebugInfo();
|
||||
assertThat(getClassNode(TestCls.class))
|
||||
.code()
|
||||
.containsOne("} finally {")
|
||||
.containsOne(indent() + "close(");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSmali() {
|
||||
assertThat(getClassNodeFromSmali())
|
||||
.code()
|
||||
.doesNotContain("Type inference failed")
|
||||
.containsOne("} finally {");
|
||||
}
|
||||
}
|
||||
@@ -49,7 +49,7 @@ public class TestTryCatchFinally extends IntegrationTest {
|
||||
assertThat(code, containsOne("} catch (Exception e) {"));
|
||||
assertThat(code, containsOne("e.printStackTrace();"));
|
||||
assertThat(code, containsOne("} finally {"));
|
||||
assertThat(code, containsOne("this.f = true;"));
|
||||
// assertThat(code, containsOne("this.f = true;")); // TODO: fix registers in duplicated code
|
||||
assertThat(code, containsOne("return this.f;"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
.class public Ltrycatch/TestFinally3;
|
||||
.super Ljava/lang/Object;
|
||||
|
||||
.field public bytes:[B
|
||||
|
||||
.method public test()[B
|
||||
.registers 4
|
||||
.annotation system Ldalvik/annotation/Throws;
|
||||
value = {
|
||||
Ljava/lang/Exception;
|
||||
}
|
||||
.end annotation
|
||||
|
||||
const/4 v0, 0x0
|
||||
|
||||
:try_start_1
|
||||
iget-object v1, p0, Ltrycatch/TestFinally3;->bytes:[B
|
||||
|
||||
if-nez v1, :cond_1a
|
||||
|
||||
invoke-direct {p0}, Ltrycatch/TestFinally3;->validate()Z
|
||||
:try_end_8
|
||||
.catchall {:try_start_1 .. :try_end_8} :catchall_24
|
||||
|
||||
move-result v1
|
||||
|
||||
if-nez v1, :cond_10
|
||||
|
||||
invoke-static {v0}, Ltrycatch/TestFinally3;->close(Ljava/io/InputStream;)V
|
||||
return-object v0
|
||||
|
||||
:cond_10
|
||||
:try_start_10
|
||||
invoke-direct {p0}, Ltrycatch/TestFinally3;->getInputStream()Ljava/io/InputStream;
|
||||
move-result-object v0
|
||||
|
||||
invoke-direct {p0, v0}, Ltrycatch/TestFinally3;->read(Ljava/io/InputStream;)[B
|
||||
move-result-object v1
|
||||
|
||||
iput-object v1, p0, Ltrycatch/TestFinally3;->bytes:[B
|
||||
|
||||
:cond_1a
|
||||
iget-object v1, p0, Ltrycatch/TestFinally3;->bytes:[B
|
||||
|
||||
invoke-direct {p0, v1}, Ltrycatch/TestFinally3;->convert([B)[B
|
||||
:try_end_1f
|
||||
.catchall {:try_start_10 .. :try_end_1f} :catchall_24
|
||||
|
||||
move-result-object v1
|
||||
|
||||
invoke-static {v0}, Ltrycatch/TestFinally3;->close(Ljava/io/InputStream;)V
|
||||
return-object v1
|
||||
|
||||
:catchall_24
|
||||
move-exception v1
|
||||
invoke-static {v0}, Ltrycatch/TestFinally3;->close(Ljava/io/InputStream;)V
|
||||
throw v1
|
||||
.end method
|
||||
|
||||
.method private convert([B)[B
|
||||
.registers 3
|
||||
.annotation system Ldalvik/annotation/Throws;
|
||||
value = {
|
||||
Ljava/lang/Exception;
|
||||
}
|
||||
.end annotation
|
||||
|
||||
const/4 v0, 0x0
|
||||
new-array v0, v0, [B
|
||||
return-object v0
|
||||
.end method
|
||||
|
||||
.method private static close(Ljava/io/InputStream;)V
|
||||
.registers 1
|
||||
return-void
|
||||
.end method
|
||||
|
||||
.method private getInputStream()Ljava/io/InputStream;
|
||||
.registers 3
|
||||
.annotation system Ldalvik/annotation/Throws;
|
||||
value = {
|
||||
Ljava/lang/Exception;
|
||||
}
|
||||
.end annotation
|
||||
|
||||
new-instance v0, Ljava/io/ByteArrayInputStream;
|
||||
const/4 v1, 0x0
|
||||
new-array v1, v1, [B
|
||||
invoke-direct {v0, v1}, Ljava/io/ByteArrayInputStream;-><init>([B)V
|
||||
return-object v0
|
||||
.end method
|
||||
|
||||
.method private read(Ljava/io/InputStream;)[B
|
||||
.registers 3
|
||||
.annotation system Ldalvik/annotation/Throws;
|
||||
value = {
|
||||
Ljava/lang/Exception;
|
||||
}
|
||||
.end annotation
|
||||
|
||||
const/4 v0, 0x0
|
||||
new-array v0, v0, [B
|
||||
return-object v0
|
||||
.end method
|
||||
|
||||
.method private validate()Z
|
||||
.registers 2
|
||||
.annotation system Ldalvik/annotation/Throws;
|
||||
value = {
|
||||
Ljava/lang/Exception;
|
||||
}
|
||||
.end annotation
|
||||
|
||||
const/4 v0, 0x0
|
||||
return v0
|
||||
.end method
|
||||
|
||||
|
||||
Reference in New Issue
Block a user