Skip to content

Commit

Permalink
fix: don't run class process from visitors to avoid deadlock (#743)
Browse files Browse the repository at this point in the history
  • Loading branch information
skylot committed Aug 27, 2019
1 parent 1cbaad3 commit db892ad
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 45 deletions.
4 changes: 1 addition & 3 deletions jadx-core/src/main/java/jadx/core/ProcessClass.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
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 {

Expand All @@ -27,8 +26,7 @@ public static void process(ClassNode cls) {
process(topParentClass);
return;
}
if (cls.getState() == PROCESS_COMPLETE
|| cls.getState() == UNLOADED) {
if (cls.getState().isProcessed()) {
// nothing to do
return;
}
Expand Down
23 changes: 5 additions & 18 deletions jadx-core/src/main/java/jadx/core/codegen/InsnGen.java
Original file line number Diff line number Diff line change
Expand Up @@ -591,10 +591,8 @@ private void filledNewArray(FilledNewArrayNode insn, CodeWriter code) throws Cod
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;
}
Expand Down Expand Up @@ -787,21 +785,10 @@ void generateMethodArguments(CodeWriter code, InsnNode insn, int startArgNum,
*/
private boolean processOverloadedArg(CodeWriter code, InsnNode insn, MethodNode callMth, InsnArg arg, int origPos) {
List<ArgType> 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()) {
Expand Down
35 changes: 23 additions & 12 deletions jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,31 +110,37 @@ public ClassNode(DexNode dex, ClassDef cls) {
}

loadAnnotations(cls);

initAccessFlags(cls);
parseClassSignature();
setFieldsTypesFromSignature();
methods.forEach(MethodNode::initMethodTypes);

int sfIdx = cls.getSourceFileIndex();
if (sfIdx != DexNode.NO_INDEX) {
String fileName = dex.getString(sfIdx);
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;
Expand Down Expand Up @@ -247,8 +253,13 @@ private void addSourceFilenameAttr(String fileName) {
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() {
Expand Down
16 changes: 8 additions & 8 deletions jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,15 @@ public void load() throws DecodeException {
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);
Expand Down Expand Up @@ -172,15 +173,14 @@ public void checkInstructions() {
}
}

private void initMethodTypes() {
public void initMethodTypes() {
List<ArgType> types = parseSignature();
if (types == null) {
this.retType = mthInfo.getReturnType();
this.argTypes = mthInfo.getArgumentsTypes();
} else {
this.argTypes = types;
}
initArguments(this.argTypes);
}

@Nullable
Expand Down Expand Up @@ -253,11 +253,11 @@ private void initArguments(List<ArgType> args) {
}
}

/**
* Return null only if method not yet loaded
*/
@Nullable
@NotNull
public List<ArgType> getArgTypes() {
if (argTypes == null) {
throw new JadxRuntimeException("Method types not initialized: " + this);
}
return argTypes;
}

Expand Down
10 changes: 9 additions & 1 deletion jadx-core/src/main/java/jadx/core/dex/nodes/ProcessState.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
1 change: 0 additions & 1 deletion jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ public List<ArgType> getMethodArgTypes(MethodInfo callMth) {
public List<GenericInfo> getClassGenerics(ArgType type) {
ClassNode classNode = resolveClass(ClassInfo.fromType(this, type));
if (classNode != null) {
classNode.loadAndProcess();
return classNode.getGenerics();
}
NClass clsDetails = getClsp().getClsDetails(type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,6 @@ private static void processAnonymousConstructor(MethodNode mth, ConstructorInsn
if (!mth.getParentClass().getInnerClasses().contains(classNode)) {
return;
}
classNode.loadAndProcess();
Map<InsnArg, FieldNode> argsMap = getArgsToFieldsMapping(callMthNode, co);
if (argsMap.isEmpty() && !callMthNode.getArgRegs().isEmpty()) {
return;
Expand Down
3 changes: 2 additions & 1 deletion jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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;
Expand Down Expand Up @@ -223,7 +224,7 @@ private void insertResources(RootNode root) {
}

protected void decompileWithoutUnload(JadxDecompiler jadx, ClassNode cls) {
cls.loadAndProcess();
ProcessClass.process(cls);
generateClsCode(cls);
// don't unload class
}
Expand Down

0 comments on commit db892ad

Please sign in to comment.