feat: generate missing throws declarations, validate exceptions (#2441) (PR #2475)

* fix: generate missing throws declarations, validate exceptions (#2441)

* use ClspGraph.isImplements to check base classes, some code improvements

---------

Co-authored-by: Skylot <118523+skylot@users.noreply.github.com>
This commit is contained in:
nitram84
2025-05-09 22:08:41 +02:00
committed by GitHub
parent fd6cb2451b
commit 8bbdbfc563
15 changed files with 607 additions and 5 deletions
@@ -16,7 +16,9 @@ public class Consts {
public static final String CLASS_STRING = "java.lang.String";
public static final String CLASS_CLASS = "java.lang.Class";
public static final String CLASS_THROWABLE = "java.lang.Throwable";
public static final String CLASS_ERROR = "java.lang.Error";
public static final String CLASS_EXCEPTION = "java.lang.Exception";
public static final String CLASS_RUNTIME_EXCEPTION = "java.lang.RuntimeException";
public static final String CLASS_ENUM = "java.lang.Enum";
public static final String CLASS_STRING_BUILDER = "java.lang.StringBuilder";
@@ -35,6 +35,7 @@ import jadx.core.dex.visitors.InitCodeVariables;
import jadx.core.dex.visitors.InlineMethods;
import jadx.core.dex.visitors.MarkMethodsForInline;
import jadx.core.dex.visitors.MethodInvokeVisitor;
import jadx.core.dex.visitors.MethodThrowsVisitor;
import jadx.core.dex.visitors.MethodVisitor;
import jadx.core.dex.visitors.ModVisitor;
import jadx.core.dex.visitors.MoveInlineVisitor;
@@ -180,6 +181,8 @@ public class Jadx {
passes.add(new ReturnVisitor());
passes.add(new CleanRegions());
passes.add(new MethodThrowsVisitor());
passes.add(new CodeShrinkVisitor());
passes.add(new MethodInvokeVisitor());
passes.add(new SimplifyVisitor());
@@ -24,6 +24,7 @@ import jadx.core.dex.attributes.nodes.MethodBridgeAttr;
import jadx.core.dex.attributes.nodes.MethodInlineAttr;
import jadx.core.dex.attributes.nodes.MethodOverrideAttr;
import jadx.core.dex.attributes.nodes.MethodReplaceAttr;
import jadx.core.dex.attributes.nodes.MethodThrowsAttr;
import jadx.core.dex.attributes.nodes.MethodTypeVarsAttr;
import jadx.core.dex.attributes.nodes.PhiListAttr;
import jadx.core.dex.attributes.nodes.RegDebugInfoAttr;
@@ -76,6 +77,7 @@ public final class AType<T extends IJadxAttribute> implements IJadxAttrType<T> {
public static final AType<MethodTypeVarsAttr> METHOD_TYPE_VARS = new AType<>();
public static final AType<AttrList<TryCatchBlockAttr>> TRY_BLOCKS_LIST = new AType<>();
public static final AType<CodeFeaturesAttr> METHOD_CODE_FEATURES = new AType<>();
public static final AType<MethodThrowsAttr> METHOD_THROWS = new AType<>();
// region
public static final AType<DeclareVariablesAttr> DECLARE_VARIABLES = new AType<>();
@@ -0,0 +1,40 @@
package jadx.core.dex.attributes.nodes;
import java.util.Set;
import jadx.api.plugins.input.data.attributes.IJadxAttrType;
import jadx.api.plugins.input.data.attributes.PinnedAttribute;
import jadx.core.dex.attributes.AType;
public class MethodThrowsAttr extends PinnedAttribute {
private final Set<String> list;
private boolean visited;
public MethodThrowsAttr(Set<String> list) {
this.list = list;
}
public boolean isVisited() {
return visited;
}
public void setVisited(boolean visited) {
this.visited = visited;
}
public Set<String> getList() {
return list;
}
@Override
public IJadxAttrType<MethodThrowsAttr> getAttrType() {
return AType.METHOD_THROWS;
}
@Override
public String toString() {
return "THROWS:" + list;
}
}
@@ -34,7 +34,9 @@ public abstract class ArgType {
public static final ArgType STRING = objectNoCache(Consts.CLASS_STRING);
public static final ArgType ENUM = objectNoCache(Consts.CLASS_ENUM);
public static final ArgType THROWABLE = objectNoCache(Consts.CLASS_THROWABLE);
public static final ArgType ERROR = objectNoCache(Consts.CLASS_ERROR);
public static final ArgType EXCEPTION = objectNoCache(Consts.CLASS_EXCEPTION);
public static final ArgType RUNTIME_EXCEPTION = objectNoCache(Consts.CLASS_RUNTIME_EXCEPTION);
public static final ArgType OBJECT_ARRAY = array(OBJECT);
public static final ArgType WILDCARD = wildcard();
@@ -26,6 +26,7 @@ import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.attributes.AType;
import jadx.core.dex.attributes.nodes.LoopInfo;
import jadx.core.dex.attributes.nodes.MethodOverrideAttr;
import jadx.core.dex.attributes.nodes.MethodThrowsAttr;
import jadx.core.dex.attributes.nodes.NotificationAttrNode;
import jadx.core.dex.info.AccessInfo;
import jadx.core.dex.info.AccessInfo.AFType;
@@ -477,11 +478,15 @@ public class MethodNode extends NotificationAttrNode implements IMethodDetails,
@Override
public List<ArgType> getThrows() {
ExceptionsAttr exceptionsAttr = get(JadxAttrType.EXCEPTIONS);
if (exceptionsAttr == null) {
return Collections.emptyList();
MethodThrowsAttr throwsAttr = get(AType.METHOD_THROWS);
if (throwsAttr != null) {
return Utils.collectionMap(throwsAttr.getList(), ArgType::object);
}
return Utils.collectionMap(exceptionsAttr.getList(), ArgType::object);
ExceptionsAttr exceptionsAttr = get(JadxAttrType.EXCEPTIONS);
if (exceptionsAttr != null) {
return Utils.collectionMap(exceptionsAttr.getList(), ArgType::object);
}
return Collections.emptyList();
}
/**
@@ -0,0 +1,300 @@
package jadx.core.dex.visitors;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.jetbrains.annotations.Nullable;
import jadx.api.plugins.input.data.attributes.JadxAttrType;
import jadx.api.plugins.input.data.attributes.types.ExceptionsAttr;
import jadx.core.Consts;
import jadx.core.clsp.ClspClass;
import jadx.core.clsp.ClspMethod;
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.attributes.AType;
import jadx.core.dex.attributes.nodes.MethodThrowsAttr;
import jadx.core.dex.info.ClassInfo;
import jadx.core.dex.info.MethodInfo;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.InvokeNode;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.InsnArg;
import jadx.core.dex.instructions.args.InsnWrapArg;
import jadx.core.dex.instructions.args.RegisterArg;
import jadx.core.dex.nodes.BlockNode;
import jadx.core.dex.nodes.ClassNode;
import jadx.core.dex.nodes.InsnNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.nodes.RootNode;
import jadx.core.dex.trycatch.CatchAttr;
import jadx.core.dex.trycatch.ExceptionHandler;
import jadx.core.dex.visitors.regions.RegionMakerVisitor;
import jadx.core.dex.visitors.typeinference.TypeCompare;
import jadx.core.dex.visitors.typeinference.TypeCompareEnum;
import jadx.core.utils.exceptions.JadxException;
@JadxVisitor(
name = "MethodThrowsVisitor",
desc = "Scan methods to collect thrown exceptions",
runAfter = {
RegionMakerVisitor.class // Run after RegionMakerVisitor to ignore throw instructions of synchronized regions
}
)
public class MethodThrowsVisitor extends AbstractVisitor {
private enum ExceptionType {
THROWS_REQUIRED, RUNTIME, UNKNOWN_TYPE, NO_EXCEPTION
}
private RootNode root;
@Override
public void init(RootNode root) throws JadxException {
this.root = root;
}
@Override
public void visit(MethodNode mth) throws JadxException {
MethodThrowsAttr attr = mth.get(AType.METHOD_THROWS);
if (attr == null) {
attr = new MethodThrowsAttr(new HashSet<>());
mth.addAttr(attr);
}
if (!attr.isVisited()) {
attr.setVisited(true);
processInstructions(mth);
}
List<ArgType> invalid = new ArrayList<>();
ExceptionsAttr exceptions = mth.get(JadxAttrType.EXCEPTIONS);
if (exceptions != null && !exceptions.getList().isEmpty()) {
for (String throwsTypeStr : exceptions.getList()) {
ArgType excType = ArgType.object(throwsTypeStr);
if (validateException(excType) == ExceptionType.NO_EXCEPTION) {
invalid.add(excType);
} else {
attr.getList().add(excType.getObject());
}
}
}
if (!invalid.isEmpty()) {
mth.addWarnComment("Byte code manipulation detected: skipped illegal throws declarations: " + invalid);
}
mergeExceptions(attr.getList());
}
private void mergeExceptions(Set<String> excSet) {
if (excSet.contains(Consts.CLASS_EXCEPTION)) {
excSet.removeIf(e -> !e.equals(Consts.CLASS_EXCEPTION));
return;
}
if (excSet.contains(Consts.CLASS_THROWABLE)) {
excSet.removeIf(e -> !e.equals(Consts.CLASS_THROWABLE));
return;
}
List<String> toRemove = new ArrayList<>();
for (String ex1 : excSet) {
for (String ex2 : excSet) {
if (ex1.equals(ex2)) {
continue;
}
if (isBaseException(ex1, ex2)) {
toRemove.add(ex1);
}
}
}
toRemove.forEach(excSet::remove);
}
private void processInstructions(MethodNode mth) throws JadxException {
if (mth.isNoCode() || mth.getBasicBlocks() == null) {
return;
}
blocks: for (final BlockNode block : mth.getBasicBlocks()) {
// Skip e.g. throw instructions of synchronized regions
boolean skipExceptions = block.contains(AFlag.REMOVE) || block.contains(AFlag.DONT_GENERATE);
Set<String> excludedExceptions = new HashSet<>();
CatchAttr catchAttr = block.get(AType.EXC_CATCH);
if (catchAttr != null) {
for (ExceptionHandler handler : catchAttr.getHandlers()) {
if (handler.isCatchAll()) {
continue blocks;
}
excludedExceptions.add(handler.getArgType().toString());
}
}
for (final InsnNode insn : block.getInstructions()) {
checkInsn(mth, insn, excludedExceptions, skipExceptions);
}
}
}
private void checkInsn(MethodNode mth, InsnNode insn, Set<String> excludedExceptions, boolean skipExceptions) throws JadxException {
if (!skipExceptions && insn.getType() == InsnType.THROW && !insn.contains(AFlag.DONT_GENERATE)) {
InsnArg throwArg = insn.getArg(0);
if (throwArg instanceof RegisterArg) {
RegisterArg regArg = (RegisterArg) throwArg;
ArgType exceptionType = regArg.getType();
if (exceptionType.equals(ArgType.THROWABLE)) {
InsnNode assignInsn = regArg.getAssignInsn();
if (assignInsn != null
&& assignInsn.getType() == InsnType.MOVE_EXCEPTION
&& assignInsn.getResult().contains(AFlag.CUSTOM_DECLARE)) {
// arg variable is from catch statement, ignore Throwable rethrow
return;
}
}
visitThrows(mth, exceptionType);
} else {
if (throwArg instanceof InsnWrapArg) {
InsnWrapArg insnWrapArg = (InsnWrapArg) throwArg;
ArgType exceptionType = insnWrapArg.getType();
visitThrows(mth, exceptionType);
}
}
return;
}
if (insn.getType() == InsnType.INVOKE) {
InvokeNode invokeNode = (InvokeNode) insn;
MethodInfo callMth = invokeNode.getCallMth();
String signature = callMth.makeSignature(true);
ClassInfo classInfo = callMth.getDeclClass();
ClassNode classNode = root.resolveClass(classInfo);
if (classNode != null) {
MethodNode cMth = searchOverriddenMethod(classNode, callMth, signature);
if (cMth == null) {
return;
}
visit(cMth);
MethodThrowsAttr cAttr = cMth.get(AType.METHOD_THROWS);
MethodThrowsAttr attr = mth.get(AType.METHOD_THROWS);
if (attr != null && cAttr != null && !cAttr.getList().isEmpty()) {
attr.getList().addAll(filterExceptions(cAttr.getList(), excludedExceptions));
}
} else {
ClspClass clsDetails = root.getClsp().getClsDetails(classInfo.getType());
if (clsDetails != null) {
ClspMethod cMth = searchOverriddenMethod(clsDetails, signature);
if (cMth != null && cMth.getThrows() != null && !cMth.getThrows().isEmpty()) {
MethodThrowsAttr attr = mth.get(AType.METHOD_THROWS);
if (attr != null) {
for (ArgType ex : cMth.getThrows()) {
attr.getList().add(ex.getObject());
}
}
}
}
}
}
}
private void visitThrows(MethodNode mth, ArgType excType) {
if (excType.isTypeKnown() && isThrowsRequired(mth, excType)) {
mth.get(AType.METHOD_THROWS).getList().add(excType.getObject());
}
}
private boolean isThrowsRequired(MethodNode mth, ArgType type) {
ExceptionType result = validateException(type);
if (result == ExceptionType.UNKNOWN_TYPE) {
mth.addInfoComment("Thrown type has an unknown type hierarchy: " + type);
return true; // assume an exception
}
return result == ExceptionType.THROWS_REQUIRED;
}
private ExceptionType validateException(ArgType clsType) {
if (clsType == null || clsType.equals(ArgType.OBJECT)) {
return ExceptionType.NO_EXCEPTION;
}
if (!clsType.isTypeKnown() || !root.getClsp().isClsKnown(clsType.getObject())) {
return ExceptionType.UNKNOWN_TYPE;
}
if (isImplements(clsType, ArgType.RUNTIME_EXCEPTION) || isImplements(clsType, ArgType.ERROR)) {
return ExceptionType.RUNTIME;
}
if (isImplements(clsType, ArgType.THROWABLE) || isImplements(clsType, ArgType.EXCEPTION)) {
return ExceptionType.THROWS_REQUIRED;
}
return ExceptionType.NO_EXCEPTION;
}
/**
* @return is 'possibleParent' a exception class of 'exception'
*/
private boolean isBaseException(String exception, String possibleParent) {
if (exception.equals(possibleParent)) {
return true;
}
return root.getClsp().isImplements(exception, possibleParent);
}
private boolean isImplements(ArgType type, ArgType baseType) {
if (type.equals(baseType)) {
return true;
}
return root.getClsp().isImplements(type.getObject(), baseType.getObject());
}
private Collection<String> filterExceptions(Set<String> exceptions, Set<String> excludedExceptions) {
Set<String> filteredExceptions = new HashSet<>();
for (String exception : exceptions) {
boolean filtered = false;
for (String excluded : excludedExceptions) {
filtered = exception.equals(excluded) || isBaseException(exception, excluded);
if (filtered) {
break;
}
}
if (!filtered) {
filteredExceptions.add(exception);
}
}
return filteredExceptions;
}
private @Nullable MethodNode searchOverriddenMethod(ClassNode cls, MethodInfo mth, String signature) {
// search by exact full signature (with return value) to fight obfuscation (see test
// 'TestOverrideWithSameName')
String shortId = mth.getShortId();
for (MethodNode supMth : cls.getMethods()) {
if (supMth.getMethodInfo().getShortId().equals(shortId)) {
return supMth;
}
}
// search by signature without return value and check if return value is wider type
for (MethodNode supMth : cls.getMethods()) {
if (supMth.getMethodInfo().getShortId().startsWith(signature) && !supMth.getAccessFlags().isStatic()) {
TypeCompare typeCompare = cls.root().getTypeCompare();
ArgType supRetType = supMth.getMethodInfo().getReturnType();
ArgType mthRetType = mth.getReturnType();
TypeCompareEnum res = typeCompare.compareTypes(supRetType, mthRetType);
if (res.isWider()) {
return supMth;
}
}
}
return null;
}
private ClspMethod searchOverriddenMethod(ClspClass clsDetails, String signature) {
Map<String, ClspMethod> methodsMap = clsDetails.getMethodsMap();
for (Map.Entry<String, ClspMethod> entry : methodsMap.entrySet()) {
String mthShortId = entry.getKey();
// do not check full signature, classpath methods can be trusted
// i.e. doesn't contain methods with same signature in one class
if (mthShortId.startsWith(signature)) {
return entry.getValue();
}
}
return null;
}
}
@@ -0,0 +1,21 @@
package jadx.tests.integration.others;
import org.junit.jupiter.api.Test;
import jadx.tests.api.SmaliTest;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
class TestInvalidExceptions extends SmaliTest {
@Test
void test() {
allowWarnInCode();
assertThat(getClassNodeFromSmali())
.code()
.containsOne("invalidException() throws FileNotFoundException {")
.containsOne("Byte code manipulation detected: skipped illegal throws declaration")
.removeBlockComments()
.doesNotContain("String");
}
}
@@ -0,0 +1,19 @@
package jadx.tests.integration.others;
import org.junit.jupiter.api.Test;
import jadx.tests.api.SmaliTest;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
class TestInvalidExceptions2 extends SmaliTest {
@Test
void test() {
allowWarnInCode();
disableCompilation();
assertThat(getClassNodeFromSmali())
.code()
.containsOne("throwPossibleExceptionType() throws UnknownTypeHierarchyException {");
}
}
@@ -0,0 +1,17 @@
package jadx.tests.integration.others;
import org.junit.jupiter.api.Test;
import jadx.tests.api.SmaliTest;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
class TestMissingExceptions extends SmaliTest {
@Test
void test() {
assertThat(getClassNodeFromSmali())
.code()
.countString(6, "FileNotFoundException");
}
}
@@ -0,0 +1,92 @@
package jadx.tests.integration.others;
import java.io.FileNotFoundException;
import java.io.IOException;
import org.junit.jupiter.api.Test;
import jadx.tests.api.IntegrationTest;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
public class TestThrows extends IntegrationTest {
public static class MissingThrowsTest extends Exception {
private void throwCustomException() throws MissingThrowsTest {
throw new MissingThrowsTest();
}
private void throwException() throws Exception {
throw new Exception();
}
private void throwRuntimeException1() {
throw new RuntimeException();
}
private void throwRuntimeException2() {
throw new NullPointerException();
}
private void throwError() {
throw new Error();
}
private void throwError2() {
throw new OutOfMemoryError();
}
@SuppressWarnings("checkstyle:illegalThrows")
private void throwThrowable() throws Throwable {
throw new Throwable();
}
private void exceptionSource() throws FileNotFoundException {
throw new FileNotFoundException("");
}
public void mergeThrownExceptions() throws IOException {
exceptionSource();
}
public void rethrowThrowable() {
try {
} catch (Throwable t) {
throw t;
}
}
public void doSomething1(int i) throws FileNotFoundException {
if (i == 1) {
doSomething2(i);
} else {
doSomething1(i);
}
}
public void doSomething2(int i) throws FileNotFoundException {
if (i == 1) {
exceptionSource();
} else {
doSomething1(i);
}
}
}
@Test
public void test() {
assertThat(getClassNode(MissingThrowsTest.class))
.code()
.containsOne("throwCustomException() throws TestThrows$MissingThrowsTest {")
.containsOne("throwException() throws Exception {")
.containsOne("throwRuntimeException1() {")
.containsOne("throwRuntimeException2() {")
.containsOne("throwError() {")
.containsOne("throwError2() {")
.containsOne("throwThrowable() throws Throwable {")
.containsOne("exceptionSource() throws FileNotFoundException {")
.containsOne("mergeThrownExceptions() throws IOException {")
.containsOne("rethrowThrowable() {");
}
}
@@ -27,7 +27,7 @@ public class TestVariablesGeneric extends SmaliTest {
assertThat(getClassNodeFromSmali())
.code()
.doesNotContain("iVar2")
.containsOne("public static <T> j a(i<? super T> iVar, c<T> cVar) {")
.containsOne("public static <T> j a(i<? super T> iVar, c<T> cVar) throws OnErrorFailedException {")
.containsOne("if (iVar == null) {")
.countString(2, "} catch (Throwable th");
}
@@ -0,0 +1,16 @@
.class public Lothers/TestInvalidExceptions;
.super Ljava/lang/Object;
.method private invalidException()V
.registers 3
.annotation system Ldalvik/annotation/Throws;
value = {
Ljava/lang/String;
}
.end annotation
new-instance v0, Ljava/io/FileNotFoundException;
const-string v1, ""
invoke-direct {v0, v1}, Ljava/io/FileNotFoundException;-><init>(Ljava/lang/String;)V
throw v0
.end method
@@ -0,0 +1,16 @@
.class public Lothers/TestInvalidExceptions2;
.super Ljava/lang/Object;
.method private throwPossibleExceptionType()V
.registers 3
.annotation system Ldalvik/annotation/Throws;
value = {
Ljadx/UnknownTypeHierarchyException;
}
.end annotation
new-instance v0, Ljadx/UnknownTypeHierarchyException;
const-string v1, ""
invoke-direct {v0, v1}, Ljadx/UnknownTypeHierarchyException;-><init>(Ljava/lang/String;)V
throw v0
.end method
@@ -0,0 +1,67 @@
.class public Lothers/TestMissingExceptions;
.super Ljava/lang/Object;
.method private exceptionSource()V
.registers 3
new-instance v0, Ljava/io/FileNotFoundException;
const-string v1, ""
invoke-direct {v0, v1}, Ljava/io/FileNotFoundException;-><init>(Ljava/lang/String;)V
throw v0
.end method
.method public doSomething1(I)V
.registers 3
.param p1, "i" # I
const/4 v0, 0x1
if-ne p1, v0, :cond_7
invoke-virtual {p0, p1}, Lothers/TestMissingExceptions;->doSomething2(I)V
goto :goto_a
:cond_7
invoke-virtual {p0, p1}, Lothers/TestMissingExceptions;->doSomething1(I)V
:goto_a
return-void
.end method
.method public doSomething2(I)V
.registers 3
.param p1, "i" # I
const/4 v0, 0x1
if-ne p1, v0, :cond_7
invoke-direct {p0}, Lothers/TestMissingExceptions;->exceptionSource()V
goto :goto_a
:cond_7
invoke-virtual {p0, p1}, Lothers/TestMissingExceptions;->doSomething1(I)V
:goto_a
return-void
.end method
.method public mergeThrownExcetions()V
.registers 1
.annotation system Ldalvik/annotation/Throws;
value = {
Ljava/io/IOException;
}
.end annotation
invoke-direct {p0}, Lothers/TestMissingExceptions;->exceptionSource()V
return-void
.end method
.method public missingThrowsAnnotation()V
.registers 1
invoke-direct {p0}, Lothers/TestMissingExceptions;->exceptionSource()V
return-void
.end method