Temporarily disable new var-folding behavior until regression is case "doNotFoldVarUsedInComparisonIfOriginalPossiblyUpdates" is fixed

Author: Col-E

recaf-core/src/main/java/software/coley/recaf/services/deobfuscation/transform/generic/VariableFoldingTransformer.java

@@ -10,7 +10,6 @@
 import org.objectweb.asm.Type;
 import org.objectweb.asm.tree.AbstractInsnNode;
 import org.objectweb.asm.tree.ClassNode;
-import org.objectweb.asm.tree.FieldInsnNode;
 import org.objectweb.asm.tree.IincInsnNode;
 import org.objectweb.asm.tree.InsnList;
 import org.objectweb.asm.tree.InsnNode;
@@ -46,7 +45,6 @@
 import software.coley.recaf.workspace.model.resource.WorkspaceResource;
 
 import java.util.Collections;
-import java.util.Iterator;
 import java.util.NavigableSet;
 import java.util.Set;
 import java.util.TreeSet;
@@ -260,7 +258,8 @@ else if (!throwingObserved) {
 
 					// Remove variable writes + usages if:
 					// - The variable is a redundant copy of an existing other variable
-					for (int keyX : states.keySet()) {
+					// TODO: Enable this after fixing the 'doNotFoldVarUsedInComparisonIfOriginalPossiblyUpdates' test
+					if (false) for (int keyX : states.keySet()) {
 						// Check if the current store is redundant with another variable.
 						int slotX = slotFromKey(keyX);
 						if (slotX != vin.var && isRedundantStore(states, instructions, i, slotX, vin.var, varSort)) {
@@ -339,13 +338,8 @@ private static boolean isRedundantStore(@Nonnull Int2ObjectMap<LocalAccessState>
 		if (!isMatchingStore(typeSort, writeVin.getOpcode()))
 			return false;
 
-		// Check if the prior instruction is a cast.
-		// We want to keep patterns like 'Collection -> List' being stored into new variables.
+		// Check if the prior instruction is the copy source of 'load x'.
 		AbstractInsnNode previousInsn = writeInsnY.getPrevious();
-		if (previousInsn == null || previousInsn.getOpcode() == CHECKCAST)
-			return false;
-
-		// Contents of both locals must be equal.
 		if (!(previousInsn instanceof VarInsnNode prevVin
 				&& isVarLoad(prevVin.getOpcode())
 				&& prevVin.var == slotX))
@@ -363,6 +357,10 @@ && isVarLoad(prevVin.getOpcode())
 				int opcode = vin.getOpcode();
 				if ((var == slotX || var == slotY) && isVarStore(opcode))
 					return false;
+			} else if (insn instanceof IincInsnNode iinc) {
+				int var = iinc.var;
+				if ((var == slotX || var == slotY))
+					return false;
 			}
 		}
 

recaf-core/src/test/java/software/coley/recaf/services/deobfuscation/BaseDeobfuscationTest.java

@@ -73,8 +73,10 @@ protected void validateNoTransformation(@Nonnull String assembly, @Nonnull List<
 		// Transforming should not actually result in any changes
 		JvmTransformResult result = assertDoesNotThrow(() -> newApplier().transformJvm(transformers));
 		assertTrue(result.getTransformerFailures().isEmpty(), "There were transformation failures");
-		assertTrue(result.getTransformedClasses().isEmpty(), "There were unexpected transformations applied");
-		assertTrue(result.getModifiedClassesPerTransformer().isEmpty(), "There were unexpected transformations applied");
+		if (!result.getTransformedClasses().isEmpty()) {
+			String transformedDisassembly = disassembleTransformed(result, isFullBody);
+			fail("There were unexpected transformations applied:\n\n" + transformedDisassembly);
+		}
 	}
 
 	protected void validateBeforeAfterDecompile(@Nonnull String assembly, @Nonnull List<Class<? extends JvmClassTransformer>> transformers,

recaf-core/src/test/java/software/coley/recaf/services/deobfuscation/FoldingDeobfuscationTest.java

@@ -2263,6 +2263,8 @@ void foldVar() {
 	}
 
 	@Test
+	@Disabled
+	/** TODO: Enable after fixing {@link #doNotFoldVarUsedInComparisonIfOriginalPossiblyUpdates()} */
 	void foldVarWithRedundantCopyVariable() {
 		String asm = """
 				.method public example ()I {
@@ -2351,6 +2353,8 @@ void foldVarWithRedundantCopyVariable() {
 	}
 
 	@Test
+	@Disabled
+	/** TODO: Enable after fixing {@link #doNotFoldVarUsedInComparisonIfOriginalPossiblyUpdates()} */
 	void foldVarWithWideRedundantCopyVariable() {
 		String asm = """
 				.method public static example (J)I {
@@ -2375,6 +2379,34 @@ void foldVarWithWideRedundantCopyVariable() {
 		});
 	}
 
+	@Test
+	@Disabled
+	void doNotFoldVarUsedInComparisonIfOriginalPossiblyUpdates() {
+		String asm = """
+				.method public static example (I)I {
+					parameters: { a },
+				    code: {
+				    A:
+				        iload a
+				        istore b
+				        iload a
+				        invokestatic Example.lookup (I)I
+				        istore a
+				        iload a
+				        iload b
+				        if_icmpne B
+				        iload a
+				        ireturn
+				    B:
+				        iload b
+				        ireturn
+				    C:
+				    }
+				}
+				""";
+		validateNoTransformation(asm, List.of(VariableFoldingTransformer.class, OpaqueConstantFoldingTransformer.class));
+	}
+
 	@Test
 	@Disabled
 	void foldVarWithSeriesOfRedundantBackToBackCycledWrites() {