fix: don't inline consts into anonymous class constructor (#1901)

This commit is contained in:
Skylot
2023-06-13 20:01:29 +01:00
parent 82cafe6e94
commit dddb7b2878
7 changed files with 109 additions and 131 deletions
@@ -185,6 +185,10 @@ public class RegisterArg extends InsnArg implements Named {
return regNum == ((RegisterArg) arg).getRegNum();
}
public boolean sameType(InsnArg arg) {
return this.getType().equals(arg.getType());
}
public boolean sameCodeVar(RegisterArg arg) {
return this.getSVar().getCodeVar() == arg.getSVar().getCodeVar();
}
@@ -1,10 +1,7 @@
package jadx.core.dex.visitors;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import jadx.api.plugins.input.data.AccessFlags;
@@ -63,7 +60,6 @@ public class ClassModifier extends AbstractVisitor {
removeSyntheticFields(cls);
cls.getMethods().forEach(ClassModifier::removeSyntheticMethods);
cls.getMethods().forEach(ClassModifier::removeEmptyMethods);
cls.getMethods().forEach(ClassModifier::processAnonymousConstructor);
return false;
}
@@ -161,7 +157,8 @@ public class ClassModifier extends AbstractVisitor {
return;
}
// remove synthetic constructor for inner classes
if (mth.isConstructor() && mth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE)) {
if (mth.isConstructor()
&& (mth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE) || mth.contains(AFlag.ANONYMOUS_CONSTRUCTOR))) {
InsnNode insn = BlockUtils.getOnlyOneInsnFromMth(mth);
if (insn != null) {
List<RegisterArg> args = mth.getArgRegs();
@@ -173,26 +170,32 @@ public class ClassModifier extends AbstractVisitor {
}
private static boolean isRemovedClassInArgs(ClassNode cls, List<RegisterArg> mthArgs) {
boolean removedFound = false;
for (RegisterArg arg : mthArgs) {
ArgType argType = arg.getType();
if (!argType.isObject()) {
continue;
}
boolean remove = false;
ClassNode argCls = cls.root().resolveClass(argType);
if (argCls == null) {
// check if missing class from current top class
ClassInfo argClsInfo = ClassInfo.fromType(cls.root(), argType);
if (argClsInfo.isInner()
if (argClsInfo.getParentClass() != null
&& cls.getFullName().startsWith(argClsInfo.getParentClass().getFullName())) {
return true;
remove = true;
}
} else {
if (argCls.contains(AFlag.DONT_GENERATE) || isEmptySyntheticClass(argCls)) {
return true;
remove = true;
}
}
if (remove) {
arg.add(AFlag.REMOVE);
removedFound = true;
}
}
return false;
return removedFound;
}
/**
@@ -332,91 +335,6 @@ public class ClassModifier extends AbstractVisitor {
}
}
/**
* Remove super call and put into removed fields from anonymous constructor
*/
private static void processAnonymousConstructor(MethodNode mth) {
if (!mth.contains(AFlag.ANONYMOUS_CONSTRUCTOR)) {
return;
}
List<InsnNode> usedInsns = new ArrayList<>();
Map<InsnArg, FieldNode> argsMap = getArgsToFieldsMapping(mth, usedInsns);
for (Map.Entry<InsnArg, FieldNode> entry : argsMap.entrySet()) {
FieldNode field = entry.getValue();
if (field == null) {
continue;
}
InsnArg arg = entry.getKey();
field.addAttr(new FieldReplaceAttr(arg));
field.add(AFlag.DONT_GENERATE);
if (arg.isRegister()) {
arg.add(AFlag.SKIP_ARG);
SkipMethodArgsAttr.skipArg(mth, ((RegisterArg) arg));
}
}
for (InsnNode usedInsn : usedInsns) {
usedInsn.add(AFlag.DONT_GENERATE);
}
}
private static Map<InsnArg, FieldNode> getArgsToFieldsMapping(MethodNode mth, List<InsnNode> usedInsns) {
MethodInfo callMth = mth.getMethodInfo();
ClassNode cls = mth.getParentClass();
List<RegisterArg> argList = mth.getArgRegs();
ClassNode outerCls = mth.getUseIn().get(0).getParentClass();
int startArg = 0;
if (callMth.getArgsCount() != 0 && callMth.getArgumentsTypes().get(0).equals(outerCls.getClassInfo().getType())) {
startArg = 1;
}
Map<InsnArg, FieldNode> map = new LinkedHashMap<>();
int argsCount = argList.size();
for (int i = startArg; i < argsCount; i++) {
RegisterArg arg = argList.get(i);
InsnNode useInsn = getParentInsnSkipMove(arg);
if (useInsn == null) {
return Collections.emptyMap();
}
switch (useInsn.getType()) {
case IPUT:
FieldNode fieldNode = cls.searchField((FieldInfo) ((IndexInsnNode) useInsn).getIndex());
if (fieldNode == null || !fieldNode.getAccessFlags().isSynthetic()) {
return Collections.emptyMap();
}
map.put(arg, fieldNode);
usedInsns.add(useInsn);
break;
case CONSTRUCTOR:
ConstructorInsn superConstr = (ConstructorInsn) useInsn;
if (!superConstr.isSuper()) {
return Collections.emptyMap();
}
usedInsns.add(useInsn);
break;
default:
return Collections.emptyMap();
}
}
return map;
}
private static InsnNode getParentInsnSkipMove(RegisterArg arg) {
SSAVar sVar = arg.getSVar();
if (sVar.getUseCount() != 1) {
return null;
}
RegisterArg useArg = sVar.getUseList().get(0);
InsnNode parentInsn = useArg.getParentInsn();
if (parentInsn == null) {
return null;
}
if (parentInsn.getType() == InsnType.MOVE) {
return getParentInsnSkipMove(parentInsn.getResult());
}
return parentInsn;
}
private static boolean isNonDefaultConstructorExists(MethodNode defCtor) {
ClassNode parentClass = defCtor.getParentClass();
for (MethodNode mth : parentClass.getMethods()) {
@@ -222,9 +222,9 @@ public class ConstInlineVisitor extends AbstractVisitor {
}
if (parentInsn.getType() == InsnType.CONSTRUCTOR) {
// don't inline into anonymous call if it can be inlined later
ConstructorInsn ctrInsn = (ConstructorInsn) parentInsn;
MethodNode ctrMth = mth.root().getMethodUtils().resolveMethod(ctrInsn);
if (ctrMth != null && ctrMth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE)) {
MethodNode ctrMth = mth.root().getMethodUtils().resolveMethod((ConstructorInsn) parentInsn);
if (ctrMth != null
&& (ctrMth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE) || ctrMth.contains(AFlag.ANONYMOUS_CONSTRUCTOR))) {
return false;
}
}
@@ -15,12 +15,12 @@ import org.slf4j.LoggerFactory;
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.attributes.AType;
import jadx.core.dex.attributes.nodes.DeclareVariablesAttr;
import jadx.core.dex.instructions.BaseInvokeNode;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.CodeVar;
import jadx.core.dex.instructions.args.RegisterArg;
import jadx.core.dex.instructions.args.SSAVar;
import jadx.core.dex.instructions.mods.ConstructorInsn;
import jadx.core.dex.nodes.IBlock;
import jadx.core.dex.nodes.IContainer;
import jadx.core.dex.nodes.IRegion;
@@ -116,15 +116,26 @@ public class ProcessVariables extends AbstractVisitor {
}
private boolean isArgUnused(MethodNode mth, RegisterArg arg) {
if (arg.contains(AFlag.REMOVE) || arg.contains(AFlag.SKIP_ARG)) {
if (arg.contains(AFlag.REMOVE)) {
return true;
}
// check constructors for removed args
InsnNode parentInsn = arg.getParentInsn();
if (parentInsn instanceof BaseInvokeNode
&& mth.root().getMethodUtils().isSkipArg(((BaseInvokeNode) parentInsn), arg)) {
arg.add(AFlag.DONT_GENERATE);
arg.add(AFlag.REMOVE);
return true;
if (parentInsn != null
&& parentInsn.getType() == InsnType.CONSTRUCTOR
&& parentInsn.contains(AType.METHOD_DETAILS)) {
MethodNode resolveMth = mth.root().getMethodUtils().resolveMethod(((ConstructorInsn) parentInsn));
if (resolveMth != null && resolveMth.contains(AType.SKIP_MTH_ARGS)) {
int insnPos = parentInsn.getArgIndex(arg);
List<RegisterArg> mthArgs = resolveMth.getArgRegs();
if (0 <= insnPos && insnPos < mthArgs.size()) {
RegisterArg mthArg = mthArgs.get(insnPos);
if (mthArg.contains(AFlag.REMOVE) && arg.sameType(mthArg)) {
arg.add(AFlag.DONT_GENERATE);
return true;
}
}
}
}
return false;
}
@@ -1,6 +1,7 @@
package jadx.tests.external;
import java.io.File;
import java.util.function.BiFunction;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -15,6 +16,7 @@ import jadx.api.JadxDecompiler;
import jadx.api.JadxInternalAccess;
import jadx.api.metadata.ICodeAnnotation;
import jadx.api.metadata.ICodeNodeRef;
import jadx.api.metadata.annotations.NodeDeclareRef;
import jadx.core.dex.nodes.ClassNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.nodes.RootNode;
@@ -148,26 +150,39 @@ public abstract class BaseExternalTest extends TestUtils {
private String cutMethodCode(ICodeInfo codeInfo, MethodNode mth) {
int startPos = getCommentStartPos(codeInfo, mth.getDefPosition());
int stopPos = getNextNodePos(mth, codeInfo);
int stopPos = getMethodEnd(mth, codeInfo);
return codeInfo.getCodeStr().substring(startPos, stopPos);
}
private int getNextNodePos(MethodNode mth, ICodeInfo codeInfo) {
int pos = mth.getDefPosition() + 1;
while (true) {
ICodeNodeRef nodeBelow = codeInfo.getCodeMetadata().getNodeBelow(pos);
if (nodeBelow == null) {
return codeInfo.getCodeStr().length();
private int getMethodEnd(MethodNode mth, ICodeInfo codeInfo) {
// skip nested nodes DEF/END until first unpaired END annotation (end of this method)
Integer end = codeInfo.getCodeMetadata().searchDown(mth.getDefPosition() + 1, new BiFunction<>() {
int nested = 0;
@Override
public Integer apply(Integer pos, ICodeAnnotation ann) {
switch (ann.getAnnType()) {
case DECLARATION:
ICodeNodeRef node = ((NodeDeclareRef) ann).getNode();
switch (node.getAnnType()) {
case CLASS:
case METHOD:
nested++;
break;
}
break;
case END:
if (nested == 0) {
return pos;
}
nested--;
break;
}
return null;
}
if (nodeBelow.getAnnType() != ICodeAnnotation.AnnType.METHOD) {
return nodeBelow.getDefPosition();
}
MethodNode nodeMth = (MethodNode) nodeBelow;
if (nodeMth.getParentClass().equals(mth.getParentClass())) { // skip methods from anonymous classes
return getCommentStartPos(codeInfo, nodeMth.getDefPosition());
}
pos = nodeMth.getDefPosition() + 1;
}
});
return end != null ? end : codeInfo.getCodeStr().length();
}
protected int getCommentStartPos(ICodeInfo codeInfo, int pos) {
@@ -5,12 +5,9 @@ import java.util.regex.Pattern;
import org.junit.jupiter.api.Test;
import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.IntegrationTest;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
public class TestConditions3 extends IntegrationTest {
@@ -59,12 +56,10 @@ public class TestConditions3 extends IntegrationTest {
@Test
public void test() {
ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();
assertThat(code, containsString("return null;"));
assertThat(code, not(containsString("else")));
assertThat(code, not(containsString("AnonymousClass_1")));
assertThat(getClassNode(TestCls.class))
.code()
.contains("return null;")
.doesNotContain("else")
.doesNotContain("AnonymousClass_1");
}
}
@@ -0,0 +1,35 @@
package jadx.tests.integration.inner;
import org.junit.jupiter.api.Test;
import jadx.tests.api.IntegrationTest;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
/**
* Don't inline const string into anonymous class constructor
*/
public class TestAnonymousClass21 extends IntegrationTest {
@SuppressWarnings("Convert2Lambda")
public static class TestCls {
public void test() {
String str = "str";
new Thread(new Runnable() {
@Override
public void run() {
System.out.println(str);
}
}).start();
}
}
@Test
public void test() {
noDebugInfo();
assertThat(getClassNode(TestCls.class))
.code()
.containsOne("String str = \"str\";")
.containsOne("System.out.println(str);");
}
}