Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minor improvements to ExceptionalUnitGraph and ThrowableSet #1667

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 35 additions & 53 deletions src/main/java/soot/toolkits/exceptions/ThrowableSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,15 @@
* with a given name.
* </p>
*/

public class ThrowableSet {

private static final boolean INSTRUMENTING = false;
private final SootClass JAVA_LANG_OBJECT_CLASS = Scene.v().getObjectType().getSootClass();

// This pattern allows for the field to be static but to delay it's
// initialization until it's first use.
private static final class LazyInit {
static final SootClass JAVA_LANG_OBJECT_CLASS = Scene.v().getObjectType().getSootClass();
}

/**
* Set of exception types included within the set.
Expand Down Expand Up @@ -121,24 +125,23 @@ public class ThrowableSet {
* @param exclude
* The set of {@link AnySubType} objects representing the types to be excluded from the set.
*/
protected ThrowableSet(Set<RefLikeType> include, Set<AnySubType> exclude) {
private ThrowableSet(Set<RefLikeType> include, Set<AnySubType> exclude) {
exceptionsIncluded = getImmutable(include);
exceptionsExcluded = getImmutable(exclude);
// We don't need to clone include and exclude to guarantee
// immutability since ThrowableSet(Set,Set) is private to this
// class, where it is only called (via
// Manager.v().registerSetIfNew()) with arguments which the
// We don't need to clone include and exclude to guarantee immutability
// since ThrowableSet(Set,Set) is private to this class, where it is only
// called (via Manager.v().registerSetIfNew()) with arguments which the
// callers do not subsequently modify.
}

private static <T> Set<T> getImmutable(Set<T> in) {
if ((null == in) || in.isEmpty()) {
if (in == null || in.isEmpty()) {
return Collections.emptySet();
}
if (1 == in.size()) {
} else if (in.size() == 1) {
return Collections.singleton(in.iterator().next());
} else {
return Collections.unmodifiableSet(in);
}
return Collections.unmodifiableSet(in);
}

/**
Expand Down Expand Up @@ -201,9 +204,9 @@ private void addToMemoizedAdds(Object key, ThrowableSet value) {
*
* @return a set containing <code>e</code> as well as the exceptions in this set.
*
* @throws {@link
* ThrowableSet.IllegalStateException} if this <code>ThrowableSet</code> is the result of a
* {@link #whichCatchableAs(RefType)} operation and, thus, unable to represent the addition of <code>e</code>.
* @throws IllegalStateException
* if this <code>ThrowableSet</code> is the result of a {@link #whichCatchableAs(RefType)} operation and, thus,
* unable to represent the addition of <code>e</code>.
*/
public ThrowableSet add(RefType e) throws ThrowableSet.AlreadyHasExclusionsException {
if (INSTRUMENTING) {
Expand Down Expand Up @@ -275,7 +278,7 @@ public ThrowableSet add(RefType e) throws ThrowableSet.AlreadyHasExclusionsExcep

private boolean hasNoHierarchy(RefType type) {
final SootClass sootClass = type.getSootClass();
return !(sootClass.hasSuperclass() || JAVA_LANG_OBJECT_CLASS == sootClass);
return !(sootClass.hasSuperclass() || LazyInit.JAVA_LANG_OBJECT_CLASS == sootClass);
}

/**
Expand Down Expand Up @@ -535,14 +538,7 @@ private ThrowableSet add(Set<RefLikeType> addedExceptions) {
}
}
}

ThrowableSet result = null;
if (changes > 0) {
result = Manager.v().registerSetIfNew(resultSet, this.exceptionsExcluded);
} else {
result = this;
}
return result;
return (changes > 0) ? Manager.v().registerSetIfNew(resultSet, this.exceptionsExcluded) : this;
}

/**
Expand Down Expand Up @@ -570,14 +566,7 @@ private ThrowableSet remove(Set<RefLikeType> removedExceptions) {
}
}
}

ThrowableSet result = null;
if (changes > 0) {
result = Manager.v().registerSetIfNew(resultSet, this.exceptionsExcluded);
} else {
result = this;
}
return result;
return (changes > 0) ? Manager.v().registerSetIfNew(resultSet, this.exceptionsExcluded) : this;
}

/**
Expand Down Expand Up @@ -646,7 +635,7 @@ public boolean catchableAs(RefType catcher) {

if (exceptionsIncluded.contains(catcher)) {
if (INSTRUMENTING) {
if (exceptionsExcluded.size() == 0) {
if (exceptionsExcluded.isEmpty()) {
Manager.v().catchableAsFromMap++;
} else {
Manager.v().catchableAsFromSearch++;
Expand All @@ -655,7 +644,7 @@ public boolean catchableAs(RefType catcher) {
return true;
} else {
if (INSTRUMENTING) {
if (exceptionsExcluded.size() == 0) {
if (exceptionsExcluded.isEmpty()) {
Manager.v().catchableAsFromSearch++;
}
}
Expand All @@ -671,13 +660,11 @@ public boolean catchableAs(RefType catcher) {
} else {
RefType thrownBase = ((AnySubType) thrownType).getBase();
if (catcherHasNoHierarchy) {
if (thrownBase.equals(catcher) || thrownBase.getClassName().equals("java.lang.Throwable")) {
if (thrownBase.equals(catcher) || "java.lang.Throwable".equals(thrownBase.getClassName())) {
return true;
}
}
// At runtime, thrownType might be instantiated by any
// of thrownBase's subtypes, so:
else if (h.canStoreType(thrownBase, catcher) || h.canStoreType(catcher, thrownBase)) {
} else if (h.canStoreType(thrownBase, catcher) || h.canStoreType(catcher, thrownBase)) {
// At runtime, thrownType might be instantiated by any of thrownBase's subtypes
return true;
}
}
Expand Down Expand Up @@ -760,7 +747,7 @@ public Pair whichCatchableAs(RefType catcher) {
if (base.equals(catcher)) {
caughtIncluded = addExceptionToSet(inclusion, caughtIncluded);
} else {
if (base.getClassName().equals("java.lang.Throwable")) {
if ("java.lang.Throwable".equals(base.getClassName())) {
caughtIncluded = addExceptionToSet(catcher, caughtIncluded);
}
uncaughtIncluded = addExceptionToSet(inclusion, uncaughtIncluded);
Expand Down Expand Up @@ -818,7 +805,7 @@ private <T> Set<T> addExceptionToSet(T e, Set<T> set) {
*/
@Override
public String toString() {
StringBuffer buffer = new StringBuffer(this.toBriefString());
StringBuilder buffer = new StringBuilder(this.toBriefString());
buffer.append(":\n ");
for (RefLikeType ei : exceptionsIncluded) {
buffer.append('+');
Expand Down Expand Up @@ -882,7 +869,7 @@ private String toAbbreviatedString(Set<? extends RefLikeType> s, char connector)

Collection<RefLikeType> vmErrorThrowables = ThrowableSet.Manager.v().VM_ERRORS.exceptionsIncluded;
boolean containsAllVmErrors = s.containsAll(vmErrorThrowables);
StringBuffer buf = new StringBuffer();
StringBuilder buf = new StringBuilder();

if (containsAllVmErrors) {
buf.append(connector);
Expand Down Expand Up @@ -1161,7 +1148,8 @@ public static Manager v() {
* @return a <code>ThrowableSet</code> representing the set of exceptions corresponding to <code>include</code> -
* <code>exclude</code>.
*/
protected ThrowableSet registerSetIfNew(Set<RefLikeType> include, Set<AnySubType> exclude) {
private ThrowableSet registerSetIfNew(Set<RefLikeType> include, Set<AnySubType> exclude) {
//NOTE: This method must be private in accordance with the comment in the ThrowableSet constructor.
if (INSTRUMENTING) {
registrationCalls++;
}
Expand All @@ -1181,9 +1169,7 @@ protected ThrowableSet registerSetIfNew(Set<RefLikeType> include, Set<AnySubType
* @return a string listing the counts.
*/
public String reportInstrumentation() {
int setCount = registry.size();

StringBuffer buf = new StringBuffer("registeredSets: ").append(setCount).append("\naddsOfRefType: ")
StringBuilder buf = new StringBuilder("registeredSets: ").append(registry.size()).append("\naddsOfRefType: ")
.append(addsOfRefType).append("\naddsOfAnySubType: ").append(addsOfAnySubType).append("\naddsOfSet: ")
.append(addsOfSet).append("\naddsInclusionFromMap: ").append(addsInclusionFromMap)
.append("\naddsInclusionFromMemo: ").append(addsInclusionFromMemo).append("\naddsInclusionFromSearch: ")
Expand Down Expand Up @@ -1218,8 +1204,9 @@ public AlreadyHasExclusionsException(String s) {
* The return type for {@link ThrowableSet#whichCatchableAs(RefType)}, consisting of a pair of ThrowableSets.
*/
public static class Pair {
private ThrowableSet caught;
private ThrowableSet uncaught;

private final ThrowableSet caught;
private final ThrowableSet uncaught;

/**
* Constructs a <code>ThrowableSet.Pair</code>.
Expand Down Expand Up @@ -1270,10 +1257,7 @@ public boolean equals(Object o) {
return false;
}
Pair tsp = (Pair) o;
if (this.caught.equals(tsp.caught) && this.uncaught.equals(tsp.uncaught)) {
return true;
}
return false;
return this.caught.equals(tsp.caught) && this.uncaught.equals(tsp.uncaught);
}

@Override
Expand All @@ -1287,7 +1271,6 @@ public int hashCode() {

/**
* Comparator used to implement sortedThrowableIterator().
*
*/
private static class ThrowableComparator<T extends RefLikeType> implements java.util.Comparator<T> {

Expand Down Expand Up @@ -1322,6 +1305,5 @@ public int compare(T o1, T o2) {
return t1.toString().compareTo(t2.toString());
}
}

}
}
77 changes: 39 additions & 38 deletions src/main/java/soot/toolkits/graph/ExceptionalUnitGraph.java
Original file line number Diff line number Diff line change
Expand Up @@ -255,47 +255,50 @@ protected void initialize(ThrowAnalysis throwAnalysis, boolean omitExceptingUnit
* extending <code>ExceptionalUnitGraph</code>. If a <code>Unit</code> throws one or more exceptions which are
* caught within the method, it will be mapped to a <code>Collection</code> of <code>ExceptionDest</code>s
* describing the sets of exceptions that the <code>Unit</code> might throw to each {@link Trap}. But if all of a
* <code>Unit</code>'s exceptions escape the method, it will be mapped to <code>null</code, rather than to a
* <code>Unit</code>'s exceptions escape the method, it will be mapped to <code>null</code>, rather than to a
* <code>Collection</code> containing a single <code>ExceptionDest</code> with a <code>null</code> trap. (The
* special case for <code>Unit</code>s with no caught exceptions allows <code>buildExceptionDests()</code> to
* ignore completely <code>Unit</code>s which are outside the scope of all <code>Trap</code>s.)
* </p>
*/
protected Map<Unit, Collection<ExceptionDest>> buildExceptionDests(ThrowAnalysis throwAnalysis) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restructured this method a bit to avoid unnecessary work when body.getTraps() is empty and to avoid obtaining ThrowableSet.Manager.v().EMPTY repeatedly.

Chain<Unit> units = body.getUnits();
Map<Unit, ThrowableSet> unitToUncaughtThrowables = new LinkedHashMap<Unit, ThrowableSet>(units.size());
Map<Unit, Collection<ExceptionDest>> result = null;

// Record the caught exceptions.
for (Trap trap : body.getTraps()) {
RefType catcher = trap.getException().getType();
for (Iterator<Unit> unitIt = units.iterator(trap.getBeginUnit(), units.getPredOf(trap.getEndUnit())); unitIt
.hasNext();) {
Unit unit = unitIt.next();
ThrowableSet thrownSet = unitToUncaughtThrowables.get(unit);
if (thrownSet == null) {
thrownSet = throwAnalysis.mightThrow(unit);
}
final Chain<Trap> traps = body.getTraps();
if (!traps.isEmpty()) {
final ThrowableSet EMPTY = ThrowableSet.Manager.v().EMPTY;
final Chain<Unit> units = body.getUnits();
final Map<Unit, ThrowableSet> unitToUncaughtThrowables = new LinkedHashMap<Unit, ThrowableSet>(units.size());

// Record the caught exceptions.
for (Trap trap : traps) {
RefType catcher = trap.getException().getType();
for (Iterator<Unit> it = units.iterator(trap.getBeginUnit(), units.getPredOf(trap.getEndUnit())); it.hasNext();) {
Unit unit = it.next();
ThrowableSet thrownSet = unitToUncaughtThrowables.get(unit);
if (thrownSet == null) {
thrownSet = throwAnalysis.mightThrow(unit);
}

ThrowableSet.Pair catchableAs = thrownSet.whichCatchableAs(catcher);
if (!catchableAs.getCaught().equals(ThrowableSet.Manager.v().EMPTY)) {
result = addDestToMap(result, unit, trap, catchableAs.getCaught());
unitToUncaughtThrowables.put(unit, catchableAs.getUncaught());
} else {
assert thrownSet.equals(catchableAs.getUncaught()) : "ExceptionalUnitGraph.buildExceptionDests(): "
+ "catchableAs.caught == EMPTY, but catchableAs.uncaught != thrownSet" + System.getProperty("line.separator")
+ body.getMethod().getSubSignature() + " Unit: " + unit.toString() + System.getProperty("line.separator")
+ " catchableAs.getUncaught() == " + catchableAs.getUncaught().toString()
+ System.getProperty("line.separator") + " thrownSet == " + thrownSet.toString();
ThrowableSet.Pair catchableAs = thrownSet.whichCatchableAs(catcher);
if (!EMPTY.equals(catchableAs.getCaught())) {
result = addDestToMap(result, unit, trap, catchableAs.getCaught());
unitToUncaughtThrowables.put(unit, catchableAs.getUncaught());
} else {
assert thrownSet.equals(catchableAs.getUncaught()) : "ExceptionalUnitGraph.buildExceptionDests(): "
+ "catchableAs.caught == EMPTY, but catchableAs.uncaught != thrownSet" + System.getProperty("line.separator")
+ body.getMethod().getSubSignature() + " Unit: " + unit.toString() + System.getProperty("line.separator")
+ " catchableAs.getUncaught() == " + catchableAs.getUncaught().toString()
+ System.getProperty("line.separator") + " thrownSet == " + thrownSet.toString();
}
}
}
}

for (Map.Entry<Unit, ThrowableSet> entry : unitToUncaughtThrowables.entrySet()) {
Unit unit = entry.getKey();
ThrowableSet escaping = entry.getValue();
if (escaping != ThrowableSet.Manager.v().EMPTY) {
result = addDestToMap(result, unit, null, escaping);
for (Map.Entry<Unit, ThrowableSet> entry : unitToUncaughtThrowables.entrySet()) {
ThrowableSet escaping = entry.getValue();
if (escaping != EMPTY) {
result = addDestToMap(result, entry.getKey(), null, escaping);
}
}
}
return result == null ? Collections.emptyMap() : result;
Expand All @@ -322,8 +325,8 @@ protected Map<Unit, Collection<ExceptionDest>> buildExceptionDests(ThrowAnalysis
* @return a <code>Map</code> which whose contents are equivalent to the input <code>map</code>, plus the information that
* <code>u</code> throws <code>caught</code> to <code>t</code>.
*/
private Map<Unit, Collection<ExceptionDest>> addDestToMap(Map<Unit, Collection<ExceptionDest>> map, Unit u, Trap t,
ThrowableSet caught) {
protected Map<Unit, Collection<ExceptionDest>> addDestToMap(Map<Unit, Collection<ExceptionDest>> map, Unit u,
Copy link
Member Author

@tim-hoffman tim-hoffman Jun 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it protected to allow a subclass to use and/or override it.

Trap t, ThrowableSet caught) {
Collection<ExceptionDest> dests = (map == null ? null : map.get(u));
if (dests == null) {
if (t == null) {
Expand Down Expand Up @@ -546,7 +549,7 @@ public int hashCode() {
/**
* <p>
* Utility method for checking if a {@link Unit} might have side effects. It simply returns true for any unit which invokes
* a method directly or which might invoke static initializers indirectly (by creating a new object or by refering to a
* a method directly or which might invoke static initializers indirectly (by creating a new object or by referring to a
* static field; see sections 2.17.4, 2.17.5, and 5.5 of the Java Virtual Machine Specification).
* </p>
*
Expand Down Expand Up @@ -667,8 +670,6 @@ public Collection<ExceptionDest> getExceptionDests(final Unit u) {
Collection<ExceptionDest> result = unitToExceptionDests.get(u);
if (result == null) {
ExceptionDest e = new ExceptionDest(null, null) {
private ThrowableSet throwables;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExceptionDest already defines this field, it was just private so it was not accessible. So I removed the private from that field and remove this field so there would be no duplication.


@Override
public ThrowableSet getThrowables() {
if (null == throwables) {
Expand All @@ -677,14 +678,14 @@ public ThrowableSet getThrowables() {
return throwables;
}
};
return Collections.singletonList(e);
result = Collections.singletonList(e);
}
return result;
}

public static class ExceptionDest implements ExceptionalGraph.ExceptionDest<Unit> {
private Trap trap;
private ThrowableSet throwables;
private final Trap trap;
ThrowableSet throwables;

protected ExceptionDest(Trap trap, ThrowableSet throwables) {
this.trap = trap;
Expand Down Expand Up @@ -712,7 +713,7 @@ public Unit getHandlerNode() {

@Override
public String toString() {
StringBuffer buf = new StringBuffer();
StringBuilder buf = new StringBuilder();
buf.append(getThrowables());
buf.append(" -> ");
if (trap == null) {
Expand Down