Update ASM with important bug fix for DUP and DUP2. Some refactoring …

…and adapting changes for latest ASM
narumii · Sep 22, 2024 · 5a85dbe · 5a85dbe
1 parent ebf4732
commit 5a85dbe
Show file tree

Hide file tree

Showing 9 changed files with 102 additions and 80 deletions.
diff --git a/deobfuscator-api/pom.xml b/deobfuscator-api/pom.xml
@@ -21,8 +21,9 @@
          that are not in release version yet:
           - https://gitlab.ow2.org/asm/asm/-/merge_requests/414
           - https://gitlab.ow2.org/asm/asm/-/merge_requests/415
+          - https://gitlab.ow2.org/asm/asm/-/merge_requests/416
         -->
-        <asm.version>9.8-20240915.100933-24</asm.version>
+        <asm.version>9.8-20240921.152704-25</asm.version>
 
         <log4j.version>2.23.1</log4j.version>
         <slf4j.version>2.0.13</slf4j.version>

diff --git a/deobfuscator-api/src/main/java/org/objectweb/asm/tree/AbstractInsnNode.java b/deobfuscator-api/src/main/java/org/objectweb/asm/tree/AbstractInsnNode.java
@@ -37,6 +37,7 @@
 
 import org.jetbrains.annotations.Nullable;
 import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.NamedOpcodes;
 import org.objectweb.asm.Type;
 
 /**
@@ -642,5 +643,14 @@ public AbstractInsnNode walkNextUntil(
 
     return current;
   }
+
+  public String namedOpcode() {
+    return NamedOpcodes.map(this.getOpcode());
+  }
+
+  @Override
+  public String toString() {
+    return "(" + namedOpcode() + ") " + super.toString();
+  }
   // Narumii end
 }
diff --git a/...fuscator-api/src/main/java/org/objectweb/asm/tree/analysis/OriginalSourceInterpreter.java b/...fuscator-api/src/main/java/org/objectweb/asm/tree/analysis/OriginalSourceInterpreter.java
@@ -77,7 +77,15 @@ public OriginalSourceValue newValue(final Type type) {
     if (type == Type.VOID_TYPE) {
       return null;
     }
-    return new OriginalSourceValue(type == null ? 1 : type.getSize());
+    return new OriginalSourceValue(type == null ? 1 : type.getSize(), false);
+  }
+
+  @Override
+  public OriginalSourceValue newParameterValue(boolean isInstanceMethod, int local, Type type) {
+    if (type == Type.VOID_TYPE) {
+      return null;
+    }
+    return new OriginalSourceValue(type == null ? 1 : type.getSize(), true);
   }
 
   @Override
@@ -153,7 +161,7 @@ public OriginalSourceValue unaryOperation(final AbstractInsnNode insn, final Ori
 
       if (constant != null && constant.get() instanceof Number constNum) {
         Number result = AsmMathHelper.mathUnaryOperation(constNum, insn.getOpcode());
-        return new OriginalSourceValue(size, insn, OriginalSourceValue.ConstantValue.of(result));
+        return new OriginalSourceValue(size, insn, null, OriginalSourceValue.ConstantValue.of(result));
       }
     }
     // Narumii end
@@ -198,7 +206,7 @@ public OriginalSourceValue binaryOperation(
 
       if (constant1 != null && constant2 != null && constant1.get() instanceof Number constNum1 && constant2.get() instanceof Number constNum2) {
         Number result = AsmMathHelper.mathBinaryOperation(constNum1, constNum2, insn.getOpcode());
-        return new OriginalSourceValue(size, insn, OriginalSourceValue.ConstantValue.of(result));
+        return new OriginalSourceValue(size, insn, null, OriginalSourceValue.ConstantValue.of(result));
       }
     }
     // Narumii end
@@ -238,34 +246,32 @@ public void returnOperation(
 
   @Override
   public OriginalSourceValue merge(final OriginalSourceValue value1, final OriginalSourceValue value2) {
-    if (value1.insns instanceof SmallSet && value2.insns instanceof SmallSet) {
-      Set<AbstractInsnNode> setUnion =
-          ((SmallSet<AbstractInsnNode>) value1.insns)
-              .union((SmallSet<AbstractInsnNode>) value2.insns);
-      if (setUnion == value1.insns && value1.size == value2.size && Objects.equals(value1.copiedFrom, value2.copiedFrom)) { // Narumii
-        return value1;
+    if (value1.size != value2.size || !containsAll(value1.insns, value2.insns) || !Objects.equals(value1.copiedFrom, value2.copiedFrom)) {
+      Set<AbstractInsnNode> setUnion;
+      if (value1.insns instanceof SmallSet && value2.insns instanceof SmallSet) {
+        // Use optimized merging method
+        setUnion =
+            ((SmallSet<AbstractInsnNode>) value1.insns)
+                .union((SmallSet<AbstractInsnNode>) value2.insns);
       } else {
-        // Narumii start
+        setUnion = new HashSet<>();
+        setUnion.addAll(value1.insns);
+        setUnion.addAll(value2.insns);
+      }
+
+      // Single producer
+      if (setUnion.size() == 1) {
+        AbstractInsnNode producer = setUnion.iterator().next();
+
         OriginalSourceValue copiedFrom = null;
-        if (setUnion.size() == 1 && value1.copiedFrom != null && value2.copiedFrom != null) {
+        if (value1.copiedFrom != null && value2.copiedFrom != null) {
           copiedFrom = this.merge(value1.copiedFrom, value2.copiedFrom);
         }
-        return new OriginalSourceValue(Math.min(value1.size, value2.size), setUnion, copiedFrom);
-        // Narumii end
-      }
-    }
-    if (value1.size != value2.size || !containsAll(value1.insns, value2.insns) || !Objects.equals(value1.copiedFrom, value2.copiedFrom)) { // Narumii
-      HashSet<AbstractInsnNode> setUnion = new HashSet<>();
-      setUnion.addAll(value1.insns);
-      setUnion.addAll(value2.insns);
 
-      // Narumii start
-      OriginalSourceValue copiedFrom = null;
-      if (setUnion.size() == 1 && value1.copiedFrom != null && value2.copiedFrom != null) {
-        copiedFrom = this.merge(value1.copiedFrom, value2.copiedFrom);
+        return new OriginalSourceValue(Math.min(value1.size, value2.size), producer, copiedFrom, null);
       }
-      return new OriginalSourceValue(Math.min(value1.size, value2.size), setUnion, copiedFrom);
-      // Narumii end
+      // Multiple producers
+      return new OriginalSourceValue(Math.min(value1.size, value2.size), setUnion);
     }
     return value1;
   }

diff --git a/deobfuscator-api/src/main/java/org/objectweb/asm/tree/analysis/OriginalSourceValue.java b/deobfuscator-api/src/main/java/org/objectweb/asm/tree/analysis/OriginalSourceValue.java
@@ -4,9 +4,6 @@
 import org.objectweb.asm.Type;
 import org.objectweb.asm.tree.AbstractInsnNode;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -84,63 +81,58 @@ public class OriginalSourceValue extends SourceValue {
   @Nullable
   private ConstantValue constantValue = null;
 
-  /**
-   * Children that copied this source value
-   */
-  private final List<OriginalSourceValue> children = new ArrayList<>();
+  private final boolean isMethodParameter;
 
