fix: redone shadowed fields handling (#897)

This commit is contained in:
Skylot
2020-04-27 22:10:21 +01:00
parent a7f315f596
commit 7f5092c0d5
7 changed files with 211 additions and 126 deletions
+1 -3
View File
@@ -49,13 +49,10 @@ public class Jadx {
if (args.isDebugInfo()) {
passes.add(new DebugInfoParseVisitor());
}
passes.add(new FindSuperUsageVisitor());
passes.add(new BlockSplitter());
if (args.isRawCFGOutput()) {
passes.add(DotGraphVisitor.dumpRaw());
}
passes.add(new BlockProcessor());
passes.add(new BlockExceptionHandler());
passes.add(new BlockFinish());
@@ -72,6 +69,7 @@ public class Jadx {
}
passes.add(new GenericTypesVisitor());
passes.add(new ShadowFieldVisitor());
passes.add(new DeboxingVisitor());
passes.add(new ModVisitor());
passes.add(new CodeShrinkVisitor());
@@ -1,49 +0,0 @@
package jadx.core.dex.visitors;
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.InsnArg;
import jadx.core.dex.instructions.args.RegisterArg;
import jadx.core.dex.nodes.InsnNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.visitors.blocksmaker.BlockSplitter;
import jadx.core.utils.exceptions.JadxException;
@JadxVisitor(
name = "FindSuperUsageVisitor",
desc = "Finds variables where a member of the super class is used and marks them.",
runBefore = BlockSplitter.class
)
public class FindSuperUsageVisitor extends AbstractVisitor {
@Override
public void visit(MethodNode mth) throws JadxException {
if (mth.isNoCode()) {
return;
}
process(mth);
}
private static void process(MethodNode methodNode) {
ArgType superClass = methodNode.getParentClass().getSuperClass();
if (superClass == null) {
return;
}
String superClassName = superClass.getObject();
if (superClassName.equals("java.lang.Object")) {
return;
}
for (InsnNode instruction : methodNode.getInstructions()) {
if (instruction != null) {
for (InsnArg argument : instruction.getArguments()) {
if (argument.isRegister()) {
ArgType argumentType = ((RegisterArg) argument).getInitType();
if (argumentType.isObject() && argumentType.getObject().equals(superClassName)) {
argument.add(AFlag.SUPER);
}
}
}
}
}
}
}
@@ -0,0 +1,181 @@
package jadx.core.dex.visitors;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.jetbrains.annotations.Nullable;
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.info.FieldInfo;
import jadx.core.dex.instructions.IndexInsnNode;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.InsnArg;
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.dex.nodes.RootNode;
import jadx.core.dex.visitors.shrink.CodeShrinkVisitor;
import jadx.core.dex.visitors.typeinference.TypeInferenceVisitor;
import jadx.core.utils.exceptions.JadxException;
@JadxVisitor(
name = "ShadowFieldVisitor",
desc = "Fix shadowed field access",
runAfter = TypeInferenceVisitor.class,
runBefore = CodeShrinkVisitor.class
)
public class ShadowFieldVisitor extends AbstractVisitor {
private Map<String, FieldFixInfo> fixInfoMap;
@Override
public void init(RootNode root) {
Map<String, FieldFixInfo> map = new HashMap<>();
for (ClassNode cls : root.getClasses(true)) {
Map<FieldInfo, FieldFixType> fieldFixMap = searchShadowedFields(cls);
if (!fieldFixMap.isEmpty()) {
FieldFixInfo fixInfo = new FieldFixInfo();
fixInfo.fieldFixMap = fieldFixMap;
map.put(cls.getRawName(), fixInfo);
}
}
this.fixInfoMap = map;
}
@Override
public void visit(MethodNode mth) throws JadxException {
if (mth.isNoCode()) {
return;
}
fixShadowFieldAccess(mth, fixInfoMap);
}
private static class FieldFixInfo {
Map<FieldInfo, FieldFixType> fieldFixMap;
}
private enum FieldFixType {
SUPER,
CAST
}
private static Map<FieldInfo, FieldFixType> searchShadowedFields(ClassNode thisCls) {
List<FieldNode> allFields = collectAllInstanceFields(thisCls);
if (allFields.isEmpty()) {
return Collections.emptyMap();
}
Map<String, List<FieldNode>> mapByName = groupByName(allFields);
mapByName.entrySet().removeIf(entry -> entry.getValue().size() == 1);
if (mapByName.isEmpty()) {
return Collections.emptyMap();
}
Map<FieldInfo, FieldFixType> fixMap = new HashMap<>();
for (List<FieldNode> fields : mapByName.values()) {
boolean fromThisCls = fields.get(0).getParentClass() == thisCls;
if (fromThisCls && fields.size() == 2) {
// only one super class contains same field => can use super
FieldNode otherField = fields.get(1);
if (otherField.getParentClass() != thisCls) {
fixMap.put(otherField.getFieldInfo(), FieldFixType.SUPER);
}
} else {
// several super classes contains same field => can't use super, need cast to exact class
for (FieldNode field : fields) {
if (field.getParentClass() != thisCls) {
fixMap.put(field.getFieldInfo(), FieldFixType.CAST);
}
}
}
}
return fixMap;
}
private static Map<String, List<FieldNode>> groupByName(List<FieldNode> allFields) {
Map<String, List<FieldNode>> groupByName = new HashMap<>(allFields.size());
for (FieldNode field : allFields) {
groupByName
.computeIfAbsent(field.getName(), k -> new ArrayList<>())
.add(field);
}
return groupByName;
}
private static List<FieldNode> collectAllInstanceFields(ClassNode cls) {
List<FieldNode> fieldsList = new ArrayList<>();
ClassNode currentClass = cls;
while (currentClass != null) {
for (FieldNode field : currentClass.getFields()) {
if (!field.getAccessFlags().isStatic()) {
fieldsList.add(field);
}
}
ArgType superClass = currentClass.getSuperClass();
if (superClass == null) {
break;
}
currentClass = cls.root().resolveClass(superClass);
}
return fieldsList;
}
private static void fixShadowFieldAccess(MethodNode mth, Map<String, FieldFixInfo> fixInfoMap) {
for (BlockNode block : mth.getBasicBlocks()) {
for (InsnNode insn : block.getInstructions()) {
processInsn(mth, insn, fixInfoMap);
}
}
}
private static void processInsn(MethodNode mth, InsnNode insn, Map<String, FieldFixInfo> fixInfoMap) {
FieldInfo fieldInfo = getFieldInfo(insn);
if (fieldInfo == null) {
return;
}
InsnArg arg = insn.getArg(insn.getArgsCount() - 1);
ArgType type = arg.getType();
if (!type.isTypeKnown() || !type.isObject()) {
return;
}
FieldFixInfo fieldFixInfo = fixInfoMap.get(type.getObject());
if (fieldFixInfo == null) {
return;
}
FieldFixType fieldFixType = fieldFixInfo.fieldFixMap.get(fieldInfo);
if (fieldFixType == null) {
return;
}
fixFieldAccess(mth, fieldInfo, fieldFixType, arg);
}
@Nullable
private static FieldInfo getFieldInfo(InsnNode insn) {
switch (insn.getType()) {
case IPUT:
case IGET:
return ((FieldInfo) ((IndexInsnNode) insn).getIndex());
default:
return null;
}
}
private static void fixFieldAccess(MethodNode mth, FieldInfo fieldInfo, FieldFixType fieldFixType, InsnArg arg) {
if (fieldFixType == FieldFixType.SUPER) {
if (arg.isThis()) {
// convert 'this' to 'super'
arg.add(AFlag.SUPER);
return;
}
}
// apply cast
InsnNode castInsn = new IndexInsnNode(InsnType.CAST, fieldInfo.getDeclClass().getType(), 1);
castInsn.addArg(arg.duplicate());
castInsn.add(AFlag.SYNTHETIC);
castInsn.add(AFlag.EXPLICIT_CAST);
arg.wrapInstruction(mth, castInsn, false);
}
}
@@ -2,46 +2,54 @@ package jadx.tests.integration.others;
import org.junit.jupiter.api.Test;
import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.SmaliTest;
import jadx.tests.api.IntegrationTest;
import static jadx.tests.api.utils.JadxMatchers.containsOne;
import static org.hamcrest.MatcherAssert.assertThat;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
public class TestShadowingSuperMember extends SmaliTest {
// @formatter:off
/*
public class C {
public class TestShadowingSuperMember extends IntegrationTest {
public static class TestCls {
public static class C {
public C(String s) {
}
}
public class A {
public int A00;
public static class A {
public int a00;
public A(String s) {
}
}
public class B extends A {
public C A00;
public static class B extends A {
public C a00;
public B(String str) {
super(str);
}
public int add(int b) {
return super.A00 + b;
return super.a00 + b;
}
public int sub(int b) {
return ((A) this).a00 - b;
}
}
*/
// @formatter:on
public void check() {
B b = new B("");
((A) b).a00 = 2;
assertThat(b.add(3)).isEqualTo(5);
assertThat(b.sub(3)).isEqualTo(-1);
}
}
@Test
public void test() {
allowWarnInCode();
ClassNode cls = getClassNodeFromSmaliFiles("B");
String code = cls.getCode().toString();
assertThat(code, containsOne("return super.A00 + "));
assertThat(getClassNode(TestCls.class))
.code()
.containsOne("return super.a00 + b;")
.containsOne("return super.a00 - b;")
.containsOne("((A) b).a00 = 2;");
}
}
@@ -1,16 +0,0 @@
.class public Lothers/A;
.super Ljava/lang/Object;
# instance fields
.field public A00:I
# direct methods
.method public constructor <init>(Ljava/lang/String;)V
.registers 3
.prologue
invoke-direct {p0}, Ljava/lang/Object;-><init>()V
return-void
.end method
@@ -1,25 +0,0 @@
.class public Lothers/B;
.super Lothers/A;
.field public A00:Lothers/C;
# direct methods
.method public constructor <init>(Ljava/lang/String;)V
.registers 3
.prologue
invoke-direct {p0, p1}, Lothers/A;-><init>(Ljava/lang/String;)V
return-void
.end method
.method public add(I)I
.registers 3
iget v1, p0, Lothers/A;->A00:I
add-int/2addr v1, p1
return v1
.end method
@@ -1,12 +0,0 @@
.class public Lothers/C;
.super Ljava/lang/Object;
.method public constructor <init>(Ljava/lang/String;)V
.registers 3
.prologue
invoke-direct {p0}, Ljava/lang/Object;-><init>()V
return-void
.end method