fix: allow to inline variables around 'monitor-exit' in synchronized …

…block
skylot · Aug 9, 2020 · 13609a5 · 13609a5
1 parent d6ad21f
commit 13609a5
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 5 deletions.
diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/shrink/CodeShrinkVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/shrink/CodeShrinkVisitor.java
@@ -21,6 +21,7 @@
 import jadx.core.utils.BlockUtils;
 import jadx.core.utils.InsnList;
 import jadx.core.utils.InsnRemover;
+import jadx.core.utils.RegionUtils;
 import jadx.core.utils.exceptions.JadxRuntimeException;
 
 @JadxVisitor(
@@ -109,7 +110,7 @@ private static void checkInline(MethodNode mth, BlockNode block, InsnList insnLi
 			BlockNode assignBlock = BlockUtils.getBlockByInsn(mth, assignInsn);
 			if (assignBlock != null
 					&& assignInsn != arg.getParentInsn()
-					&& canMoveBetweenBlocks(assignInsn, assignBlock, block, argsInfo.getInsn())) {
+					&& canMoveBetweenBlocks(mth, assignInsn, assignBlock, block, argsInfo.getInsn())) {
 				if (assignInline) {
 					assignInline(mth, arg, assignInsn, assignBlock);
 				} else {
@@ -150,7 +151,7 @@ private static boolean inline(MethodNode mth, RegisterArg arg, InsnNode insn, Bl
 		return replaced;
 	}
 
-	private static boolean canMoveBetweenBlocks(InsnNode assignInsn, BlockNode assignBlock,
+	private static boolean canMoveBetweenBlocks(MethodNode mth, InsnNode assignInsn, BlockNode assignBlock,
 			BlockNode useBlock, InsnNode useInsn) {
 		if (!BlockUtils.isPathExists(assignBlock, useBlock)) {
 			return false;
@@ -176,8 +177,12 @@ private static boolean canMoveBetweenBlocks(InsnNode assignInsn, BlockNode assig
 		for (BlockNode block : pathsBlocks) {
 			if (block.contains(AFlag.DONT_GENERATE)) {
 				if (BlockUtils.checkLastInsnType(block, InsnType.MONITOR_EXIT)) {
-					// don't move from synchronized block
-					return false;
+					if (RegionUtils.isBlocksInSameRegion(mth, assignBlock, useBlock)) {
+						// allow move inside same synchronized region
+					} else {
+						// don't move from synchronized block
+						return false;
+					}
 				}
 				// skip checks for not generated blocks
 				continue;

diff --git a/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java b/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java
@@ -19,6 +19,8 @@
 import jadx.core.dex.nodes.IContainer;
 import jadx.core.dex.nodes.IRegion;
 import jadx.core.dex.nodes.InsnNode;
+import jadx.core.dex.nodes.MethodNode;
+import jadx.core.dex.regions.Region;
 import jadx.core.dex.trycatch.CatchAttr;
 import jadx.core.dex.trycatch.ExceptionHandler;
 import jadx.core.dex.trycatch.TryCatchBlock;
@@ -329,6 +331,26 @@ public static IContainer getBlockContainer(IContainer container, BlockNode block
 		}
 	}
 
+	/**
+	 * Check if two blocks in same region on same level
+	 * TODO: Add 'region' annotation to all blocks to speed up checks
+	 */
+	public static boolean isBlocksInSameRegion(MethodNode mth, BlockNode firstBlock, BlockNode secondBlock) {
+		Region region = mth.getRegion();
+		if (region == null) {
+			return false;
+		}
+		IContainer firstContainer = getBlockContainer(region, firstBlock);
+		if (firstContainer instanceof IRegion) {
+			if (firstContainer instanceof IBranchRegion) {
+				return false;
+			}
+			List<IContainer> subBlocks = ((IRegion) firstContainer).getSubBlocks();
+			return subBlocks.contains(secondBlock);
+		}
+		return false;
+	}
+
 	public static boolean isDominatedBy(BlockNode dom, IContainer cont) {
 		if (dom == cont) {
 			return true;

diff --git a/jadx-core/src/test/java/jadx/tests/integration/synchronize/TestSynchronized4.java b/jadx-core/src/test/java/jadx/tests/integration/synchronize/TestSynchronized4.java
@@ -26,6 +26,8 @@ public boolean test(int i) {
 	public void test() {
 		assertThat(getClassNodeFromSmali())
 				.code()
-				.containsOne("synchronized (this.obj) {");
+				.containsOne("synchronized (this.obj) {")
+				.containsOne("return call(this.obj, i);")
+				.containsOne("return getField() == null;");
 	}
 }