-  public OriginalSourceValue(int size) {
-    this(size, new SmallSet<>());
+  public OriginalSourceValue(int size, boolean isMethodParameter) {
+    super(size, new SmallSet<>());
+    this.isMethodParameter = isMethodParameter;
+    this.copiedFrom = null;
+    this.originalSource = this;
   }
 
   public OriginalSourceValue(int size, AbstractInsnNode insnNode) {
-    this(size, new SmallSet<>(insnNode));
+    this(size, insnNode, null, null);
   }
 
+  /**
+   * Create new {@link OriginalSourceValue} from multiple producers
+   *
+   * @param size Stack size of the value
+   * @param insnSet Set of instructions that produce this value
+   */
   public OriginalSourceValue(int size, Set<AbstractInsnNode> insnSet) {
-    this(size, insnSet, null);
+    super(size, insnSet);
+    this.isMethodParameter = false;
+    this.copiedFrom = null;
+    this.originalSource = this;
   }
 
   public OriginalSourceValue(AbstractInsnNode insnNode, OriginalSourceValue copiedFrom) {
-    this(copiedFrom.size, new SmallSet<>(insnNode), copiedFrom);
-  }
-
-  public OriginalSourceValue(int size, Set<AbstractInsnNode> insnSet, @Nullable OriginalSourceValue copiedFrom) {
-    this(size, insnSet, copiedFrom, null);
-  }
-
-  public OriginalSourceValue(int size, AbstractInsnNode insnNode, @Nullable ConstantValue constantValue) {
-    this(size, new SmallSet<>(insnNode), null, constantValue);
+    this(copiedFrom.size, insnNode, copiedFrom, null);
   }
 
   /**
-   * Create a new {@link OriginalSourceValue} with the given size, instruction set, copied from value and
-   * predicted constant value.
+   * Create new {@link OriginalSourceValue} from a single producer
    *
    * @param size Stack size of the value
-   * @param insnSet Set of instructions that produce this value
+   * @param insn An instruction that produces this value
    * @param copiedFrom The value from which this value was copied or null if it was not copied
    * @param constantValue Predicted constant value if exists
    */
-  public OriginalSourceValue(int size, Set<AbstractInsnNode> insnSet, @Nullable OriginalSourceValue copiedFrom, @Nullable ConstantValue constantValue) {
-    super(size, insnSet);
+  public OriginalSourceValue(int size, AbstractInsnNode insn, @Nullable OriginalSourceValue copiedFrom, @Nullable ConstantValue constantValue) {
+    super(size, new SmallSet<>(insn));
+    this.isMethodParameter = false;
     this.copiedFrom = copiedFrom;
-    this.originalSource = copiedFrom == null ? this : copiedFrom.originalSource;
-
-    if (copiedFrom != null) {
-      // Add child
-      copiedFrom.addChild(this);
-    }
+    this.originalSource = copiedFrom == null || copiedFrom.isMethodParameter ? this : copiedFrom.originalSource;
 
     if (constantValue != null) {
       // If the constant value is present, then use it
       this.constantValue = constantValue;
     } else if (copiedFrom != null) {
       // Copy constant value from copied value
       this.constantValue = copiedFrom.constantValue;
-    } else if (insnSet.size() == 1) {
+    } else {
       // Try to infer constant value from producer
-      AbstractInsnNode insn = insnSet.iterator().next();
       if (insn.isConstant()) {
         this.constantValue = ConstantValue.of(insn.asConstant());
       }
@@ -176,12 +168,8 @@ public ConstantValue getConstantValue() {
     return constantValue;
   }
 
-  void addChild(OriginalSourceValue child) {
-    this.children.add(child);
-  }
-
-  public List<OriginalSourceValue> getChildren() {
-    return Collections.unmodifiableList(children);
+  public boolean isMethodParameter() {
+    return isMethodParameter;
   }
 
   /**

diff --git a/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/impl/StackMatch.java b/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/impl/StackMatch.java
@@ -1,5 +1,6 @@
 package uwu.narumi.deobfuscator.api.asm.matcher.impl;
 
+import org.objectweb.asm.NamedOpcodes;
 import org.objectweb.asm.tree.AbstractInsnNode;
 import org.objectweb.asm.tree.analysis.Frame;
 import org.objectweb.asm.tree.analysis.OriginalSourceValue;
@@ -12,18 +13,24 @@
 public class StackMatch extends Match {
   private final int stackValueIdx;
   private final Match match;
+  private final boolean originalValue;
 
   /**
    * @param stackValueIdx Index of the value in the stack, starting from the top of the stack, so '0' is the top value.
    * @param match A {@link Match} to match against that stack value
    */
-  private StackMatch(int stackValueIdx, Match match) {
+  private StackMatch(int stackValueIdx, Match match, boolean originalValue) {
     this.stackValueIdx = stackValueIdx;
     this.match = match;
+    this.originalValue = originalValue;
   }
 
   public static StackMatch of(int stackValueIdx, Match match) {
-    return new StackMatch(stackValueIdx, match);
+    return new StackMatch(stackValueIdx, match, false);
+  }
+
+  public static StackMatch ofOriginal(int stackValueIdx, Match match) {
+    return new StackMatch(stackValueIdx, match, true);
   }
 
   @Override
@@ -51,6 +58,10 @@ protected boolean test(MatchContext context) {
 
     Frame<OriginalSourceValue> frame = context.frame();
     OriginalSourceValue sourceValue = frame.getStack(index);
+    if (this.originalValue) {
+      sourceValue = sourceValue.originalSource;
+    }
+
     if (!sourceValue.isOneWayProduced()) {
       // We only want stack values that are one way produced
       return false;

diff --git a/...s/src/main/java/uwu/narumi/deobfuscator/core/other/composed/ComposedZelixTransformer.java b/...s/src/main/java/uwu/narumi/deobfuscator/core/other/composed/ComposedZelixTransformer.java
@@ -29,12 +29,12 @@ public ComposedZelixTransformer(Map<String, String> classInitializationOrder) {
         // Fixes flow a bit
         ZelixUselessTryCatchRemoverTransformer::new,
 
+        ZelixParametersTransformer::new,
+
         () -> new ZelixLongEncryptionMPCTransformer(classInitializationOrder),
         InlineStaticFieldTransformer::new,
         UniversalNumberTransformer::new,
 
-        ZelixParametersTransformer::new,
-
         // Cleanup
         PeepholeCleanTransformer::new
     );

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,11 +9,16 @@
 import uwu.narumi.deobfuscator.api.helper.FramedInstructionsStream;
 import uwu.narumi.deobfuscator.api.transformer.Transformer;
 
+import java.util.ArrayList;
+import java.util.List;
+
 public class UselessPopCleanTransformer extends Transformer {
   public UselessPopCleanTransformer() {
     this.rerunOnChange = true;
   }
 
+  private final List<AbstractInsnNode> poppedDups = new ArrayList<>();
+
   @Override
   protected void transform(ClassWrapper scope, Context context) throws Exception {
     FramedInstructionsStream.of(scope, context)
@@ -76,22 +81,25 @@ private boolean canPop(OriginalSourceValue sourceValue) {
       // Nothing to remove. Probably a local variable
       return false;
     }
-    if (!sourceValue.getChildren().isEmpty()) {
-      // Other source values depends on this source value
-      return false;
-    }
 
     // Check if all producers of the source value are constants
     for (AbstractInsnNode producer : sourceValue.insns) {
-      if (!(producer.isConstant() || producer.getOpcode() == DUP)) {
-        return false;
-      }
+      // Can be popped if the value is constant
+      if (producer.isConstant()) continue;
+      // Can be popped if the value is DUP, and it wasn't popped yet
+      if (producer.getOpcode() == DUP && !poppedDups.contains(producer)) continue;
+
+      return false;
     }
     return true;
   }
 
   private void popSourceValue(OriginalSourceValue value, MethodNode methodNode) {
     for (AbstractInsnNode producer : value.insns) {
+      if (producer.getOpcode() == DUP) {
+        // Prevent popping DUP twice
+        poppedDups.add(producer);
+      }
       methodNode.instructions.remove(producer);
     }
   }

diff --git a/...src/main/java/uwu/narumi/deobfuscator/core/other/impl/zkm/ZelixParametersTransformer.java b/...src/main/java/uwu/narumi/deobfuscator/core/other/impl/zkm/ZelixParametersTransformer.java
@@ -61,7 +61,7 @@ public class ZelixParametersTransformer extends Transformer {
       .and(StackMatch.of(0, OpcodeMatch.of(AALOAD)
           .and(StackMatch.of(0, NumberMatch.numInteger().save("index")
               .and(StackMatch.of(0, OpcodeMatch.of(DUP)
-                  .and(StackMatch.of(0, OpcodeMatch.of(ALOAD).and(
+                  .and(StackMatch.ofOriginal(0, OpcodeMatch.of(ALOAD).and(
                       // The object array is always the first argument to method
                       Match.predicate(context -> {
                         return ((VarInsnNode) context.insn()).var == MethodHelper.getFirstParameterIdx(context.insnContext().methodNode());
@@ -145,7 +145,7 @@ protected void transform(ClassWrapper scope, Context context) throws Exception {
               InstructionContext insnContext = methodContext.newInsnContext(insn);
 
               OriginalSourceValue arrayAccess = insnContext.frame().getStack(insnContext.frame().getStackSize() - 1);
-              if (arrayAccess.getProducer().equals(loadArrayInsn)) {
+              if (arrayAccess.originalSource.isOneWayProduced() && arrayAccess.originalSource.getProducer().equals(loadArrayInsn)) {
                 methodNode.instructions.remove(loadArrayInsn); // ALOAD
                 methodNode.instructions.remove(insn); // POP
                 markChange();

diff --git a/testData/results/custom-jars/SnakeGame-obf-zkm/d.dec b/testData/results/custom-jars/SnakeGame-obf-zkm/d.dec
@@ -50,9 +50,7 @@ public class d extends Thread {
         long var3 = 12972172530972L;
         long var5 = 121593940001805L;
         long var7 = 41896598891982L;
-        long var10002 = 78566231858904L;
         short var9 = 18292;
-        var10002 = 78566231858904L;
         int var10 = -1604886824;
 
         while (true) {