fix: for static synchronized methods remove top synchronized block (#2493)

This commit is contained in:
Skylot
2025-05-24 21:45:38 +01:00
parent 3d8e5e5851
commit f33a2e4768
2 changed files with 42 additions and 30 deletions
@@ -9,7 +9,9 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.instructions.ConstClassNode;
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.IContainer;
@@ -141,22 +143,42 @@ public class SynchronizedRegionMaker {
List<IContainer> subBlocks = startRegion.getSubBlocks();
if (!subBlocks.isEmpty() && subBlocks.get(0) instanceof SynchronizedRegion) {
SynchronizedRegion synchRegion = (SynchronizedRegion) subBlocks.get(0);
InsnNode synchInsn = synchRegion.getEnterInsn();
if (!synchInsn.getArg(0).isThis()) {
LOG.warn("In synchronized method {}, top region not synchronized by 'this' {}", mth, synchInsn);
return;
InsnNode syncInsn = synchRegion.getEnterInsn();
if (canRemoveSyncBlock(mth, syncInsn)) {
// replace synchronized block with an inner region
startRegion.getSubBlocks().set(0, synchRegion.getRegion());
// remove 'monitor-enter' instruction
InsnRemover.remove(mth, syncInsn);
// remove 'monitor-exit' instruction
for (InsnNode exit : synchRegion.getExitInsns()) {
InsnRemover.remove(mth, exit);
}
// run region cleaner again
CleanRegions.process(mth);
// assume that CodeShrinker will be run after this
}
// replace synchronized block with inner region
startRegion.getSubBlocks().set(0, synchRegion.getRegion());
// remove 'monitor-enter' instruction
InsnRemover.remove(mth, synchInsn);
// remove 'monitor-exit' instruction
for (InsnNode exit : synchRegion.getExitInsns()) {
InsnRemover.remove(mth, exit);
}
// run region cleaner again
CleanRegions.process(mth);
// assume that CodeShrinker will be run after this
}
}
private static boolean canRemoveSyncBlock(MethodNode mth, InsnNode synchInsn) {
InsnArg syncArg = synchInsn.getArg(0);
if (mth.getAccessFlags().isStatic()) {
if (syncArg.isInsnWrap() && syncArg.isConst()) {
InsnNode constInsn = syncArg.unwrap();
if (constInsn.getType() == InsnType.CONST_CLASS) {
ArgType clsType = ((ConstClassNode) constInsn).getClsType();
if (clsType.equals(mth.getParentClass().getType())) {
return true;
}
}
}
mth.addWarnComment("In static synchronized method top region not synchronized by class const: " + syncArg);
} else {
if (syncArg.isThis()) {
return true;
}
mth.addWarnComment("In synchronized method top region not synchronized by 'this': " + syncArg);
}
return false;
}
}
@@ -2,14 +2,14 @@ package jadx.tests.integration.synchronize;
import org.junit.jupiter.api.Test;
import jadx.NotYetImplemented;
import jadx.tests.api.IntegrationTest;
import jadx.tests.api.utils.assertj.JadxAssertions;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
public class TestSynchronized2 extends IntegrationTest {
@SuppressWarnings("unused")
public static class TestCls {
@SuppressWarnings("unused")
private static synchronized boolean test(Object obj) {
return obj.toString() != null;
}
@@ -17,20 +17,10 @@ public class TestSynchronized2 extends IntegrationTest {
@Test
public void test() {
JadxAssertions.assertThat(getClassNode(TestCls.class))
assertThat(getClassNode(TestCls.class))
.code()
.contains("private static synchronized boolean test(Object obj) {")
.doesNotContain("synchronized (")
.contains("obj.toString() != null;");
}
@Test
@NotYetImplemented
public void test2() {
useDexInput(); // java bytecode don't add exception handlers
JadxAssertions.assertThat(getClassNode(TestCls.class))
.code()
.contains("return obj.toString() != null;")
.doesNotContain("synchronized (");
}
}