From db892adf343c03cfc19325df014732dae2d14c63 Mon Sep 17 00:00:00 2001 From: Skylot Date: Tue, 27 Aug 2019 16:34:43 +0100 Subject: [PATCH] fix: don't run class process from visitors to avoid deadlock (#743) --- .../src/main/java/jadx/core/ProcessClass.java | 4 +-- .../main/java/jadx/core/codegen/InsnGen.java | 23 +++--------- .../java/jadx/core/dex/nodes/ClassNode.java | 35 ++++++++++++------- .../java/jadx/core/dex/nodes/MethodNode.java | 16 ++++----- .../jadx/core/dex/nodes/ProcessState.java | 10 +++++- .../java/jadx/core/dex/nodes/RootNode.java | 1 - .../jadx/core/dex/visitors/ModVisitor.java | 1 - .../java/jadx/tests/api/IntegrationTest.java | 3 +- 8 files changed, 48 insertions(+), 45 deletions(-) diff --git a/jadx-core/src/main/java/jadx/core/ProcessClass.java b/jadx-core/src/main/java/jadx/core/ProcessClass.java index dd8726115..94eb12006 100644 --- a/jadx-core/src/main/java/jadx/core/ProcessClass.java +++ b/jadx-core/src/main/java/jadx/core/ProcessClass.java @@ -14,7 +14,6 @@ import static jadx.core.dex.nodes.ProcessState.LOADED; import static jadx.core.dex.nodes.ProcessState.NOT_LOADED; import static jadx.core.dex.nodes.ProcessState.PROCESS_COMPLETE; import static jadx.core.dex.nodes.ProcessState.PROCESS_STARTED; -import static jadx.core.dex.nodes.ProcessState.UNLOADED; public final class ProcessClass { @@ -27,8 +26,7 @@ public final class ProcessClass { process(topParentClass); return; } - if (cls.getState() == PROCESS_COMPLETE - || cls.getState() == UNLOADED) { + if (cls.getState().isProcessed()) { // nothing to do return; } diff --git a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java index 1898a4c1c..b43b2de44 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java @@ -591,10 +591,8 @@ public class InsnGen { private void makeConstructor(ConstructorInsn insn, CodeWriter code) throws CodegenException { ClassNode cls = mth.dex().resolveClass(insn.getClassType()); - if (cls != null) { - cls.loadAndProcess(); - } if (cls != null && cls.isAnonymous() && !fallback) { + cls.ensureProcessed(); inlineAnonymousConstructor(code, cls, insn); return; } @@ -787,21 +785,10 @@ public class InsnGen { */ private boolean processOverloadedArg(CodeWriter code, InsnNode insn, MethodNode callMth, InsnArg arg, int origPos) { List argTypes = callMth.getArgTypes(); - if (argTypes == null) { - // try to load class - callMth.getParentClass().loadAndProcess(); - argTypes = callMth.getArgTypes(); - } - ArgType origType; - if (argTypes == null) { - mth.addComment("JADX INFO: used method not loaded: " + callMth + ", types can be incorrect"); - origType = callMth.getMethodInfo().getArgumentsTypes().get(origPos); - } else { - origType = argTypes.get(origPos); - if (origType.isGenericType() && !callMth.getParentClass().equals(mth.getParentClass())) { - // cancel cast - return false; - } + ArgType origType = argTypes.get(origPos); + if (origType.isGenericType() && !callMth.getParentClass().equals(mth.getParentClass())) { + // cancel cast + return false; } ArgType castType = null; if (insn instanceof CallMthInterface && origType.containsGenericType()) { diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java index eeb46bb17..7064be9d9 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java @@ -110,9 +110,10 @@ public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode { } loadAnnotations(cls); - + initAccessFlags(cls); parseClassSignature(); setFieldsTypesFromSignature(); + methods.forEach(MethodNode::initMethodTypes); int sfIdx = cls.getSourceFileIndex(); if (sfIdx != DexNode.NO_INDEX) { @@ -120,21 +121,26 @@ public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode { addSourceFilenameAttr(fileName); } - // restore original access flags from dalvik annotation if present - int accFlagsValue; - Annotation a = getAnnotation(Consts.DALVIK_INNER_CLASS); - if (a != null) { - accFlagsValue = (Integer) a.getValues().get("accessFlags"); - } else { - accFlagsValue = cls.getAccessFlags(); - } - this.accessFlags = new AccessInfo(accFlagsValue, AFType.CLASS); buildCache(); } catch (Exception e) { throw new JadxRuntimeException("Error decode class: " + clsInfo, e); } } + /** + * Restore original access flags from Dalvik annotation if present + */ + private void initAccessFlags(ClassDef cls) { + int accFlagsValue; + Annotation a = getAnnotation(Consts.DALVIK_INNER_CLASS); + if (a != null) { + accFlagsValue = (Integer) a.getValues().get("accessFlags"); + } else { + accFlagsValue = cls.getAccessFlags(); + } + this.accessFlags = new AccessInfo(accFlagsValue, AFType.CLASS); + } + // empty synthetic class public ClassNode(DexNode dex, String name, int accessFlags) { this.dex = dex; @@ -247,8 +253,13 @@ public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode { this.addAttr(new SourceFileAttr(fileName)); } - public void loadAndProcess() { - ProcessClass.process(this); + public void ensureProcessed() { + ClassNode topClass = getTopParentClass(); + ProcessState topState = topClass.getState(); + if (!topState.isProcessed()) { + throw new JadxRuntimeException("Expected class to be processed at this point," + + " class: " + topClass + ", state: " + topState); + } } public synchronized ICodeInfo decompile() { diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java index a71449493..ef9ae8570 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java @@ -121,14 +121,15 @@ public class MethodNode extends LineAttrNode implements ILoadable, ICodeNode { if (noCode) { regsCount = 0; codeSize = 0; - initMethodTypes(); + // TODO: registers not needed without code + initArguments(this.argTypes); return; } DexNode dex = parentClass.dex(); Code mthCode = dex.readCode(methodData); this.regsCount = mthCode.getRegistersSize(); - initMethodTypes(); + initArguments(this.argTypes); InsnDecoder decoder = new InsnDecoder(this); decoder.decodeInsns(mthCode); @@ -172,7 +173,7 @@ public class MethodNode extends LineAttrNode implements ILoadable, ICodeNode { } } - private void initMethodTypes() { + public void initMethodTypes() { List types = parseSignature(); if (types == null) { this.retType = mthInfo.getReturnType(); @@ -180,7 +181,6 @@ public class MethodNode extends LineAttrNode implements ILoadable, ICodeNode { } else { this.argTypes = types; } - initArguments(this.argTypes); } @Nullable @@ -253,11 +253,11 @@ public class MethodNode extends LineAttrNode implements ILoadable, ICodeNode { } } - /** - * Return null only if method not yet loaded - */ - @Nullable + @NotNull public List getArgTypes() { + if (argTypes == null) { + throw new JadxRuntimeException("Method types not initialized: " + this); + } return argTypes; } diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/ProcessState.java b/jadx-core/src/main/java/jadx/core/dex/nodes/ProcessState.java index 580572769..db02f08f3 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/ProcessState.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/ProcessState.java @@ -6,5 +6,13 @@ public enum ProcessState { PROCESS_STARTED, PROCESS_COMPLETE, GENERATED, - UNLOADED + UNLOADED; + + public boolean isLoaded() { + return this != NOT_LOADED; + } + + public boolean isProcessed() { + return this == PROCESS_COMPLETE || this == GENERATED || this == UNLOADED; + } } diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java index 5a0fc2988..781a047d5 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java @@ -276,7 +276,6 @@ public class RootNode { public List getClassGenerics(ArgType type) { ClassNode classNode = resolveClass(ClassInfo.fromType(this, type)); if (classNode != null) { - classNode.loadAndProcess(); return classNode.getGenerics(); } NClass clsDetails = getClsp().getClsDetails(type); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java index 7e4f51b62..538311238 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java @@ -275,7 +275,6 @@ public class ModVisitor extends AbstractVisitor { if (!mth.getParentClass().getInnerClasses().contains(classNode)) { return; } - classNode.loadAndProcess(); Map argsMap = getArgsToFieldsMapping(callMthNode, co); if (argsMap.isEmpty() && !callMthNode.getArgRegs().isEmpty()) { return; 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 c39d6d1d3..1266befe8 100644 --- a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java +++ b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java @@ -27,6 +27,7 @@ import jadx.api.ICodeInfo; import jadx.api.JadxArgs; import jadx.api.JadxDecompiler; import jadx.api.JadxInternalAccess; +import jadx.core.ProcessClass; import jadx.core.codegen.CodeGen; import jadx.core.codegen.CodeWriter; import jadx.core.dex.attributes.AFlag; @@ -223,7 +224,7 @@ public abstract class IntegrationTest extends TestUtils { } protected void decompileWithoutUnload(JadxDecompiler jadx, ClassNode cls) { - cls.loadAndProcess(); + ProcessClass.process(cls); generateClsCode(cls); // don't unload class }