Skip to content

Commit

Permalink
fix: use correct reference for replaced bridge constructor (#1441)
Browse files Browse the repository at this point in the history
  • Loading branch information
skylot committed Apr 10, 2022
1 parent 83decc2 commit b57001d
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 30 deletions.
17 changes: 13 additions & 4 deletions jadx-core/src/main/java/jadx/core/codegen/InsnGen.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import jadx.core.dex.attributes.nodes.FieldReplaceAttr;
import jadx.core.dex.attributes.nodes.GenericInfoAttr;
import jadx.core.dex.attributes.nodes.LoopLabelAttr;
import jadx.core.dex.attributes.nodes.MethodReplaceAttr;
import jadx.core.dex.attributes.nodes.SkipMethodArgsAttr;
import jadx.core.dex.info.ClassInfo;
import jadx.core.dex.info.FieldInfo;
Expand Down Expand Up @@ -694,19 +695,27 @@ private void makeConstructor(ConstructorInsn insn, ICodeWriter code) throws Code
throw new JadxRuntimeException("Constructor 'self' invoke must be removed!");
}
MethodNode callMth = mth.root().resolveMethod(insn.getCallMth());
MethodNode refMth = callMth;
if (callMth != null) {
MethodReplaceAttr replaceAttr = callMth.get(AType.METHOD_REPLACE);
if (replaceAttr != null) {
refMth = replaceAttr.getReplaceMth();
}
}

