fix: don't inline named variables (#1338)
This commit is contained in:
@@ -196,17 +196,6 @@ public class SSAVar {
|
||||
return usedInPhi != null && !usedInPhi.isEmpty();
|
||||
}
|
||||
|
||||
public int getVariableUseCount() {
|
||||
int count = useList.size();
|
||||
if (usedInPhi == null) {
|
||||
return count;
|
||||
}
|
||||
for (PhiInsn phiInsn : usedInPhi) {
|
||||
count += phiInsn.getResult().getSVar().getUseCount();
|
||||
}
|
||||
return count;
|
||||
}
|
||||
|
||||
public void setName(String name) {
|
||||
if (name != null) {
|
||||
if (codeVar == null) {
|
||||
|
||||
@@ -300,20 +300,6 @@ public class InsnNode extends LineAttrNode {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Visit all args recursively (including inner instructions),
|
||||
* but excluding wrapped args
|
||||
*/
|
||||
public void visitArgs(Consumer<InsnArg> visitor) {
|
||||
for (InsnArg arg : getArguments()) {
|
||||
if (arg.isInsnWrap()) {
|
||||
((InsnWrapArg) arg).getWrapInsn().visitArgs(visitor);
|
||||
} else {
|
||||
visitor.accept(arg);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Visit this instruction and all inner (wrapped) instructions
|
||||
* To terminate visiting return non-null value
|
||||
@@ -336,6 +322,40 @@ public class InsnNode extends LineAttrNode {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Visit all args recursively (including inner instructions), but excluding wrapped args
|
||||
*/
|
||||
public void visitArgs(Consumer<InsnArg> visitor) {
|
||||
for (InsnArg arg : getArguments()) {
|
||||
if (arg.isInsnWrap()) {
|
||||
((InsnWrapArg) arg).getWrapInsn().visitArgs(visitor);
|
||||
} else {
|
||||
visitor.accept(arg);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Visit all args recursively (including inner instructions), but excluding wrapped args.
|
||||
* To terminate visiting return non-null value
|
||||
*/
|
||||
@Nullable
|
||||
public <R> R visitArgs(Function<InsnArg, R> visitor) {
|
||||
for (InsnArg arg : getArguments()) {
|
||||
R result;
|
||||
if (arg.isInsnWrap()) {
|
||||
InsnNode wrapInsn = ((InsnWrapArg) arg).getWrapInsn();
|
||||
result = wrapInsn.visitArgs(visitor);
|
||||
} else {
|
||||
result = visitor.apply(arg);
|
||||
}
|
||||
if (result != null) {
|
||||
return result;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* 'Soft' equals, don't compare arguments, only instruction specific parameters.
|
||||
*/
|
||||
|
||||
+24
-33
@@ -3,7 +3,7 @@ package jadx.core.dex.visitors.debuginfo;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
import java.util.OptionalInt;
|
||||
import java.util.Set;
|
||||
|
||||
import org.slf4j.Logger;
|
||||
@@ -73,31 +73,22 @@ public class DebugInfoApplyVisitor extends AbstractVisitor {
|
||||
if (Consts.DEBUG_TYPE_INFERENCE) {
|
||||
LOG.info("Apply debug info for method: {}", mth);
|
||||
}
|
||||
mth.getSVars().forEach(ssaVar -> collectVarDebugInfo(mth, ssaVar));
|
||||
mth.getSVars().forEach(ssaVar -> searchAndApplyVarDebugInfo(mth, ssaVar));
|
||||
|
||||
fixLinesForReturn(mth);
|
||||
fixNamesForPhiInsns(mth);
|
||||
}
|
||||
|
||||
private static void collectVarDebugInfo(MethodNode mth, SSAVar ssaVar) {
|
||||
Set<RegDebugInfoAttr> debugInfoSet = new HashSet<>(ssaVar.getUseCount() + 1);
|
||||
addRegDbdInfo(debugInfoSet, ssaVar.getAssign());
|
||||
ssaVar.getUseList().forEach(registerArg -> addRegDbdInfo(debugInfoSet, registerArg));
|
||||
|
||||
int dbgCount = debugInfoSet.size();
|
||||
if (dbgCount == 0) {
|
||||
searchDebugInfoByOffset(mth, ssaVar);
|
||||
private static void searchAndApplyVarDebugInfo(MethodNode mth, SSAVar ssaVar) {
|
||||
if (applyDebugInfo(mth, ssaVar, ssaVar.getAssign())) {
|
||||
return;
|
||||
}
|
||||
if (dbgCount == 1) {
|
||||
RegDebugInfoAttr debugInfo = debugInfoSet.iterator().next();
|
||||
applyDebugInfo(mth, ssaVar, debugInfo.getRegType(), debugInfo.getName());
|
||||
} else {
|
||||
mth.addInfoComment("Multiple debug info for " + ssaVar + ": " + debugInfoSet);
|
||||
for (RegDebugInfoAttr debugInfo : debugInfoSet) {
|
||||
applyDebugInfo(mth, ssaVar, debugInfo.getRegType(), debugInfo.getName());
|
||||
for (RegisterArg useArg : ssaVar.getUseList()) {
|
||||
if (applyDebugInfo(mth, ssaVar, useArg)) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
searchDebugInfoByOffset(mth, ssaVar);
|
||||
}
|
||||
|
||||
private static void searchDebugInfoByOffset(MethodNode mth, SSAVar ssaVar) {
|
||||
@@ -105,14 +96,12 @@ public class DebugInfoApplyVisitor extends AbstractVisitor {
|
||||
if (debugInfoAttr == null) {
|
||||
return;
|
||||
}
|
||||
Optional<Integer> max = ssaVar.getUseList().stream()
|
||||
.map(DebugInfoApplyVisitor::getInsnOffsetByArg)
|
||||
.max(Integer::compareTo);
|
||||
OptionalInt max = ssaVar.getUseList().stream().mapToInt(DebugInfoApplyVisitor::getInsnOffsetByArg).max();
|
||||
if (!max.isPresent()) {
|
||||
return;
|
||||
}
|
||||
int startOffset = getInsnOffsetByArg(ssaVar.getAssign());
|
||||
int endOffset = max.get();
|
||||
int endOffset = max.getAsInt();
|
||||
int regNum = ssaVar.getRegNum();
|
||||
for (ILocalVar localVar : debugInfoAttr.getLocalVars()) {
|
||||
if (localVar.getRegNum() == regNum) {
|
||||
@@ -144,24 +133,26 @@ public class DebugInfoApplyVisitor extends AbstractVisitor {
|
||||
return -1;
|
||||
}
|
||||
|
||||
public static void applyDebugInfo(MethodNode mth, SSAVar ssaVar, ArgType type, String varName) {
|
||||
TypeUpdateResult result = mth.root().getTypeUpdate().applyWithWiderAllow(mth, ssaVar, type);
|
||||
public static boolean applyDebugInfo(MethodNode mth, SSAVar ssaVar, RegisterArg arg) {
|
||||
RegDebugInfoAttr debugInfoAttr = arg.get(AType.REG_DEBUG_INFO);
|
||||
if (debugInfoAttr == null) {
|
||||
return false;
|
||||
}
|
||||
return applyDebugInfo(mth, ssaVar, debugInfoAttr.getRegType(), debugInfoAttr.getName());
|
||||
}
|
||||
|
||||
public static boolean applyDebugInfo(MethodNode mth, SSAVar ssaVar, ArgType type, String varName) {
|
||||
TypeUpdateResult result = mth.root().getTypeUpdate().applyWithWiderIgnoreUnknown(mth, ssaVar, type);
|
||||
if (result == TypeUpdateResult.REJECT) {
|
||||
if (Consts.DEBUG_TYPE_INFERENCE) {
|
||||
LOG.debug("Reject debug info of type: {} and name: '{}' for {}, mth: {}", type, varName, ssaVar, mth);
|
||||
}
|
||||
} else {
|
||||
if (NameMapper.isValidAndPrintable(varName)) {
|
||||
ssaVar.setName(varName);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private static void addRegDbdInfo(Set<RegDebugInfoAttr> debugInfo, RegisterArg reg) {
|
||||
RegDebugInfoAttr debugInfoAttr = reg.get(AType.REG_DEBUG_INFO);
|
||||
if (debugInfoAttr != null) {
|
||||
debugInfo.add(debugInfoAttr);
|
||||
if (NameMapper.isValidAndPrintable(varName)) {
|
||||
ssaVar.setName(varName);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+14
-6
@@ -89,25 +89,33 @@ public class DebugInfoAttachVisitor extends AbstractVisitor {
|
||||
}
|
||||
for (int i = start; i <= end; i++) {
|
||||
InsnNode insn = insnArr[i];
|
||||
if (insn != null) {
|
||||
attachDebugInfo(insn.getResult(), debugInfoAttr, regNum);
|
||||
for (InsnArg arg : insn.getArguments()) {
|
||||
attachDebugInfo(arg, debugInfoAttr, regNum);
|
||||
}
|
||||
if (insn == null) {
|
||||
continue;
|
||||
}
|
||||
int count = 0;
|
||||
for (InsnArg arg : insn.getArguments()) {
|
||||
count += attachDebugInfo(arg, debugInfoAttr, regNum);
|
||||
}
|
||||
if (count != 0) {
|
||||
// don't apply same info for result if applied to args
|
||||
continue;
|
||||
}
|
||||
attachDebugInfo(insn.getResult(), debugInfoAttr, regNum);
|
||||
}
|
||||
}
|
||||
|
||||
mth.addAttr(new LocalVarsDebugInfoAttr(localVars));
|
||||
}
|
||||
|
||||
private void attachDebugInfo(InsnArg arg, RegDebugInfoAttr debugInfoAttr, int regNum) {
|
||||
private int attachDebugInfo(InsnArg arg, RegDebugInfoAttr debugInfoAttr, int regNum) {
|
||||
if (arg instanceof RegisterArg) {
|
||||
RegisterArg reg = (RegisterArg) arg;
|
||||
if (regNum == reg.getRegNum()) {
|
||||
reg.addAttr(debugInfoAttr);
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
public static ArgType getVarType(MethodNode mth, ILocalVar var) {
|
||||
|
||||
@@ -382,6 +382,9 @@ public class IfMakerHelper {
|
||||
}
|
||||
if (useCount > 1) {
|
||||
forceInlineInsns.add(insn);
|
||||
} else {
|
||||
// allow only forced assign inline
|
||||
pass = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,12 +4,14 @@ import java.util.ArrayList;
|
||||
import java.util.BitSet;
|
||||
import java.util.List;
|
||||
import java.util.ListIterator;
|
||||
import java.util.Objects;
|
||||
import java.util.Set;
|
||||
|
||||
import jadx.core.dex.attributes.AFlag;
|
||||
import jadx.core.dex.instructions.InsnType;
|
||||
import jadx.core.dex.instructions.args.InsnArg;
|
||||
import jadx.core.dex.instructions.args.InsnWrapArg;
|
||||
import jadx.core.dex.instructions.args.Named;
|
||||
import jadx.core.dex.instructions.args.RegisterArg;
|
||||
import jadx.core.dex.instructions.args.SSAVar;
|
||||
import jadx.core.dex.nodes.BlockNode;
|
||||
@@ -76,7 +78,9 @@ public class CodeShrinkVisitor extends AbstractVisitor {
|
||||
|
||||
private static void checkInline(MethodNode mth, BlockNode block, InsnList insnList,
|
||||
List<WrapInfo> wrapList, ArgsInfo argsInfo, RegisterArg arg) {
|
||||
if (arg.contains(AFlag.DONT_INLINE)) {
|
||||
if (arg.contains(AFlag.DONT_INLINE)
|
||||
|| arg.getParentInsn() == null
|
||||
|| arg.getParentInsn().contains(AFlag.DONT_GENERATE)) {
|
||||
return;
|
||||
}
|
||||
SSAVar sVar = arg.getSVar();
|
||||
@@ -89,21 +93,34 @@ public class CodeShrinkVisitor extends AbstractVisitor {
|
||||
|| assignInsn.contains(AFlag.WRAPPED)) {
|
||||
return;
|
||||
}
|
||||
// allow inline only one use arg
|
||||
boolean assignInline = assignInsn.contains(AFlag.FORCE_ASSIGN_INLINE);
|
||||
if (!assignInline && sVar.getVariableUseCount() != 1) {
|
||||
if (!assignInline && sVar.isUsedInPhi()) {
|
||||
return;
|
||||
}
|
||||
List<RegisterArg> useList = sVar.getUseList();
|
||||
if (!useList.isEmpty()) {
|
||||
RegisterArg useArg = useList.get(0);
|
||||
// allow inline only one use arg
|
||||
int useCount = 0;
|
||||
for (RegisterArg useArg : sVar.getUseList()) {
|
||||
InsnNode parentInsn = useArg.getParentInsn();
|
||||
if (parentInsn != null && parentInsn.contains(AFlag.DONT_GENERATE)) {
|
||||
return;
|
||||
continue;
|
||||
}
|
||||
if (!assignInline && useArg.contains(AFlag.DONT_INLINE_CONST)) {
|
||||
return;
|
||||
}
|
||||
useCount++;
|
||||
}
|
||||
if (!assignInline && useCount != 1) {
|
||||
return;
|
||||
}
|
||||
if (!assignInline && sVar.getName() != null) {
|
||||
if (searchArgWithName(assignInsn, sVar.getName())) {
|
||||
// allow inline if name is reused in result
|
||||
} else if (varWithSameNameExists(mth, sVar)) {
|
||||
// allow inline if var name is duplicated
|
||||
} else {
|
||||
// reject inline of named variable
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
int assignPos = insnList.getIndex(assignInsn);
|
||||
@@ -127,6 +144,31 @@ public class CodeShrinkVisitor extends AbstractVisitor {
|
||||
}
|
||||
}
|
||||
|
||||
private static boolean varWithSameNameExists(MethodNode mth, SSAVar inlineVar) {
|
||||
for (SSAVar ssaVar : mth.getSVars()) {
|
||||
if (ssaVar == inlineVar || ssaVar.getCodeVar() == inlineVar.getCodeVar()) {
|
||||
continue;
|
||||
}
|
||||
if (Objects.equals(ssaVar.getName(), inlineVar.getName())) {
|
||||
return ssaVar.getUseCount() > inlineVar.getUseCount();
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private static boolean searchArgWithName(InsnNode assignInsn, String varName) {
|
||||
InsnArg result = assignInsn.visitArgs(insnArg -> {
|
||||
if (insnArg instanceof Named) {
|
||||
String argName = ((Named) insnArg).getName();
|
||||
if (Objects.equals(argName, varName)) {
|
||||
return insnArg;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
});
|
||||
return result != null;
|
||||
}
|
||||
|
||||
private static boolean assignInline(MethodNode mth, RegisterArg arg, InsnNode assignInsn, BlockNode assignBlock) {
|
||||
RegisterArg useArg = arg.getSVar().getUseList().get(0);
|
||||
InsnNode useInsn = useArg.getParentInsn();
|
||||
|
||||
@@ -66,7 +66,11 @@ public final class TypeUpdate {
|
||||
* Force type setting
|
||||
*/
|
||||
public TypeUpdateResult applyWithWiderIgnSame(MethodNode mth, SSAVar ssaVar, ArgType candidateType) {
|
||||
return apply(mth, ssaVar, candidateType, TypeUpdateFlags.FLAGS_WIDER_IGNSAME);
|
||||
return apply(mth, ssaVar, candidateType, TypeUpdateFlags.FLAGS_WIDER_IGNORE_SAME);
|
||||
}
|
||||
|
||||
public TypeUpdateResult applyWithWiderIgnoreUnknown(MethodNode mth, SSAVar ssaVar, ArgType candidateType) {
|
||||
return apply(mth, ssaVar, candidateType, TypeUpdateFlags.FLAGS_WIDER_IGNORE_UNKNOWN);
|
||||
}
|
||||
|
||||
private TypeUpdateResult apply(MethodNode mth, SSAVar ssaVar, ArgType candidateType, TypeUpdateFlags flags) {
|
||||
@@ -110,6 +114,9 @@ public final class TypeUpdate {
|
||||
}
|
||||
|
||||
TypeCompareEnum compareResult = comparator.compareTypes(candidateType, currentType);
|
||||
if (compareResult == TypeCompareEnum.UNKNOWN && updateInfo.getFlags().isIgnoreUnknown()) {
|
||||
return REJECT;
|
||||
}
|
||||
if (arg.isTypeImmutable() && currentType != ArgType.UNKNOWN) {
|
||||
// don't changed type
|
||||
if (compareResult == TypeCompareEnum.EQUAL) {
|
||||
|
||||
@@ -1,39 +1,58 @@
|
||||
package jadx.core.dex.visitors.typeinference;
|
||||
|
||||
import org.jetbrains.annotations.NotNull;
|
||||
|
||||
public class TypeUpdateFlags {
|
||||
private static final int ALLOW_WIDER = 1;
|
||||
private static final int IGNORE_SAME = 2;
|
||||
private static final int IGNORE_UNKNOWN = 4;
|
||||
|
||||
public static final TypeUpdateFlags FLAGS_EMPTY = new TypeUpdateFlags(false, false);
|
||||
public static final TypeUpdateFlags FLAGS_WIDER = new TypeUpdateFlags(true, false);
|
||||
public static final TypeUpdateFlags FLAGS_WIDER_IGNSAME = new TypeUpdateFlags(true, true);
|
||||
public static final TypeUpdateFlags FLAGS_EMPTY = build(0);
|
||||
public static final TypeUpdateFlags FLAGS_WIDER = build(ALLOW_WIDER);
|
||||
public static final TypeUpdateFlags FLAGS_WIDER_IGNORE_SAME = build(ALLOW_WIDER | IGNORE_SAME);
|
||||
public static final TypeUpdateFlags FLAGS_WIDER_IGNORE_UNKNOWN = build(ALLOW_WIDER | IGNORE_UNKNOWN);
|
||||
|
||||
private final boolean allowWider;
|
||||
private final boolean ignoreSame;
|
||||
private final int flags;
|
||||
|
||||
private TypeUpdateFlags(boolean allowWider, boolean ignoreSame) {
|
||||
this.allowWider = allowWider;
|
||||
this.ignoreSame = ignoreSame;
|
||||
@NotNull
|
||||
private static TypeUpdateFlags build(int flags) {
|
||||
return new TypeUpdateFlags(flags);
|
||||
}
|
||||
|
||||
private TypeUpdateFlags(int flags) {
|
||||
this.flags = flags;
|
||||
}
|
||||
|
||||
public boolean isAllowWider() {
|
||||
return allowWider;
|
||||
return (flags & ALLOW_WIDER) != 0;
|
||||
}
|
||||
|
||||
public boolean isIgnoreSame() {
|
||||
return ignoreSame;
|
||||
return (flags & IGNORE_SAME) != 0;
|
||||
}
|
||||
|
||||
public boolean isIgnoreUnknown() {
|
||||
return (flags & IGNORE_UNKNOWN) != 0;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
StringBuilder sb = new StringBuilder();
|
||||
if (allowWider) {
|
||||
if (isAllowWider()) {
|
||||
sb.append("ALLOW_WIDER");
|
||||
}
|
||||
if (ignoreSame) {
|
||||
if (isIgnoreSame()) {
|
||||
if (sb.length() != 0) {
|
||||
sb.append('|');
|
||||
}
|
||||
sb.append("IGNORE_SAME");
|
||||
}
|
||||
if (isIgnoreUnknown()) {
|
||||
if (sb.length() != 0) {
|
||||
sb.append('|');
|
||||
}
|
||||
sb.append("IGNORE_UNKNOWN");
|
||||
}
|
||||
return sb.toString();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -537,21 +537,7 @@ public abstract class IntegrationTest extends TestUtils {
|
||||
}
|
||||
|
||||
// Use only for debug purpose
|
||||
@Deprecated
|
||||
protected void outputCFG() {
|
||||
this.args.setCfgOutput(true);
|
||||
this.args.setRawCFGOutput(true);
|
||||
}
|
||||
|
||||
// Use only for debug purpose
|
||||
@Deprecated
|
||||
protected void printDisassemble() {
|
||||
this.printDisassemble = true;
|
||||
}
|
||||
|
||||
// Use only for debug purpose
|
||||
@Deprecated
|
||||
protected void outputRawCFG() {
|
||||
this.args.setRawCFGOutput(true);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
package jadx.tests.integration.generics;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.Map;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import jadx.tests.api.IntegrationTest;
|
||||
|
||||
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
|
||||
|
||||
public class TestConstructorGenerics extends IntegrationTest {
|
||||
|
||||
@SuppressWarnings({ "MismatchedQueryAndUpdateOfCollection", "RedundantOperationOnEmptyContainer" })
|
||||
public static class TestCls {
|
||||
public String test() {
|
||||
Map<String, String> map = new HashMap<>();
|
||||
return map.get("test");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test() {
|
||||
assertThat(getClassNode(TestCls.class))
|
||||
.code()
|
||||
.containsOne("Map<String, String> map = new HashMap<>();");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNoDebug() {
|
||||
noDebugInfo();
|
||||
assertThat(getClassNode(TestCls.class))
|
||||
.code()
|
||||
.containsOne("return (String) new HashMap().get(\"test\");");
|
||||
}
|
||||
}
|
||||
@@ -19,8 +19,7 @@ public class TestAnonymousClass10 extends IntegrationTest {
|
||||
public A test() {
|
||||
Random random = new Random();
|
||||
int a2 = random.nextInt();
|
||||
int a3 = a2 + 3;
|
||||
return new A(this, a2, a3, 4, 5, random.nextDouble()) {
|
||||
return new A(this, a2, a2 + 3, 4, 5, random.nextDouble()) {
|
||||
@Override
|
||||
public void m() {
|
||||
System.out.println(1);
|
||||
|
||||
@@ -32,6 +32,6 @@ public class TestNestedLoops2 extends IntegrationTest {
|
||||
String code = cls.getCode().toString();
|
||||
|
||||
assertThat(code, containsOne("for (int i = 0; i < list.size(); i++) {"));
|
||||
assertThat(code, containsOne("while (j < list.get(i).length()) {"));
|
||||
assertThat(code, containsOne("while (j < s.length()) {"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
|
||||
|
||||
public class TestSequentialLoops2 extends IntegrationTest {
|
||||
|
||||
@SuppressWarnings({ "unused", "FieldMayBeFinal" })
|
||||
public static class TestCls {
|
||||
private static char[] lowercases = new char[] { 'a' };
|
||||
|
||||
|
||||
@@ -14,7 +14,6 @@
|
||||
|
||||
fill-array-data v1, :array_0
|
||||
|
||||
.local v1, "arr":[J
|
||||
return v1
|
||||
|
||||
:array_0
|
||||
|
||||
Reference in New Issue
Block a user