From 471910286b3944056b1e9e3821571a0379356c42 Mon Sep 17 00:00:00 2001 From: Marc Miltenberger Date: Mon, 15 Apr 2024 23:45:45 +0200 Subject: [PATCH 1/5] Fix handling of Annotations in Dex code --- src/main/java/soot/dexpler/DexAnnotation.java | 35 +++++++++++++++++-- src/main/java/soot/toDex/DexPrinter.java | 8 ++--- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/main/java/soot/dexpler/DexAnnotation.java b/src/main/java/soot/dexpler/DexAnnotation.java index 71cfc7228b2..d2fcdd97a7f 100644 --- 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 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 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,18 @@ 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 +265,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/toDex/DexPrinter.java b/src/main/java/soot/toDex/DexPrinter.java index 6430904e25e..8351ba0d284 100644 --- 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 elems = e.getValue().getElems(); if (!elems.isEmpty()) { elements = new ArrayList(); - Set alreadyWritten = new HashSet(); + Set alreadyWritten = new HashSet(); 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 buildClassAnnotations(SootClass c) { + protected Set buildClassAnnotations(SootClass c) { Set skipList = new HashSet(); Set annotations = buildCommonAnnotations(c, skipList); @@ -855,7 +855,7 @@ private Set buildCommonAnnotations(AbstractHost host, Set sk return annotations; } - private List buildVisibilityAnnotationTag(VisibilityAnnotationTag t, Set skipList) { + protected List buildVisibilityAnnotationTag(VisibilityAnnotationTag t, Set skipList) { if (t.getAnnotations() == null) { return Collections.emptyList(); } From a6a691f54fa59b456e3f8aff98a0fe46821bfb0b Mon Sep 17 00:00:00 2001 From: Marc Miltenberger Date: Mon, 15 Apr 2024 23:48:11 +0200 Subject: [PATCH 2/5] Stylecheck --- src/main/java/soot/dexpler/DexAnnotation.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/soot/dexpler/DexAnnotation.java b/src/main/java/soot/dexpler/DexAnnotation.java index d2fcdd97a7f..1a530a53ab7 100644 --- a/src/main/java/soot/dexpler/DexAnnotation.java +++ b/src/main/java/soot/dexpler/DexAnnotation.java @@ -245,9 +245,9 @@ public void handleClassAnnotation(ClassDef classDef) { } } } - if (vt.getVisibility() == AnnotationConstants.RUNTIME_INVISIBLE) + if (vt.getVisibility() == AnnotationConstants.RUNTIME_INVISIBLE) { clazz.addTag(vt); - else { + } else { // filter out the tags we handle explicitly VisibilityAnnotationTag vbCopy = new VisibilityAnnotationTag(vt.getVisibility()); for (AnnotationTag tf : vt.getAnnotations()) { @@ -255,8 +255,9 @@ public void handleClassAnnotation(ClassDef classDef) { vbCopy.addAnnotation(tf); } } - if (vbCopy.getAnnotations() != null && !vbCopy.getAnnotations().isEmpty()) + if (vbCopy.getAnnotations() != null && !vbCopy.getAnnotations().isEmpty()) { clazz.addTag(vbCopy); + } } } else { clazz.addTag(t); From 1e72bfc4952c9f21d5c091aa1aec11e719d3d505 Mon Sep 17 00:00:00 2001 From: Marc Miltenberger Date: Tue, 16 Apr 2024 09:04:53 +0200 Subject: [PATCH 3/5] Fix UnconditionalBranchFolder breaking code When a goto referenced a goto, which was touched by the condition inversion of UnconditionalBranchFolder, that goto would not be updated. Therefore, the other references to the goto point to the wrong effects of the wrong if branch after the optimization. --- src/main/java/soot/jimple/BranchableStmt.java | 12 +++++++ src/main/java/soot/jimple/GotoStmt.java | 2 +- .../java/soot/jimple/internal/JIfStmt.java | 3 +- .../scalar/UnconditionalBranchFolder.java | 32 +++++++++++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 src/main/java/soot/jimple/BranchableStmt.java diff --git a/src/main/java/soot/jimple/BranchableStmt.java b/src/main/java/soot/jimple/BranchableStmt.java new file mode 100644 index 00000000000..0c4a930ea35 --- /dev/null +++ b/src/main/java/soot/jimple/BranchableStmt.java @@ -0,0 +1,12 @@ +package soot.jimple; + +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 index 44946a594a5..d914df67ee1 100644 --- 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 index da0a7d5cc2a..f2dc6c6e2c1 100644 --- 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 index 2d3f507dd2c..c35c6602f93 100644 --- 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. From b0c5f7ba8c6a2fcdce4314550db6b25752d35fd2 Mon Sep 17 00:00:00 2001 From: Marc Miltenberger Date: Tue, 16 Apr 2024 09:14:49 +0200 Subject: [PATCH 4/5] Add missing license --- src/main/java/soot/jimple/BranchableStmt.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/main/java/soot/jimple/BranchableStmt.java b/src/main/java/soot/jimple/BranchableStmt.java index 0c4a930ea35..2bf97b1936f 100644 --- a/src/main/java/soot/jimple/BranchableStmt.java +++ b/src/main/java/soot/jimple/BranchableStmt.java @@ -1,5 +1,27 @@ 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 + * . + * #L% + */ + import soot.Unit; import soot.UnitBox; From 29ef3439519b65017914af702502eeffd81b0d77 Mon Sep 17 00:00:00 2001 From: Marc Miltenberger Date: Tue, 16 Apr 2024 09:21:39 +0200 Subject: [PATCH 5/5] trigger GitHub actions