if (insn.isSuper()) {
code.attachAnnotation(callMth);
code.attachAnnotation(refMth);
code.add("super");
} else if (insn.isThis()) {
code.attachAnnotation(callMth);
code.attachAnnotation(refMth);
code.add("this");
} else {
code.add("new ");
if (callMth == null || callMth.contains(AFlag.DONT_GENERATE)) {
if (refMth == null || refMth.contains(AFlag.DONT_GENERATE)) {
// use class reference if constructor method is missing (default constructor)
code.attachAnnotation(mth.root().resolveClass(insn.getCallMth().getDeclClass()));
} else {
code.attachAnnotation(callMth);
code.attachAnnotation(refMth);
}
mgen.getClassGen().addClsName(code, insn.getClassType());
GenericInfoAttr genericInfoAttr = insn.get(AType.GENERIC_INFO);
Expand Down
4 changes: 3 additions & 1 deletion jadx-core/src/main/java/jadx/core/dex/attributes/AType.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import jadx.core.dex.attributes.nodes.MethodBridgeAttr;
import jadx.core.dex.attributes.nodes.MethodInlineAttr;
import jadx.core.dex.attributes.nodes.MethodOverrideAttr;
import jadx.core.dex.attributes.nodes.MethodReplaceAttr;
import jadx.core.dex.attributes.nodes.MethodTypeVarsAttr;
import jadx.core.dex.attributes.nodes.PhiListAttr;
import jadx.core.dex.attributes.nodes.RegDebugInfoAttr;
Expand Down Expand Up @@ -65,11 +66,12 @@ public final class AType<T extends IJadxAttribute> implements IJadxAttrType<T> {
// method
public static final AType<LocalVarsDebugInfoAttr> LOCAL_VARS_DEBUG_INFO = new AType<>();
public static final AType<MethodInlineAttr> METHOD_INLINE = new AType<>();
public static final AType<MethodReplaceAttr> METHOD_REPLACE = new AType<>();
public static final AType<MethodBridgeAttr> BRIDGED_BY = new AType<>();
public static final AType<SkipMethodArgsAttr> SKIP_MTH_ARGS = new AType<>();
public static final AType<MethodOverrideAttr> METHOD_OVERRIDE = new AType<>();
public static final AType<MethodTypeVarsAttr> METHOD_TYPE_VARS = new AType<>();
public static final AType<AttrList<TryCatchBlockAttr>> TRY_BLOCKS_LIST = new AType<>();
public static final AType<MethodBridgeAttr> BRIDGED_BY = new AType<>();

// region
public static final AType<DeclareVariablesAttr> DECLARE_VARIABLES = new AType<>();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package jadx.core.dex.attributes.nodes;

import jadx.api.plugins.input.data.attributes.PinnedAttribute;
import jadx.core.dex.attributes.AType;
import jadx.core.dex.nodes.MethodNode;

/**
* Calls of method should be replaced by provided method (used for synthetic methods redirect)
*/
public class MethodReplaceAttr extends PinnedAttribute {

private final MethodNode replaceMth;

public MethodReplaceAttr(MethodNode replaceMth) {
this.replaceMth = replaceMth;
}

public MethodNode getReplaceMth() {
return replaceMth;
}

@Override
public AType<MethodReplaceAttr> getAttrType() {
return AType.METHOD_REPLACE;
}

@Override
public String toString() {
return "REPLACED_BY: " + replaceMth;
}
}
13 changes: 11 additions & 2 deletions jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import jadx.core.Consts;
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.attributes.nodes.FieldReplaceAttr;
import jadx.core.dex.attributes.nodes.MethodReplaceAttr;
import jadx.core.dex.attributes.nodes.SkipMethodArgsAttr;
import jadx.core.dex.info.AccessInfo;
import jadx.core.dex.info.ClassInfo;
Expand All @@ -31,6 +32,7 @@
import jadx.core.dex.nodes.FieldNode;
import jadx.core.dex.nodes.InsnNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.visitors.usage.UsageInfoVisitor;
import jadx.core.utils.BlockUtils;
import jadx.core.utils.InsnRemover;
import jadx.core.utils.exceptions.JadxException;
Expand Down Expand Up @@ -155,7 +157,7 @@ private static void removeSyntheticMethods(MethodNode mth) {
return;
}
// remove synthetic constructor for inner classes
if (af.isConstructor()) {
if (mth.isConstructor() && mth.contains(AFlag.METHOD_CANDIDATE_FOR_INLINE)) {
InsnNode insn = BlockUtils.getOnlyOneInsnFromMth(mth);
if (insn != null) {
List<RegisterArg> args = mth.getArgRegs();
Expand Down Expand Up @@ -210,7 +212,14 @@ private static void modifySyntheticMethod(ClassNode cls, MethodNode mth, InsnNod
SkipMethodArgsAttr.skipArg(mth, i);
}
}
mth.add(AFlag.DONT_GENERATE);
MethodInfo callMth = constr.getCallMth();
MethodNode callMthNode = cls.root().resolveMethod(callMth);
if (callMthNode != null) {
mth.addAttr(new MethodReplaceAttr(callMthNode));
mth.add(AFlag.DONT_GENERATE);
// code generation order should be already fixed for marked methods
UsageInfoVisitor.replaceMethodUsage(callMthNode, mth);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,17 @@ private static boolean canInline(MethodNode mth) {
}
AccessInfo accessFlags = mth.getAccessFlags();
boolean isSynthetic = accessFlags.isSynthetic() || mth.getName().contains("$");
return isSynthetic && accessFlags.isStatic();
return isSynthetic && (accessFlags.isStatic() || mth.isConstructor());
}

private static void fixClassDependencies(MethodNode mth) {
ClassNode parentClass = mth.getTopParentClass();
for (MethodNode useInMth : mth.getUseIn()) {
// remove possible cross dependency to force class with inline method to be processed before its
// usage
// remove possible cross dependency
// to force class with inline method to be processed before its usage
ClassNode useTopCls = useInMth.getTopParentClass();
parentClass.setDependencies(ListUtils.safeRemoveAndTrim(parentClass.getDependencies(), useTopCls));
useTopCls.addCodegenDep(parentClass);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package jadx.core.dex.visitors.usage;

import java.util.Collections;
import java.util.List;

import jadx.api.plugins.input.data.ICallSite;
import jadx.api.plugins.input.data.ICodeReader;
import jadx.api.plugins.input.data.IMethodHandle;
Expand All @@ -18,6 +21,7 @@
import jadx.core.dex.visitors.JadxVisitor;
import jadx.core.dex.visitors.OverrideMethodVisitor;
import jadx.core.dex.visitors.rename.RenameVisitor;
import jadx.core.utils.ListUtils;
import jadx.core.utils.input.InsnDataUtils;

@JadxVisitor(
Expand Down Expand Up @@ -134,4 +138,11 @@ private static void processInsn(RootNode root, MethodNode mth, InsnData insnData
}
}
}

public static void replaceMethodUsage(MethodNode mergeIntoMth, MethodNode sourceMth) {
List<MethodNode> mergedUsage = ListUtils.distinctMergeSortedLists(mergeIntoMth.getUseIn(), sourceMth.getUseIn());
mergedUsage.remove(sourceMth);
mergeIntoMth.setUseIn(mergedUsage);
sourceMth.setUseIn(Collections.emptyList());
}
}
3 changes: 2 additions & 1 deletion jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assumptions;
Expand Down Expand Up @@ -219,7 +220,7 @@ public List<ClassNode> decompileFiles(List<File> files) {
return sortedClsNodes;
}

@Nullable
@NotNull
public ClassNode searchCls(List<ClassNode> list, String clsName) {
for (ClassNode cls : list) {
if (cls.getClassInfo().getFullName().equals(clsName)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
package jadx.tests.api.utils.assertj;

import java.util.Map;

import org.assertj.core.api.AbstractObjectAssert;
import org.assertj.core.api.Assertions;

import jadx.api.CodePosition;
import jadx.api.ICodeInfo;
import jadx.core.dex.nodes.ClassNode;
import jadx.core.dex.nodes.ICodeNode;
import jadx.tests.api.IntegrationTest;

import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
import static org.assertj.core.api.Assertions.fail;

public class JadxClassNodeAssertions extends AbstractObjectAssert<JadxClassNodeAssertions, ClassNode> {
public JadxClassNodeAssertions(ClassNode cls) {
Expand Down Expand Up @@ -52,4 +57,22 @@ public JadxClassNodeAssertions runDecompiledAutoCheck(IntegrationTest testInstan
testInstance.runDecompiledAutoCheck(actual);
return this;
}

public void checkCodeAnnotationFor(String refStr, ICodeNode node) {
checkCodeAnnotationFor(refStr, 0, node);
}

public void checkCodeAnnotationFor(String refStr, int refOffset, ICodeNode node) {
ICodeInfo code = actual.getCode();
int codePos = code.getCodeStr().indexOf(refStr);
assertThat(codePos).describedAs("String '%s' not found", refStr).isNotEqualTo(-1);
int refPos = codePos + refOffset;
for (Map.Entry<CodePosition, Object> entry : code.getAnnotations().entrySet()) {
if (entry.getKey().getPos() == refPos) {
Assertions.assertThat(entry.getValue()).isEqualTo(node);
return;
}
}
fail("Annotation for reference string: '%s' at position %d not found", refStr, refPos);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@

import org.junit.jupiter.api.Test;

import jadx.api.CommentsLevel;
import jadx.core.dex.nodes.ClassNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.utils.ListUtils;
import jadx.tests.api.SmaliTest;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;

public class TestAnonymousClass14 extends SmaliTest {
// @formatter:off
Expand Down Expand Up @@ -44,11 +45,23 @@ public void use(Thread thread) {

@Test
public void test() {
ClassNode clsNode = getClassNodeFromSmaliFiles("inner", "TestAnonymousClass14", "OuterCls");
String code = clsNode.getCode().toString();
code = code.replaceAll("/\\*.*?\\*/", ""); // remove block comments
getArgs().setCommentsLevel(CommentsLevel.WARN);
ClassNode outerCls = getClassNodeFromSmaliFiles("OuterCls");
assertThat(outerCls).code()
.doesNotContain("synthetic", "AnonymousClass1")
.describedAs("only one constructor").containsOne("private TestCls(")
.describedAs("constructor without args").containsOne("private TestCls() {");

assertThat(code, not(containsString("AnonymousClass1")));
assertThat(code, not(containsString("synthetic")));
MethodNode makeTestClsMth = outerCls.searchMethodByShortName("makeTestCls");
assertThat(makeTestClsMth).isNotNull();

ClassNode testCls = searchCls(outerCls.getInnerClasses(), "TestCls");
MethodNode ctrMth = ListUtils.filterOnlyOne(testCls.getMethods(),
m -> m.isConstructor() && !m.getAccessFlags().isSynthetic());
assertThat(ctrMth).isNotNull();
assertThat(ctrMth.getUseIn()).hasSize(1);
assertThat(ctrMth.getUseIn().get(0)).isEqualTo(makeTestClsMth);

assertThat(outerCls).checkCodeAnnotationFor("new TestCls();", 4, ctrMth);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,20 @@

import org.junit.jupiter.api.Test;

import jadx.core.dex.nodes.ClassNode;
import jadx.api.CommentsLevel;
import jadx.tests.api.IntegrationTest;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;

public class TestOuterConstructorCall extends IntegrationTest {

@SuppressWarnings({ "InnerClassMayBeStatic", "unused" })
public static class TestCls {
private TestCls(Inner inner) {
System.out.println(inner);
}

private class Inner {
@SuppressWarnings("unused")
private TestCls test() {
return new TestCls(this);
}
Expand All @@ -26,11 +24,11 @@ private TestCls test() {

@Test
public void test() {
ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();

assertThat(code, containsString("private class Inner {"));
assertThat(code, containsString("return new TestOuterConstructorCall$TestCls(this);"));
assertThat(code, not(containsString("synthetic")));
getArgs().setCommentsLevel(CommentsLevel.WARN);
assertThat(getClassNode(TestCls.class))
.code()
.containsOne("class Inner {")
.containsOne("return new TestOuterConstructorCall$TestCls(this);")
.doesNotContain("synthetic", "this$0");
}
}

0 comments on commit b57001d

Please sign in to comment.