fix useless popping two-sized values

narumii · Sep 11, 2024 · 906abc4 · 906abc4
1 parent f27d634
commit 906abc4
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 30 deletions.
diff --git a/deobfuscator-impl/src/main/java/uwu/narumi/deobfuscator/Deobfuscator.java b/deobfuscator-impl/src/main/java/uwu/narumi/deobfuscator/Deobfuscator.java
@@ -81,7 +81,7 @@ private void loadInput() {
       LOGGER.info("Loading jar file: {}", this.options.inputJar());
       // Load jar
       FileHelper.loadFilesFromZip(this.options.inputJar(), this::loadClass);
-      LOGGER.info("Loaded jar file: {}\n", this.options.inputJar());
+      LOGGER.info("Loaded jar file: {}", this.options.inputJar());
     }
 
     for (DeobfuscatorOptions.ExternalClass clazz : this.options.classes()) {
@@ -91,7 +91,7 @@ private void loadInput() {
         // Load class
         this.loadClass(clazz.relativePath(), inputStream.readAllBytes());
 
-        LOGGER.info("Loaded class: {}\n", clazz.relativePath());
+        LOGGER.info("Loaded class: {}", clazz.relativePath());
       } catch (IOException e) {
         LOGGER.error("Could not load class: {}", clazz.relativePath(), e);
       }

diff --git a/...va/uwu/narumi/deobfuscator/core/other/impl/clean/peephole/UselessPopCleanTransformer.java b/...va/uwu/narumi/deobfuscator/core/other/impl/clean/peephole/UselessPopCleanTransformer.java
@@ -9,7 +9,6 @@
 
 import java.util.stream.Stream;
 
-// TODO: Handle DUP2
 public class UselessPopCleanTransformer extends FramedInstructionsTransformer {
   public UselessPopCleanTransformer() {
     this.rerunOnChange = true;
@@ -28,18 +27,27 @@ protected boolean transformInstruction(Context context, InstructionContext insnC
     boolean shouldRemovePop = false;
 
     OriginalSourceValue firstValue = insnContext.frame().getStack(insnContext.frame().getStackSize() - 1);
-    if (insn.getOpcode() == POP && areProducersConstant(firstValue)) {
-      // Pop the value from the stack
-      popSourceValue(firstValue, insnContext.methodNode());
-      shouldRemovePop = true;
-    } else if (insn.getOpcode() == POP2) {
-      if (areTwoSizedValues(firstValue)) {
-        // Pop 2-sized value from the stack
+    // Return if we can't remove the source value
+    if (!isSourceValueRemovable(firstValue)) return false;
+
+    if (insn.getOpcode() == POP) {
+      if (areProducersConstant(firstValue)) {
+        // Pop the value from the stack
         popSourceValue(firstValue, insnContext.methodNode());
         shouldRemovePop = true;
+      }
+    } else if (insn.getOpcode() == POP2) {
+      if (firstValue.getSize() == 2) {
+        if (areProducersConstant(firstValue)) {
+          // Pop 2-sized value from the stack
+          popSourceValue(firstValue, insnContext.methodNode());
+          shouldRemovePop = true;
+        }
       } else {
         int index = insnContext.frame().getStackSize() - 2;
         OriginalSourceValue secondValue = index >= 0 ? insnContext.frame().getStack(insnContext.frame().getStackSize() - 2) : null;
+        // Return if we can't remove the source value
+        if (secondValue != null && !isSourceValueRemovable(firstValue)) return false;
 
         // Pop two values from the stack
         if (areProducersConstant(firstValue) && (secondValue == null || areProducersConstant(secondValue))) {
@@ -61,23 +69,15 @@ protected boolean transformInstruction(Context context, InstructionContext insnC
   }
 
   private boolean isSourceValueRemovable(OriginalSourceValue sourceValue) {
-    if (sourceValue.insns.isEmpty()) {
-      // Nothing to remove. Probably a local variable
-      return false;
-    }
-    if (!sourceValue.getChildren().isEmpty()) {
-      // Other source values depends on this source value
-      return false;
-    }
-
-    return true;
+    // Other source values depends on this source value
+    return sourceValue.getChildren().isEmpty();
   }
 
   /**
    * Checks if all producers of the source value are constants
    */
   private boolean areProducersConstant(OriginalSourceValue sourceValue) {
-    if (!isSourceValueRemovable(sourceValue)) return false;
+    if (sourceValue.insns.isEmpty()) return false;
 
     for (AbstractInsnNode producer : sourceValue.insns) {
       if (!(producer.isConstant() || producer.getOpcode() == DUP)) {
@@ -87,15 +87,6 @@ private boolean areProducersConstant(OriginalSourceValue sourceValue) {
     return true;
   }
 
-  /**
-   * Checks if all producers of the source value are 2-sized values
-   */
-  private boolean areTwoSizedValues(OriginalSourceValue sourceValue) {
-    if (!isSourceValueRemovable(sourceValue)) return false;
-
-    return sourceValue.getSize() == 2;
-  }
-
   private void popSourceValue(OriginalSourceValue value, MethodNode methodNode) {
     for (AbstractInsnNode producer : value.insns) {
       methodNode.instructions.remove(producer);