MatchContext immutability fixes

narumii · Sep 10, 2024 · f607aa4 · f607aa4
1 parent 0ca94ac
commit f607aa4
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 73 deletions.
diff --git a/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/Match.java b/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/Match.java
@@ -1,5 +1,6 @@
 package uwu.narumi.deobfuscator.api.asm.matcher;
 
+import org.jetbrains.annotations.ApiStatus;
 import org.objectweb.asm.tree.AbstractInsnNode;
 import org.objectweb.asm.tree.analysis.OriginalSourceValue;
 import uwu.narumi.deobfuscator.api.asm.InstructionContext;
@@ -24,83 +25,90 @@ public abstract class Match {
   /**
    * Tests given instruction if it matches current {@link Match}
    *
-   * @param context Instruction context
+   * @param insnContext Instruction context
    * @return If matches
    */
-  public boolean matches(InstructionContext context) {
-    return this.matchResult(context) != null;
+  public boolean matches(InstructionContext insnContext) {
+    return this.matchResult(insnContext) != null;
   }
 
   /**
-   * @return {@link MatchContext} if matches or {@code null} if it does not match
+   * Matches the instrustion and merges if successful
+   *
+   * @param insnContext         Instruction context
+   * @param currentMatchContext Match context
+   * @return If matches
    */
-  public MatchContext matchResult(InstructionContext context) {
-    return this.matchResult(MatchContext.of(context));
-  }
-
-  public boolean matches(MatchContext context) {
-    return this.matchResult(context) != null;
+  @ApiStatus.Internal
+  public boolean matchAndMerge(InstructionContext insnContext, MatchContext currentMatchContext) {
+    MatchContext result = this.matchResult(insnContext);
+    if (result != null) {
+      currentMatchContext.merge(result);
+    }
+    return result != null;
   }
 
   /**
    * @return {@link MatchContext} if matches or {@code null} if it does not match
    */
-  public MatchContext matchResult(MatchContext context) {
-    boolean match = this.test(context);
+  public MatchContext matchResult(InstructionContext insnContext) {
+    // Create MatchContext
+    MatchContext context = MatchContext.of(insnContext);
+
+    // Test against this match
+    if (!this.test(context)) {
+      // No match
+      return null;
+    }
 
-    if (match) {
-      if (!this.stackMatches.isEmpty()) {
-        // Match values from stack
+    if (!this.stackMatches.isEmpty()) {
+      // Match values from stack
+
+      if (context.frame() == null) {
+        // If we expect stack values, then frame can't be null
+        return null;
+      }
 
-        if (context.frame() == null) {
-          // If we expect stack values, then frame can't be null
+      // Pop values from stack and match them
+      for (int i = 0; i < this.stackMatches.size(); i++) {
+        int stackValueIdx = context.frame().getStackSize() - (i + 1);
+        if (stackValueIdx < 0) {
+          // If the stack value should exist but does not, then it does not match
           return null;
         }
 
-        // Pop values from stack and match them
-        for (int i = 0; i < this.stackMatches.size(); i++) {
-          int stackValueIdx = context.frame().getStackSize() - (i + 1);
-          if (stackValueIdx < 0) {
-            // If the stack value should exist but does not, then it does not match
-            return null;
-          }
-
-          Match stackMatch = this.stackMatches.get(i);
-          if (stackMatch instanceof SkipMatch) {
-            // Skip match earlier
-            continue;
-          }
-
-          OriginalSourceValue sourceValue = context.frame().getStack(stackValueIdx);
-          if (!sourceValue.isOneWayProduced()) {
-            // We only want stack values that are one way produced
-            return null;
-          }
-
-          AbstractInsnNode stackValueInsn = sourceValue.getProducer();
-          MatchContext resultContext = stackMatch.matchResult(context.insnContext().of(stackValueInsn));
-          if (resultContext != null) {
-            // Merge contexts
-            context.merge(resultContext);
-          } else {
-            return null;
-          }
+        Match stackMatch = this.stackMatches.get(i);
+        if (stackMatch instanceof SkipMatch) {
+          // Skip match earlier
+          continue;
         }
-      }
 
-      if (this.saveId != null) {
-        // Save to storage
-        context.storage().put(this.saveId, context);
-      }
+        OriginalSourceValue sourceValue = context.frame().getStack(stackValueIdx);
+        if (!sourceValue.isOneWayProduced()) {
+          // We only want stack values that are one way produced
+          return null;
+        }
 
-      context.collectedInsns().add(context.insn());
+        AbstractInsnNode stackValueInsn = sourceValue.getProducer();
+        MatchContext resultContext = stackMatch.matchResult(context.insnContext().of(stackValueInsn));
+        if (resultContext != null) {
+          // Merge contexts
+          context.merge(resultContext);
+        } else {
+          return null;
+        }
+      }
+    }
 
-      // We have match!
-      return context.freeze();
+    if (this.saveId != null) {
+      // Save to storage
+      context.storage().put(this.saveId, context);
     }
 
-    // We don't have match
-    return null;
+    context.collectedInsns().add(context.insn());
+
+    // We have match!
+    return context.freeze();
   }
 
   /**
@@ -109,15 +117,15 @@ public MatchContext matchResult(MatchContext context) {
   protected abstract boolean test(MatchContext context);
 
   public Match and(Match match) {
-    return Match.predicate(context -> matches(context) && match.matches(context));
+    return Match.predicate(context -> matchAndMerge(context.insnContext(), context) && match.matchAndMerge(context.insnContext(), context));
   }
 
   public Match or(Match match) {
-    return Match.predicate(context -> matches(context) || match.matches(context));
+    return Match.predicate(context -> matchAndMerge(context.insnContext(), context) || match.matchAndMerge(context.insnContext(), context));
   }
 
   public Match not() {
-    return Match.predicate(context -> !matches(context));
+    return Match.predicate(context -> !matchAndMerge(context.insnContext(), context));
   }
 
   public Match defineTransformation(Transformation transformation) {

diff --git a/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/MatchContext.java b/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/MatchContext.java
@@ -36,7 +36,12 @@ public MatchContext freeze() {
    */
   public void merge(MatchContext other) {
     this.storage.putAll(other.storage);
-    this.collectedInsns.addAll(other.collectedInsns);
+    for (AbstractInsnNode insn : other.collectedInsns) {
+      // Don't allow duplicates
+      if (this.collectedInsns.contains(insn)) continue;
+
+      this.collectedInsns.add(insn);
+    }
   }
 
   /**

diff --git a/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/group/AllMatch.java b/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/group/AllMatch.java
@@ -18,7 +18,7 @@ public static AllMatch of(Match... matches) {
   @Override
   protected boolean test(MatchContext context) {
     for (Match match : matches) {
-      if (!match.matches(context)) return false;
+      if (!match.matchAndMerge(context.insnContext(), context)) return false;
     }
 
     return true;

diff --git a/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/group/AnyMatch.java b/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/group/AnyMatch.java
@@ -18,7 +18,7 @@ public static AnyMatch of(Match... matches) {
   @Override
   protected boolean test(MatchContext context) {
     for (Match match : matches) {
-      if (match.matches(context)) return true;
+      if (match.matchAndMerge(context.insnContext(), context)) return true;
     }
 
     return false;

diff --git a/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/group/NotMatch.java b/deobfuscator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/group/NotMatch.java
@@ -17,6 +17,6 @@ public static NotMatch of(Match match) {
 
   @Override
   protected boolean test(MatchContext context) {
-    return !match.matches(context);
+    return !match.matchAndMerge(context.insnContext(), context);
   }
 }
diff --git a/...ator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/group/PositionedMatch.java b/...ator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/group/PositionedMatch.java
@@ -20,13 +20,7 @@ private PositionedMatch(int offset, boolean skipAsmInstructions, Match match) {
 
   @Override
   protected boolean test(MatchContext context) {
-    MatchContext resultContext = this.match.matchResult(context.insnContext().of(walk(context.insn())));
-    if (resultContext != null) {
-      context.merge(resultContext);
-      return true;
-    } else {
-      return false;
-    }
+    return this.match.matchAndMerge(context.insnContext().of(walk(context.insn())), context);
   }
 
   private AbstractInsnNode walk(AbstractInsnNode node) {

diff --git a/...scator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/group/SequenceMatch.java b/...scator-api/src/main/java/uwu/narumi/deobfuscator/api/asm/matcher/group/SequenceMatch.java
@@ -82,10 +82,7 @@ protected boolean test(MatchContext context) {
 
       // Find match
       Match match = this.matches[matchIdx];
-      MatchContext resultContext = match.matchResult(currentInsnContext);
-      if (resultContext != null) {
-        context.merge(resultContext);
-      } else {
+      if (!match.matchAndMerge(currentInsnContext, context)) {
         // No match
         return false;
       }