From 30355cc9d6488f3bfdfcff3f00905b999fbbf5da Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 6 Dec 2013 23:42:04 +0400 Subject: [PATCH] core: remove synthetic fields for inner classes --- .../main/java/jadx/core/codegen/ClassGen.java | 3 + .../main/java/jadx/core/codegen/InsnGen.java | 74 ++++++----- .../core/dex/attributes/AttributeFlag.java | 2 + .../java/jadx/core/dex/info/AccessInfo.java | 19 ++- .../core/dex/instructions/args/ArgType.java | 77 +++++++---- .../java/jadx/core/dex/nodes/MethodNode.java | 5 + .../jadx/core/dex/visitors/ClassModifier.java | 125 +++++++++++++++--- .../jadx/core/dex/visitors/ModVisitor.java | 37 ++---- .../jadx/tests/internal/TestInnerClass.java | 4 +- .../TestStringBuilderElimination.java | 4 +- 10 files changed, 236 insertions(+), 114 deletions(-) diff --git a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java index fc1202ae9..dd6946486 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java @@ -297,6 +297,9 @@ public class ClassGen { } for (FieldNode f : fields) { + if(f.getAttributes().contains(AttributeFlag.DONT_GENERATE)) { + continue; + } annotationGen.addForField(code, f); code.startLine(f.getAccessFlags().makeString()); code.add(TypeGen.translate(this, f.getType())); 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 0f7b7b5b1..5129c3350 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java @@ -1,5 +1,6 @@ package jadx.core.codegen; +import jadx.core.dex.attributes.AttributeFlag; import jadx.core.dex.attributes.AttributeType; import jadx.core.dex.attributes.IAttribute; import jadx.core.dex.attributes.MethodInlineAttr; @@ -127,8 +128,12 @@ public class InsnGen { } private String ifield(FieldInfo field, InsnArg arg) throws CodegenException { + FieldNode fieldNode = mth.getParentClass().searchField(field); + if(fieldNode != null && fieldNode.getAttributes().contains(AttributeFlag.DONT_GENERATE)) { + return ""; + } String name = field.getName(); - // TODO: add jadx argument "" + // TODO: add jadx argument "add this" // FIXME: check variable names in scope if (false && arg.isThis()) { boolean useShort = true; @@ -143,7 +148,8 @@ public class InsnGen { return name; } } - return arg(arg) + "." + name; + String argStr = arg(arg); + return argStr.isEmpty() ? name : argStr + "." + name; } protected String sfield(FieldInfo field) { @@ -513,7 +519,7 @@ public class InsnGen { private void makeConstructor(ConstructorInsn insn, CodeWriter code, EnumSet state) throws CodegenException { - ClassNode cls = root.resolveClass(insn.getClassType()); + ClassNode cls = mth.dex().resolveClass(insn.getClassType()); if (cls != null && cls.isAnonymous()) { // anonymous class construction ClassInfo parent; @@ -527,19 +533,21 @@ public class InsnGen { code.incIndent(2); new ClassGen(cls, mgen.getClassGen().getParentGen(), fallback).makeClassBody(code); code.decIndent(2); - } else if (insn.isSuper()) { - code.add("super"); - addArgs(code, insn, 0); - } else if (insn.isThis()) { - code.add("this"); - addArgs(code, insn, 0); - } else if (insn.isSelf()) { + return; + } + if (insn.isSelf()) { // skip state.add(IGState.SKIP); + return; + } + if (insn.isSuper()) { + code.add("super"); + } else if (insn.isThis()) { + code.add("this"); } else { code.add("new ").add(useClass(insn.getClassType())); - addArgs(code, insn, 0); } + generateArguments(code, insn, 0, mth.dex().resolveMethod(insn.getCallMth())); } private void makeInvoke(InvokeNode insn, CodeWriter code) throws CodegenException { @@ -560,8 +568,12 @@ public class InsnGen { case VIRTUAL: case INTERFACE: InsnArg arg = insn.getArg(0); - if (!arg.isThis()) { // FIXME: add 'this' for equals methods in scope - code.add(arg(arg)).add('.'); + // FIXME: add 'this' for equals methods in scope + if (!arg.isThis()) { + String argStr = arg(arg); + if(!argStr.isEmpty()) { + code.add(argStr).add('.'); + } } k++; break; @@ -581,11 +593,18 @@ public class InsnGen { break; } code.add(callMth.getName()); - if (callMthNode != null && callMthNode.isArgsOverload()) { - int argsCount = insn.getArgsCount(); - List originalType = callMth.getArgumentsTypes(); - int origPos = 0; + generateArguments(code, insn, k, callMthNode); + } + private void generateArguments(CodeWriter code, InsnNode insn, int k, MethodNode callMth) throws CodegenException { + if (callMth != null && callMth.getAttributes().contains(AttributeFlag.SKIP_FIRST_ARG)) { + k++; + } + int argsCount = insn.getArgsCount(); + if (callMth != null && callMth.isArgsOverload()) { + // add additional argument casts for overloaded methods + List originalType = callMth.getMethodInfo().getArgumentsTypes(); + int origPos = 0; code.add('('); for (int i = k; i < argsCount; i++) { InsnArg arg = insn.getArg(i); @@ -602,21 +621,16 @@ public class InsnGen { } code.add(')'); } else { - addArgs(code, insn, k); - } - } - - private void addArgs(CodeWriter code, InsnNode insn, int k) throws CodegenException { - int argsCount = insn.getArgsCount(); - code.add('('); - if (k < argsCount) { - code.add(arg(insn.getArg(k), false)); - for (int i = k + 1; i < argsCount; i++) { - code.add(", "); - code.add(arg(insn.getArg(i), false)); + code.add('('); + if (k < argsCount) { + code.add(arg(insn.getArg(k), false)); + for (int i = k + 1; i < argsCount; i++) { + code.add(", "); + code.add(arg(insn.getArg(i), false)); + } } + code.add(')'); } - code.add(')'); } private void inlineMethod(MethodNode callMthNode, InvokeNode insn, CodeWriter code) throws CodegenException { diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/AttributeFlag.java b/jadx-core/src/main/java/jadx/core/dex/attributes/AttributeFlag.java index a06bee1e0..6a26f0f60 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/AttributeFlag.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/AttributeFlag.java @@ -16,5 +16,7 @@ public enum AttributeFlag { DONT_GENERATE, SKIP, + SKIP_FIRST_ARG, + INCONSISTENT_CODE, // warning about incorrect decompilation } diff --git a/jadx-core/src/main/java/jadx/core/dex/info/AccessInfo.java b/jadx-core/src/main/java/jadx/core/dex/info/AccessInfo.java index 117822bf3..3ad74245f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/info/AccessInfo.java +++ b/jadx-core/src/main/java/jadx/core/dex/info/AccessInfo.java @@ -105,10 +105,6 @@ public class AccessInfo { return (accFlags & AccessFlags.ACC_VOLATILE) != 0; } - public int getFlags() { - return accFlags; - } - public AFType getType() { return type; } @@ -178,8 +174,21 @@ public class AccessInfo { return code.toString(); } + public String rawString() { + switch (type){ + case CLASS: + return AccessFlags.classString(accFlags); + case FIELD: + return AccessFlags.fieldString(accFlags); + case METHOD: + return AccessFlags.methodString(accFlags); + default: + return "?"; + } + } + @Override public String toString() { - return "AccessInfo: " + type + " " + accFlags + " (" + makeString() + ")"; + return "AccessInfo: " + type + " 0x" + Integer.toHexString(accFlags) + " (" + rawString() + ")"; } } diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java b/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java index 5be980eef..8020c5aca 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java @@ -82,7 +82,7 @@ public abstract class ArgType { return new UnknownArg(types); } - private static abstract class KnownTypeArg extends ArgType { + private abstract static class KnownTypeArg extends ArgType { @Override public boolean isTypeKnown() { return true; @@ -234,7 +234,7 @@ public abstract class ArgType { } private static final class UnknownArg extends ArgType { - private final PrimitiveType possibleTypes[]; + private final PrimitiveType[] possibleTypes; public UnknownArg(PrimitiveType[] types) { this.possibleTypes = types; @@ -253,19 +253,22 @@ public abstract class ArgType { @Override public boolean contains(PrimitiveType type) { - for (PrimitiveType t : possibleTypes) - if (t == type) + for (PrimitiveType t : possibleTypes) { + if (t == type) { return true; + } + } return false; } @Override public ArgType selectFirst() { PrimitiveType f = possibleTypes[0]; - if (f == PrimitiveType.OBJECT || f == PrimitiveType.ARRAY) + if (f == PrimitiveType.OBJECT || f == PrimitiveType.ARRAY) { return object(Consts.CLASS_OBJECT); - else + } else { return primitive(f); + } } @Override @@ -275,10 +278,11 @@ public abstract class ArgType { @Override public String toString() { - if (possibleTypes.length == PrimitiveType.values().length) + if (possibleTypes.length == PrimitiveType.values().length) { return "?"; - else + } else { return "?" + Arrays.toString(possibleTypes); + } } } @@ -358,32 +362,39 @@ public abstract class ArgType { } if (!a.isTypeKnown()) { if (b.isTypeKnown()) { - if (a.contains(b.getPrimitiveType())) + if (a.contains(b.getPrimitiveType())) { return b; - else + } else { return null; + } } else { // both types unknown List types = new ArrayList(); for (PrimitiveType type : a.getPossibleTypes()) { - if (b.contains(type)) + if (b.contains(type)) { types.add(type); + } } if (types.size() == 0) { return null; } else if (types.size() == 1) { PrimitiveType nt = types.get(0); - if (nt == PrimitiveType.OBJECT || nt == PrimitiveType.ARRAY) + if (nt == PrimitiveType.OBJECT || nt == PrimitiveType.ARRAY) { return unknown(nt); - else + } else { return primitive(nt); + } } else { return unknown(types.toArray(new PrimitiveType[types.size()])); } } } else { - if (a.isGenericType()) return a; - if (b.isGenericType()) return b; + if (a.isGenericType()) { + return a; + } + if (b.isGenericType()) { + return b; + } if (a.isObject() && b.isObject()) { String aObj = a.getObject(); @@ -450,19 +461,20 @@ public abstract class ArgType { public static ArgType parseSignature(String sign) { int b = sign.indexOf('<'); - if (b == -1) + if (b == -1) { return parse(sign); - - if (sign.charAt(0) == '[') + } + if (sign.charAt(0) == '[') { return array(parseSignature(sign.substring(1))); - + } String obj = sign.substring(0, b) + ";"; String genericsStr = sign.substring(b + 1, sign.length() - 2); List generics = parseSignatureList(genericsStr); - if (generics != null) + if (generics != null) { return generic(obj, generics.toArray(new ArgType[generics.size()])); - else + } else { return object(obj); + } } public static List parseSignatureList(String str) { @@ -481,7 +493,6 @@ public abstract class ArgType { if (str.equals("*")) { return Arrays.asList(UNKNOWN); } - List signs = new ArrayList(3); int obj = 0; int objStart = 0; @@ -513,8 +524,9 @@ public abstract class ArgType { if (gen == 0) { obj = 0; String o = str.substring(objStart, pos); - if (o.length() > 0) + if (o.length() > 0) { type = genericType(o); + } } break; @@ -572,8 +584,9 @@ public abstract class ArgType { } prev = arg; } else { - if (!arg.getObject().equals(Consts.CLASS_OBJECT)) + if (!arg.getObject().equals(Consts.CLASS_OBJECT)) { genList.add(arg); + } } } if (prev != null) { @@ -640,10 +653,18 @@ public abstract class ArgType { @Override public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null) return false; - if (hash != obj.hashCode()) return false; - if (getClass() != obj.getClass()) return false; + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (hash != obj.hashCode()) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } return internalEquals(obj); } } 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 b9e1abd37..7b678a107 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 @@ -229,6 +229,11 @@ public class MethodNode extends LineAttrNode implements ILoadable { } } + public RegisterArg removeFirstArgument() { + this.getAttributes().add(AttributeFlag.SKIP_FIRST_ARG); + return argsList.remove(0); + } + public RegisterArg getThisArg() { return thisArg; } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java index b189aff3e..318ac0dff 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java @@ -1,9 +1,18 @@ package jadx.core.dex.visitors; +import jadx.core.dex.attributes.AttributeFlag; import jadx.core.dex.info.AccessInfo; +import jadx.core.dex.info.ClassInfo; +import jadx.core.dex.info.FieldInfo; import jadx.core.dex.info.MethodInfo; +import jadx.core.dex.instructions.IndexInsnNode; +import jadx.core.dex.instructions.InsnType; +import jadx.core.dex.instructions.args.InsnArg; +import jadx.core.dex.instructions.args.RegisterArg; import jadx.core.dex.nodes.BlockNode; import jadx.core.dex.nodes.ClassNode; +import jadx.core.dex.nodes.FieldNode; +import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.utils.exceptions.JadxException; @@ -18,6 +27,78 @@ public class ClassModifier extends AbstractVisitor { visit(inner); } + removeSyntheticFields(cls); + removeSyntheticMethods(cls); + removeEmptyMethods(cls); + return false; + } + + private static void removeSyntheticFields(ClassNode cls) { + if (!cls.getClassInfo().isInner() || cls.getAccessFlags().isStatic()) { + return; + } + // remove fields if it is synthetic and type is a outer class + for (FieldNode field : cls.getFields()) { + if (field.getAccessFlags().isSynthetic() && field.getType().isObject()) { + ClassNode fieldsCls = cls.dex().resolveClass(ClassInfo.fromType(field.getType())); + if (fieldsCls != null + && cls.getClassInfo().getParentClass().equals(fieldsCls.getClassInfo())) { + int found = 0; + for (MethodNode mth : cls.getMethods()) { + if (removeFieldUsage(field, fieldsCls, mth)) { + found++; + } + } + if (found != 0) { + // TODO: make new flag for skip field generation and usage + field.getAttributes().add(AttributeFlag.DONT_GENERATE); + } + } + } + } + } + + private static boolean removeFieldUsage(FieldNode field, ClassNode fieldsCls, MethodNode mth) { + if (!mth.getAccessFlags().isConstructor()) { + return false; + } + List args = mth.getArguments(false); + if (args.isEmpty()) { + return false; + } + RegisterArg arg = args.get(0); + if (!arg.getType().equals(fieldsCls.getClassInfo().getType())) { + return false; + } + BlockNode block = mth.getBasicBlocks().get(0); + List instructions = block.getInstructions(); + if (instructions.isEmpty()) { + return false; + } + InsnNode insn = instructions.get(0); + if (insn.getType() != InsnType.IPUT) { + return false; + } + IndexInsnNode putInsn = (IndexInsnNode) insn; + FieldInfo fieldInfo = (FieldInfo) putInsn.getIndex(); + if (!fieldInfo.equals(field.getFieldInfo()) || !putInsn.getArg(0).equals(arg)) { + return false; + } + mth.removeFirstArgument(); + InstructionRemover.remove(block, insn); + // other arg usage -> wrap with IGET insn + List useList = arg.getTypedVar().getUseList(); + if (useList.size() > 1) { + InsnNode iget = new IndexInsnNode(InsnType.IGET, fieldInfo, 1); + iget.addArg(insn.getArg(1)); + for (InsnArg insnArg : useList) { + insnArg.wrapInstruction(iget); + } + } + return true; + } + + private static void removeSyntheticMethods(ClassNode cls) { for (Iterator it = cls.getMethods().iterator(); it.hasNext(); ) { MethodNode mth = it.next(); AccessInfo af = mth.getAccessFlags(); @@ -29,26 +110,7 @@ public class ClassModifier extends AbstractVisitor { it.remove(); } } - - // remove public empty constructors - if (af.isConstructor() - && af.isPublic() - && mth.getArguments(false).isEmpty()) { - List bb = mth.getBasicBlocks(); - if (bb.isEmpty() || allBlocksEmpty(bb)) { - it.remove(); - } - } } - return false; - } - - private static boolean allBlocksEmpty(List blocks) { - for (BlockNode block : blocks) { - if (block.getInstructions().size() != 0) - return false; - } - return true; } private static boolean isMethodUniq(ClassNode cls, MethodNode mth) { @@ -65,4 +127,29 @@ public class ClassModifier extends AbstractVisitor { } return true; } + + private static void removeEmptyMethods(ClassNode cls) { + for (MethodNode mth : cls.getMethods()) { + AccessInfo af = mth.getAccessFlags(); + + // remove public empty constructors + if (af.isConstructor() + && af.isPublic() + && mth.getArguments(false).isEmpty()) { + List bb = mth.getBasicBlocks(); + if (bb.isEmpty() || allBlocksEmpty(bb)) { + mth.getAttributes().add(AttributeFlag.DONT_GENERATE); + } + } + } + } + + private static boolean allBlocksEmpty(List blocks) { + for (BlockNode block : blocks) { + if (block.getInstructions().size() != 0) { + return false; + } + } + return true; + } } 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 56630fa3d..af9550cd0 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 @@ -1,7 +1,6 @@ package jadx.core.dex.visitors; import jadx.core.deobf.NameMapper; -import jadx.core.dex.attributes.AttributeFlag; import jadx.core.dex.attributes.AttributeType; import jadx.core.dex.info.MethodInfo; import jadx.core.dex.instructions.ConstClassNode; @@ -23,7 +22,6 @@ import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.trycatch.ExcHandlerAttr; import jadx.core.dex.trycatch.ExceptionHandler; -import jadx.core.utils.exceptions.JadxRuntimeException; import java.util.List; @@ -54,7 +52,6 @@ public class ModVisitor extends AbstractVisitor { private void replaceStep(MethodNode mth) { ClassNode parentClass = mth.getParentClass(); - ConstructorInsn superCall = null; for (BlockNode block : mth.getBasicBlocks()) { InstructionRemover remover = new InstructionRemover(block.getInstructions()); @@ -67,32 +64,18 @@ public class ModVisitor extends AbstractVisitor { MethodInfo callMth = inv.getCallMth(); if (callMth.isConstructor()) { ConstructorInsn co = new ConstructorInsn(mth, inv); - if (co.isSuper()) { - try { - // inline super call args - for (int j = 0; j < co.getArgsCount(); j++) { - InsnArg arg = co.getArg(j); - if (arg.isRegister()) { - CodeShrinker.inlineArgument(mth, (RegisterArg) arg); - } - } - } catch (JadxRuntimeException e) { - // inline args into super fail - LOG.warn("Can't inline args into super call: " + inv + ", mth: " + mth); - mth.getAttributes().add(AttributeFlag.INCONSISTENT_CODE); - } finally { - superCall = co; - remover.add(insn); - } + boolean remove = false; + if (co.isSuper() && (co.getArgsCount() == 0 || parentClass.isEnum())) { + remove = true; } else if (co.isThis() && co.getArgsCount() == 0) { - MethodNode defCo = mth.getParentClass() - .searchMethodByName(co.getCallMth().getShortId()); + MethodNode defCo = mth.getParentClass().searchMethodByName(callMth.getShortId()); if (defCo == null || defCo.isNoCode()) { // default constructor not implemented - remover.add(insn); - } else { - replaceInsn(block, i, co); + remove = true; } + } + if (remove) { + remover.add(insn); } else { replaceInsn(block, i, co); } @@ -158,10 +141,6 @@ public class ModVisitor extends AbstractVisitor { } remover.perform(); } - if (superCall != null && !parentClass.isEnum() && superCall.getArgsCount() != 0) { - List insns = mth.getEnterBlock().getInstructions(); - insns.add(0, superCall); - } } /** diff --git a/jadx-core/src/test/java/jadx/tests/internal/TestInnerClass.java b/jadx-core/src/test/java/jadx/tests/internal/TestInnerClass.java index 347f61b91..5304afeb0 100644 --- a/jadx-core/src/test/java/jadx/tests/internal/TestInnerClass.java +++ b/jadx-core/src/test/java/jadx/tests/internal/TestInnerClass.java @@ -27,7 +27,7 @@ public class TestInnerClass extends InternalJadxTest { assertThat(code, containsString("Inner {")); assertThat(code, containsString("Inner2 extends Thread {")); assertThat(code, not(containsString("super();"))); - // assertThat(code, not(containsString("this$0"))); - // assertThat(code, not(containsString("/* synthetic */"))); + assertThat(code, not(containsString("this$"))); + assertThat(code, not(containsString("/* synthetic */"))); } } diff --git a/jadx-core/src/test/java/jadx/tests/internal/TestStringBuilderElimination.java b/jadx-core/src/test/java/jadx/tests/internal/TestStringBuilderElimination.java index 64a2b0df8..8d82eb655 100644 --- a/jadx-core/src/test/java/jadx/tests/internal/TestStringBuilderElimination.java +++ b/jadx-core/src/test/java/jadx/tests/internal/TestStringBuilderElimination.java @@ -3,6 +3,8 @@ package jadx.tests.internal; import jadx.api.InternalJadxTest; import jadx.core.dex.nodes.ClassNode; +import org.junit.Test; + import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.not; import static org.junit.Assert.assertThat; @@ -21,7 +23,7 @@ public class TestStringBuilderElimination extends InternalJadxTest { } } - // @Test + @Test public void test() { ClassNode cls = getClassNode(MyException.class); String code = cls.getCode().toString();