fix: remove shadowed catch handlers (#1377)
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
package jadx.core.dex.visitors;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
|
||||
import org.jetbrains.annotations.Nullable;
|
||||
@@ -15,12 +16,16 @@ import jadx.core.dex.attributes.AFlag;
|
||||
import jadx.core.dex.attributes.AType;
|
||||
import jadx.core.dex.info.ClassInfo;
|
||||
import jadx.core.dex.instructions.InsnType;
|
||||
import jadx.core.dex.instructions.args.ArgType;
|
||||
import jadx.core.dex.nodes.InsnNode;
|
||||
import jadx.core.dex.nodes.MethodNode;
|
||||
import jadx.core.dex.trycatch.CatchAttr;
|
||||
import jadx.core.dex.trycatch.ExcHandlerAttr;
|
||||
import jadx.core.dex.trycatch.ExceptionHandler;
|
||||
import jadx.core.dex.visitors.typeinference.TypeCompare;
|
||||
import jadx.core.dex.visitors.typeinference.TypeCompareEnum;
|
||||
import jadx.core.utils.exceptions.JadxException;
|
||||
import jadx.core.utils.exceptions.JadxRuntimeException;
|
||||
|
||||
import static jadx.core.dex.visitors.ProcessInstructionsVisitor.getNextInsnOffset;
|
||||
|
||||
@@ -51,7 +56,7 @@ public class AttachTryCatchVisitor extends AbstractVisitor {
|
||||
tries.forEach(tryData -> LOG.debug(" - {}", tryData));
|
||||
}
|
||||
for (ITry tryData : tries) {
|
||||
List<ExceptionHandler> handlers = attachHandlers(mth, tryData.getCatch(), insnByOffset);
|
||||
List<ExceptionHandler> handlers = convertToHandlers(mth, tryData.getCatch(), insnByOffset);
|
||||
if (handlers.isEmpty()) {
|
||||
continue;
|
||||
}
|
||||
@@ -102,7 +107,7 @@ public class AttachTryCatchVisitor extends AbstractVisitor {
|
||||
}
|
||||
}
|
||||
|
||||
private static List<ExceptionHandler> attachHandlers(MethodNode mth, ICatch catchBlock, InsnNode[] insnByOffset) {
|
||||
private static List<ExceptionHandler> convertToHandlers(MethodNode mth, ICatch catchBlock, InsnNode[] insnByOffset) {
|
||||
int[] handlerOffsetArr = catchBlock.getHandlers();
|
||||
String[] handlerTypes = catchBlock.getTypes();
|
||||
|
||||
@@ -117,6 +122,7 @@ public class AttachTryCatchVisitor extends AbstractVisitor {
|
||||
if (allHandlerOffset >= 0) {
|
||||
Utils.addToList(list, createHandler(mth, insnByOffset, allHandlerOffset, null));
|
||||
}
|
||||
checkAndFilterHandlers(mth, list);
|
||||
return list;
|
||||
}
|
||||
|
||||
@@ -143,6 +149,45 @@ public class AttachTryCatchVisitor extends AbstractVisitor {
|
||||
return handler;
|
||||
}
|
||||
|
||||
private static void checkAndFilterHandlers(MethodNode mth, List<ExceptionHandler> list) {
|
||||
if (list.size() <= 1) {
|
||||
return;
|
||||
}
|
||||
// Remove shadowed handlers (with same or narrow type compared to previous)
|
||||
TypeCompare typeCompare = mth.root().getTypeCompare();
|
||||
Iterator<ExceptionHandler> it = list.iterator();
|
||||
ArgType maxType = null;
|
||||
while (it.hasNext()) {
|
||||
ExceptionHandler handler = it.next();
|
||||
ArgType maxCatch = maxCatchFromHandler(handler, typeCompare);
|
||||
if (maxType == null) {
|
||||
maxType = maxCatch;
|
||||
} else {
|
||||
TypeCompareEnum result = typeCompare.compareObjects(maxType, maxCatch);
|
||||
if (result.isWiderOrEqual()) {
|
||||
if (Consts.DEBUG_EXC_HANDLERS) {
|
||||
LOG.debug("Removed shadowed catch handler: {}, from list: {}", handler, list);
|
||||
}
|
||||
it.remove();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static ArgType maxCatchFromHandler(ExceptionHandler handler, TypeCompare typeCompare) {
|
||||
List<ClassInfo> catchTypes = handler.getCatchTypes();
|
||||
if (catchTypes.isEmpty()) {
|
||||
return ArgType.THROWABLE;
|
||||
}
|
||||
if (catchTypes.size() == 1) {
|
||||
return catchTypes.get(0).getType();
|
||||
}
|
||||
return catchTypes.stream()
|
||||
.map(ClassInfo::getType)
|
||||
.max(typeCompare.getComparator())
|
||||
.orElseThrow(() -> new JadxRuntimeException("Failed to get max type from catch list: " + catchTypes));
|
||||
}
|
||||
|
||||
private static InsnNode insertNOP(InsnNode[] insnByOffset, int offset) {
|
||||
InsnNode nop = new InsnNode(InsnType.NOP, 0);
|
||||
nop.setOffset(offset);
|
||||
|
||||
@@ -9,6 +9,7 @@ import java.util.Objects;
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
import jadx.core.dex.info.ClassInfo;
|
||||
import jadx.core.dex.instructions.args.ArgType;
|
||||
import jadx.core.dex.instructions.args.ArgType.WildcardBound;
|
||||
import jadx.core.dex.instructions.args.PrimitiveType;
|
||||
@@ -42,6 +43,17 @@ public class TypeCompare {
|
||||
return compareObjects(first.getType(), second.getType());
|
||||
}
|
||||
|
||||
public TypeCompareEnum compareTypes(ClassInfo first, ClassInfo second) {
|
||||
return compareObjects(first.getType(), second.getType());
|
||||
}
|
||||
|
||||
public TypeCompareEnum compareObjects(ArgType first, ArgType second) {
|
||||
if (first == second || Objects.equals(first, second)) {
|
||||
return TypeCompareEnum.EQUAL;
|
||||
}
|
||||
return compareObjectsNoPreCheck(first, second);
|
||||
}
|
||||
|
||||
/**
|
||||
* Compare two type and return result for first argument (narrow, wider or conflict)
|
||||
*/
|
||||
@@ -81,7 +93,7 @@ public class TypeCompare {
|
||||
boolean firstObj = first.isObject();
|
||||
boolean secondObj = second.isObject();
|
||||
if (firstObj && secondObj) {
|
||||
return compareObjects(first, second);
|
||||
return compareObjectsNoPreCheck(first, second);
|
||||
} else {
|
||||
// primitive types conflicts with objects
|
||||
if (firstObj && secondPrimitive) {
|
||||
@@ -159,7 +171,7 @@ public class TypeCompare {
|
||||
return CONFLICT;
|
||||
}
|
||||
|
||||
private TypeCompareEnum compareObjects(ArgType first, ArgType second) {
|
||||
private TypeCompareEnum compareObjectsNoPreCheck(ArgType first, ArgType second) {
|
||||
boolean objectsEquals = first.getObject().equals(second.getObject());
|
||||
boolean firstGenericType = first.isGenericType();
|
||||
boolean secondGenericType = second.isGenericType();
|
||||
@@ -262,7 +274,7 @@ public class TypeCompare {
|
||||
return NARROW;
|
||||
}
|
||||
for (ArgType extendType : extendTypes) {
|
||||
TypeCompareEnum res = compareObjects(extendType, objType);
|
||||
TypeCompareEnum res = compareObjectsNoPreCheck(extendType, objType);
|
||||
if (!res.isNarrow()) {
|
||||
return res;
|
||||
}
|
||||
|
||||
@@ -39,6 +39,10 @@ public enum TypeCompareEnum {
|
||||
return this == WIDER || this == WIDER_BY_GENERIC;
|
||||
}
|
||||
|
||||
public boolean isWiderOrEqual() {
|
||||
return isEqual() || isWider();
|
||||
}
|
||||
|
||||
public boolean isNarrow() {
|
||||
return this == NARROW || this == NARROW_BY_GENERIC;
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import java.io.IOException;
|
||||
import java.lang.reflect.InvocationTargetException;
|
||||
import java.lang.reflect.Method;
|
||||
import java.lang.reflect.Modifier;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
@@ -16,8 +17,9 @@ import java.util.concurrent.Executors;
|
||||
import java.util.concurrent.Future;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.TimeoutException;
|
||||
import java.util.jar.JarEntry;
|
||||
import java.util.jar.JarOutputStream;
|
||||
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
import org.jetbrains.annotations.Nullable;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.Assumptions;
|
||||
@@ -95,6 +97,8 @@ public abstract class IntegrationTest extends TestUtils {
|
||||
protected boolean useEclipseCompiler;
|
||||
private int targetJavaVersion = 8;
|
||||
|
||||
private boolean saveTestJar = false;
|
||||
|
||||
protected Map<Integer, String> resMap = Collections.emptyMap();
|
||||
|
||||
private boolean allowWarnInCode;
|
||||
@@ -457,17 +461,28 @@ public abstract class IntegrationTest extends TestUtils {
|
||||
Path outTmp = FileUtils.createTempDir("jadx-tmp-classes");
|
||||
List<File> files = StaticCompiler.compile(compileFileList, outTmp.toFile(), withDebugInfo, useEclipseCompiler, targetJavaVersion);
|
||||
files.forEach(File::deleteOnExit);
|
||||
if (saveTestJar) {
|
||||
saveToJar(files, outTmp);
|
||||
}
|
||||
// remove classes which are parents for test class
|
||||
String clsName = clsFullName.substring(clsFullName.lastIndexOf('.') + 1);
|
||||
files.removeIf(next -> !next.getName().contains(clsName));
|
||||
return files;
|
||||
}
|
||||
|
||||
@NotNull
|
||||
protected static String removeLineComments(ClassNode cls) {
|
||||
String code = cls.getCode().getCodeStr().replaceAll("\\W*//.*", "");
|
||||
System.out.println(code);
|
||||
return code;
|
||||
private void saveToJar(List<File> files, Path baseDir) throws IOException {
|
||||
Path jarFile = Files.createTempFile("tests-" + getTestName() + '-', ".jar");
|
||||
try (JarOutputStream jar = new JarOutputStream(Files.newOutputStream(jarFile))) {
|
||||
for (File file : files) {
|
||||
Path fullPath = file.toPath();
|
||||
Path relativePath = baseDir.relativize(fullPath);
|
||||
JarEntry entry = new JarEntry(relativePath.toString());
|
||||
jar.putNextEntry(entry);
|
||||
jar.write(Files.readAllBytes(fullPath));
|
||||
jar.closeEntry();
|
||||
}
|
||||
}
|
||||
LOG.info("Test jar saved to: {}", jarFile.toAbsolutePath());
|
||||
}
|
||||
|
||||
public JadxArgs getArgs() {
|
||||
@@ -540,4 +555,9 @@ public abstract class IntegrationTest extends TestUtils {
|
||||
protected void printDisassemble() {
|
||||
this.printDisassemble = true;
|
||||
}
|
||||
|
||||
// Use only for debug purpose
|
||||
protected void saveTestJar() {
|
||||
this.saveTestJar = true;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
package jadx.tests.integration.trycatch;
|
||||
|
||||
import jadx.tests.api.IntegrationTest;
|
||||
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 TestNestedTryCatch3 extends IntegrationTest {
|
||||
|
||||
public static class TestCls {
|
||||
public I test() {
|
||||
try {
|
||||
try {
|
||||
return new A();
|
||||
} catch (Throwable e) {
|
||||
return new B();
|
||||
}
|
||||
} catch (Throwable e) {
|
||||
return new C();
|
||||
}
|
||||
}
|
||||
|
||||
private interface I {
|
||||
}
|
||||
|
||||
private static class A implements I {
|
||||
}
|
||||
|
||||
private static class B implements I {
|
||||
}
|
||||
|
||||
private static class C implements I {
|
||||
}
|
||||
}
|
||||
|
||||
@TestWithProfiles({ TestProfile.JAVA8, TestProfile.DX_J8 })
|
||||
public void test() {
|
||||
assertThat(getClassNode(TestCls.class))
|
||||
.code()
|
||||
.containsOne("return new A();")
|
||||
.containsOne("return new B();")
|
||||
.containsOne("return new C();")
|
||||
.countString(2, "} catch (Throwable ");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user