From 89977a8438107814dfd001d132f7045cba8784bb Mon Sep 17 00:00:00 2001 From: Roman Shevchenko Date: Fri, 28 Sep 2018 18:17:53 +0200 Subject: [PATCH] [java decompiler] improves anonymous classes verification - puts the check under an option - uses 'EnclosingMethod' attribute to skip unrelated methods --- .../decompiler/main/ClassesProcessor.java | 140 +++++++++--------- .../main/extern/IFernflowerPreferences.java | 2 + .../java/decompiler/SingleClassesTest.java | 3 +- 3 files changed, 75 insertions(+), 70 deletions(-) diff --git a/src/org/jetbrains/java/decompiler/main/ClassesProcessor.java b/src/org/jetbrains/java/decompiler/main/ClassesProcessor.java index 0e29334..1ee22f2 100644 --- a/src/org/jetbrains/java/decompiler/main/ClassesProcessor.java +++ b/src/org/jetbrains/java/decompiler/main/ClassesProcessor.java @@ -18,6 +18,7 @@ import org.jetbrains.java.decompiler.modules.decompiler.vars.VarVersionPair; import org.jetbrains.java.decompiler.struct.StructClass; import org.jetbrains.java.decompiler.struct.StructContext; import org.jetbrains.java.decompiler.struct.StructMethod; +import org.jetbrains.java.decompiler.struct.attr.StructEnclosingMethodAttribute; import org.jetbrains.java.decompiler.struct.attr.StructGeneralAttribute; import org.jetbrains.java.decompiler.struct.attr.StructInnerClassesAttribute; import org.jetbrains.java.decompiler.struct.consts.ConstantPool; @@ -56,6 +57,7 @@ public class ClassesProcessor implements CodeConstants { Map mapNewSimpleNames = new HashMap<>(); boolean bDecompileInner = DecompilerContext.getOption(IFernflowerPreferences.DECOMPILE_INNER); + boolean verifyAnonymousClasses = DecompilerContext.getOption(IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES); // create class nodes for (StructClass cl : context.getClasses().values()) { @@ -171,27 +173,22 @@ public class ClassesProcessor implements CodeConstants { nestedNode.type = rec.type; nestedNode.access = rec.accessFlags; + // sanity checks of the class supposed to be anonymous + if (verifyAnonymousClasses && nestedNode.type == ClassNode.CLASS_ANONYMOUS && !isAnonymous(nestedNode.classStruct, scl)) { + nestedNode.type = ClassNode.CLASS_LOCAL; + } + if (nestedNode.type == ClassNode.CLASS_ANONYMOUS) { StructClass cl = nestedNode.classStruct; + // remove static if anonymous class (a common compiler bug) + nestedNode.access &= ~CodeConstants.ACC_STATIC; int[] interfaces = cl.getInterfaces(); - - // sanity checks of the class supposed to be anonymous - boolean isAnonymousChecked = checkClassAnonymous(cl, scl); - - if(isAnonymousChecked) { - // remove static if anonymous class (a common compiler bug) - nestedNode.access &= ~CodeConstants.ACC_STATIC; - - if (interfaces.length > 0) { - nestedNode.anonymousClassType = new VarType(cl.getInterface(0), true); - } - else { - nestedNode.anonymousClassType = new VarType(cl.superClass.getString(), true); - } - } else { // change it to a local class - nestedNode.type = ClassNode.CLASS_LOCAL; - nestedNode.access &= (CodeConstants.ACC_ABSTRACT | CodeConstants.ACC_FINAL); + if (interfaces.length > 0) { + nestedNode.anonymousClassType = new VarType(cl.getInterface(0), true); + } + else { + nestedNode.anonymousClassType = new VarType(cl.superClass.getString(), true); } } else if (nestedNode.type == ClassNode.CLASS_LOCAL) { @@ -212,85 +209,90 @@ public class ClassesProcessor implements CodeConstants { } } } - - private boolean checkClassAnonymous(StructClass cl, StructClass enclosing_cl) { - - int[] interfaces = cl.getInterfaces(); - boolean hasNonTrivialSuperClass = cl.superClass != null && !VarType.VARTYPE_OBJECT.equals(new VarType(cl.superClass.getString(), true)); - + + private static boolean isAnonymous(StructClass cl, StructClass enclosingCl) { // checking super class and interfaces - if(interfaces.length > 0) { - if(hasNonTrivialSuperClass || interfaces.length > 1) { // can't have multiple 'sources' - String message = "Inconsistent anonymous class definition: '" + cl.qualifiedName+"'. Multiple interfaces and/or super class defined."; + int[] interfaces = cl.getInterfaces(); + if (interfaces.length > 0) { + boolean hasNonTrivialSuperClass = cl.superClass != null && !VarType.VARTYPE_OBJECT.equals(new VarType(cl.superClass.getString(), true)); + if (hasNonTrivialSuperClass || interfaces.length > 1) { // can't have multiple 'sources' + String message = "Inconsistent anonymous class definition: '" + cl.qualifiedName + "'. Multiple interfaces and/or super class defined."; DecompilerContext.getLogger().writeMessage(message, IFernflowerLogger.Severity.WARN); return false; } - } else if(cl.superClass == null) { // neither interface nor super class defined - String message = "Inconsistent anonymous class definition: '" + cl.qualifiedName+"'. Neither interface nor super class defined."; + } + else if (cl.superClass == null) { // neither interface nor super class defined + String message = "Inconsistent anonymous class definition: '" + cl.qualifiedName + "'. Neither interface nor super class defined."; DecompilerContext.getLogger().writeMessage(message, IFernflowerLogger.Severity.WARN); return false; } - + // FIXME: check constructors // FIXME: check enclosing class/method - - ConstantPool pool = enclosing_cl.getPool(); - - int ref_counter = 0; - boolean ref_not_new = false; - - // checking references in the enclosing class (TODO: limit to the enclosing method?) - for (StructMethod mt : enclosing_cl.getMethods()) { + + ConstantPool pool = enclosingCl.getPool(); + + int refCounter = 0; + boolean refNotNew = false; + + StructEnclosingMethodAttribute attribute = cl.getAttribute(StructGeneralAttribute.ATTRIBUTE_ENCLOSING_METHOD); + String enclosingMethod = attribute != null ? attribute.getMethodName() : null; + + // checking references in the enclosing class + for (StructMethod mt : enclosingCl.getMethods()) { + if (enclosingMethod != null && !enclosingMethod.equals(mt.getName())) { + continue; + } + try { mt.expandData(); + InstructionSequence seq = mt.getInstructionSequence(); - - if(seq != null) { - + if (seq != null) { int len = seq.length(); for (int i = 0; i < len; i++) { Instruction instr = seq.getInstr(i); - - switch(instr.opcode) { - case opc_checkcast: - case opc_instanceof: - if(cl.qualifiedName.equals(pool.getPrimitiveConstant(instr.operand(0)).getString())) { - ref_counter++; - ref_not_new = true; - } - break; - case opc_new: - case opc_anewarray: - case opc_multianewarray: - if(cl.qualifiedName.equals(pool.getPrimitiveConstant(instr.operand(0)).getString())) { - ref_counter++; - } - break; - case opc_getstatic: - case opc_putstatic: - if(cl.qualifiedName.equals(pool.getLinkConstant(instr.operand(0)).classname)) { - ref_counter++; - ref_not_new = true; - } + switch (instr.opcode) { + case opc_checkcast: + case opc_instanceof: + if (cl.qualifiedName.equals(pool.getPrimitiveConstant(instr.operand(0)).getString())) { + refCounter++; + refNotNew = true; + } + break; + case opc_new: + case opc_anewarray: + case opc_multianewarray: + if (cl.qualifiedName.equals(pool.getPrimitiveConstant(instr.operand(0)).getString())) { + refCounter++; + } + break; + case opc_getstatic: + case opc_putstatic: + if (cl.qualifiedName.equals(pool.getLinkConstant(instr.operand(0)).classname)) { + refCounter++; + refNotNew = true; + } } } } mt.releaseResources(); - } catch(IOException ex) { - String message = "Could not read method while checking anonymous class definition: '"+enclosing_cl.qualifiedName+"', '"+ - InterpreterUtil.makeUniqueKey(mt.getName(), mt.getDescriptor())+"'"; + } + catch (IOException ex) { + String message = "Could not read method while checking anonymous class definition: '" + enclosingCl.qualifiedName + "', '" + + InterpreterUtil.makeUniqueKey(mt.getName(), mt.getDescriptor()) + "'"; DecompilerContext.getLogger().writeMessage(message, IFernflowerLogger.Severity.WARN); return false; } - - if(ref_counter > 1 || ref_not_new) { - String message = "Inconsistent references to the class '"+cl.qualifiedName+"' which is supposed to be anonymous"; + + if (refCounter > 1 || refNotNew) { + String message = "Inconsistent references to the class '" + cl.qualifiedName + "' which is supposed to be anonymous"; DecompilerContext.getLogger().writeMessage(message, IFernflowerLogger.Severity.WARN); return false; } } - + return true; } diff --git a/src/org/jetbrains/java/decompiler/main/extern/IFernflowerPreferences.java b/src/org/jetbrains/java/decompiler/main/extern/IFernflowerPreferences.java index 612b425..5448e79 100644 --- a/src/org/jetbrains/java/decompiler/main/extern/IFernflowerPreferences.java +++ b/src/org/jetbrains/java/decompiler/main/extern/IFernflowerPreferences.java @@ -33,6 +33,7 @@ public interface IFernflowerPreferences { String LAMBDA_TO_ANONYMOUS_CLASS = "lac"; String BYTECODE_SOURCE_MAPPING = "bsm"; String IGNORE_INVALID_BYTECODE = "iib"; + String VERIFY_ANONYMOUS_CLASSES = "vac"; String LOG_LEVEL = "log"; String MAX_PROCESSING_METHOD = "mpm"; @@ -78,6 +79,7 @@ public interface IFernflowerPreferences { defaults.put(LAMBDA_TO_ANONYMOUS_CLASS, "0"); defaults.put(BYTECODE_SOURCE_MAPPING, "0"); defaults.put(IGNORE_INVALID_BYTECODE, "0"); + defaults.put(VERIFY_ANONYMOUS_CLASSES, "0"); defaults.put(LOG_LEVEL, IFernflowerLogger.Severity.INFO.name()); defaults.put(MAX_PROCESSING_METHOD, "0"); diff --git a/test/org/jetbrains/java/decompiler/SingleClassesTest.java b/test/org/jetbrains/java/decompiler/SingleClassesTest.java index 7482741..f2b28b9 100644 --- a/test/org/jetbrains/java/decompiler/SingleClassesTest.java +++ b/test/org/jetbrains/java/decompiler/SingleClassesTest.java @@ -24,7 +24,8 @@ public class SingleClassesTest { fixture = new DecompilerTestFixture(); fixture.setUp(IFernflowerPreferences.BYTECODE_SOURCE_MAPPING, "1", IFernflowerPreferences.DUMP_ORIGINAL_LINES, "1", - IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1"); + IFernflowerPreferences.IGNORE_INVALID_BYTECODE, "1", + IFernflowerPreferences.VERIFY_ANONYMOUS_CLASSES, "1"); } @After