Merge pull request #2068 from MarcMil/fix-dex-annotations

Fix handling of Annotations in Dex code & UnconditionalBranchFolder
soot-oss · Apr 16, 2024 · 62926b9 · 62926b9
2 parents 7f43e77 + 29ef343
commit 62926b9
Show file tree

Hide file tree

Showing 6 changed files with 107 additions and 8 deletions.
diff --git a/src/main/java/soot/dexpler/DexAnnotation.java b/src/main/java/soot/dexpler/DexAnnotation.java
@@ -25,6 +25,8 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -113,6 +115,18 @@ public class DexAnnotation {
   private final SootClass clazz;
   private final Dependencies deps;
 
+  protected static Set<String> isHandled = new HashSet<>();
+
+  static {
+    isHandled.add(SootToDexUtils.getDexClassName(DALVIK_ANNOTATION_DEFAULT));
+    isHandled.add(SootToDexUtils.getDexClassName(DALVIK_ANNOTATION_SIGNATURE));
+    isHandled.add(SootToDexUtils.getDexClassName(DALVIK_ANNOTATION_MEMBERCLASSES));
+    isHandled.add(SootToDexUtils.getDexClassName(DALVIK_ANNOTATION_INNERCLASS));
+    isHandled.add(SootToDexUtils.getDexClassName(DALVIK_ANNOTATION_ENCLOSINGMETHOD));
+    isHandled.add(SootToDexUtils.getDexClassName(DALVIK_ANNOTATION_ENCLOSINGCLASS));
+    isHandled.add(SootToDexUtils.getDexClassName(JAVA_DEPRECATED));
+  }
+
   public DexAnnotation(SootClass clazz, Dependencies deps) {
     this.clazz = clazz;
     this.deps = deps;
@@ -161,8 +175,11 @@ public void handleClassAnnotation(ClassDef classDef) {
           // to methods through the creation of new
           // AnnotationDefaultTag.
           VisibilityAnnotationTag vt = (VisibilityAnnotationTag) t;
-          for (AnnotationTag a : vt.getAnnotations()) {
+          Iterator<AnnotationTag> it = vt.getAnnotations().iterator();
+          while (it.hasNext()) {
+            AnnotationTag a = it.next();
             if (a.getType().equals("Ldalvik/annotation/AnnotationDefault;")) {
+              it.remove();
               for (AnnotationElem ae : a.getElems()) {
                 if (ae instanceof AnnotationAnnotationElem) {
                   AnnotationAnnotationElem aae = (AnnotationAnnotationElem) ae;
@@ -228,8 +245,19 @@ public void handleClassAnnotation(ClassDef classDef) {
               }
             }
           }
-          if (!(vt.getVisibility() == AnnotationConstants.RUNTIME_INVISIBLE)) {
+          if (vt.getVisibility() == AnnotationConstants.RUNTIME_INVISIBLE) {
             clazz.addTag(vt);
+          } else {
+            // filter out the tags we handle explicitly
+            VisibilityAnnotationTag vbCopy = new VisibilityAnnotationTag(vt.getVisibility());
+            for (AnnotationTag tf : vt.getAnnotations()) {
+              if (!isHandled(tf)) {
+                vbCopy.addAnnotation(tf);
+              }
+            }
+            if (vbCopy.getAnnotations() != null && !vbCopy.getAnnotations().isEmpty()) {
+              clazz.addTag(vbCopy);
+            }
           }
         } else {
           clazz.addTag(t);
@@ -238,6 +266,10 @@ public void handleClassAnnotation(ClassDef classDef) {
     }
   }
 
+  private boolean isHandled(AnnotationTag tf) {
+    return isHandled.contains(tf.getType());
+  }
+
   private Type getSootType(AnnotationElem e) {
     Type annotationType;
     switch (e.getKind()) {

diff --git a/src/main/java/soot/jimple/BranchableStmt.java b/src/main/java/soot/jimple/BranchableStmt.java
@@ -0,0 +1,34 @@
+package soot.jimple;
+
+/*-
+ * #%L
+ * Soot - a J*va Optimization Framework
+ * %%
+ * Copyright (C) 1997 - 1999 Raja Vallee-Rai
+ * %%
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation, either version 2.1 of the
+ * License, or (at your option) any later version.
+ * 
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Lesser Public License for more details.
+ * 
+ * You should have received a copy of the GNU General Lesser Public
+ * License along with this program.  If not, see
+ * <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ * #L%
+ */
+
+import soot.Unit;
+import soot.UnitBox;
+
+public interface BranchableStmt extends Stmt {
+  public Unit getTarget();
+
+  public void setTarget(Unit target);
+
+  public UnitBox getTargetBox();
+}
diff --git a/src/main/java/soot/jimple/GotoStmt.java b/src/main/java/soot/jimple/GotoStmt.java
@@ -25,7 +25,7 @@
 import soot.Unit;
 import soot.UnitBox;
 
-public interface GotoStmt extends Stmt {
+public interface GotoStmt extends BranchableStmt {
   public Unit getTarget();
 
   public void setTarget(Unit target);

diff --git a/src/main/java/soot/jimple/internal/JIfStmt.java b/src/main/java/soot/jimple/internal/JIfStmt.java
@@ -34,6 +34,7 @@
 import soot.baf.Baf;
 import soot.jimple.AbstractJimpleValueSwitch;
 import soot.jimple.BinopExpr;
+import soot.jimple.BranchableStmt;
 import soot.jimple.ConvertToBaf;
 import soot.jimple.EqExpr;
 import soot.jimple.GeExpr;
@@ -50,7 +51,7 @@
 import soot.jimple.StmtSwitch;
 import soot.util.Switch;
 
-public class JIfStmt extends AbstractStmt implements IfStmt {
+public class JIfStmt extends AbstractStmt implements IfStmt, BranchableStmt {
 
   protected final ValueBox conditionBox;
   protected final UnitBox targetBox;

diff --git a/src/main/java/soot/jimple/toolkits/scalar/UnconditionalBranchFolder.java b/src/main/java/soot/jimple/toolkits/scalar/UnconditionalBranchFolder.java
@@ -39,6 +39,7 @@
 import soot.Unit;
 import soot.UnitBox;
 import soot.Value;
+import soot.jimple.BranchableStmt;
 import soot.jimple.ConditionExpr;
 import soot.jimple.GotoStmt;
 import soot.jimple.IfStmt;
@@ -213,6 +214,37 @@ public Result transform() {
                   succAsGoto.setTarget(ifTarget);
                   stmtAsIfStmt.setTarget(gotoTarget);
                   ifTarget = gotoTarget;
+                  // We need to check whether anyone has a goto to the "goto X", because this no has to also go
+                  // to Y
+
+                  // If we wouldn't do that, we would e.g. go from
+                  // if $i0 == 6 goto label04;
+                  // label03:
+                  // goto label21;
+                  // ...
+                  // goto label3;
+
+                  // to
+                  // if $i0 != 6 goto label21;
+                  // label03:
+                  // goto label04;
+                  // label04:
+                  // ...
+                  // goto label03;
+                  // this would alter the semantics, since the previous go-tos now go to the other branch!
+                  if (!succAsGoto.getBoxesPointingToThis().isEmpty()) {
+                    // we cannot simply use getBoxesPointingToThis, because we do not want to update
+                    // trap references
+                    for (Unit i : units) {
+                      if (i instanceof BranchableStmt) {
+                        BranchableStmt b = (BranchableStmt) i;
+                        if (b.getTarget() == succAsGoto) {
+                          b.setTarget(gotoTarget);
+                        }
+                      }
+                    }
+                  }
+
                   // NOTE: No need to remove the goto [successor] because it
                   // is processed by the next iteration of the main loop.
                   // NOTE: Nothing is removed here, it is a simple refactoring.

diff --git a/src/main/java/soot/toDex/DexPrinter.java b/src/main/java/soot/toDex/DexPrinter.java
@@ -496,9 +496,9 @@ private EncodedValue buildEncodedValueForAnnotation(AnnotationElem elem) {
         Collection<AnnotationElem> elems = e.getValue().getElems();
         if (!elems.isEmpty()) {
           elements = new ArrayList<AnnotationElement>();
-          Set<String> alreadyWritten = new HashSet<String>();
+          Set<AnnotationElem> alreadyWritten = new HashSet<AnnotationElem>();
           for (AnnotationElem ae : elems) {
-            if (!alreadyWritten.add(ae.getName())) {
+            if (!alreadyWritten.add(ae)) {
               throw new DexPrinterException("Duplicate annotation attribute: " + ae.getName());
             }
             elements.add(new ImmutableAnnotationElement(ae.getName(), buildEncodedValueForAnnotation(ae)));
@@ -670,7 +670,7 @@ protected void addClassDefinition(ClassDef classDef) {
     }
   }
 
-  private Set<Annotation> buildClassAnnotations(SootClass c) {
+  protected Set<Annotation> buildClassAnnotations(SootClass c) {
     Set<String> skipList = new HashSet<String>();
     Set<Annotation> annotations = buildCommonAnnotations(c, skipList);
 
@@ -855,7 +855,7 @@ private Set<Annotation> buildCommonAnnotations(AbstractHost host, Set<String> sk
     return annotations;
   }
 
-  private List<ImmutableAnnotation> buildVisibilityAnnotationTag(VisibilityAnnotationTag t, Set<String> skipList) {
+  protected List<ImmutableAnnotation> buildVisibilityAnnotationTag(VisibilityAnnotationTag t, Set<String> skipList) {
     if (t.getAnnotations() == null) {
       return Collections.emptyList();
     }