Closed giltho closed 3 years ago
@StevenArzt Hi Steven, is this behaviour of WeakObjectType intended? Can you provide some explanation on this?
The WeakObjectType
is a type reference that can resolve to Object
, Serializable
or Cloneable
. We use it during type resolving when we don't know the precise type yet. Previously, SPARK added all three types to the set of possible types. It then created typing candidates based on the sets of possible types for all locals, i.e., made flat mapping with all possible combinations of types and locals. If you have a method with 50 - 100 locals and 3 possible types each, this does not scale. We therefore cut down on this combinatorial explosion by reducing the three types - when they occur together - to a special fake type, which we called WeakObjectType
. SPARK implicitly handles the case that this type actually gives you three choices in the end.
Resolving the SootClass
should work, if you keep the semantics that the class and class name are only placeholders and that you are actually dealing with three possible classes.
@StevenArzt Thank for the explanation! The problem caused issue is in the TypeManager.isUnresolved function: https://github.com/soot-oss/soot/blob/e4e9ff59167be047d624298ba0a5f329bc912226/src/main/java/soot/jimple/spark/internal/TypeManager.java#L80-L94
Without modifying the WeakOjbectType class, I would try to resolve the class in the isUnresolved function one more time as in the following code, do you agree with this fix?
public static boolean isUnresolved(Type type) {
if (type instanceof ArrayType) {
ArrayType at = (ArrayType) type;
type = at.getArrayElementType();
}
if (!(type instanceof RefType)) {
return false;
}
RefType rt = (RefType) type;
if (!rt.hasSootClass()) {
// try to resolve sootClass one more time.
SootClass c = Scene.v().forceResolve(rt.getClassName(), SootClass.HIERARCHY);
if (c == null) {
return true;
} else {
rt.setSootClass(c);
}
}
SootClass cl = rt.getSootClass();
return cl.resolvingLevel() < SootClass.HIERARCHY;
}
That should be fine, sounds good to me.
The new version unfortunately proved to be much slower than the original code. I decided to introduce a special-case for WeakObjectType
to keep the fix alive, while avoiding to resolve tons of classes that we don't really need.
Look good to me, though the performance did not reflected on time used in the GitHub actions. Not sure if we have tests for this. @giltho Did this fix/change solve your problems?
The taint wrapper tests in StubDroid (part of FlowDroid) went from ~3s before the initial fix to ~12s with the initial fix per test case. With my change, it is back to ~3s per test case. FlowDroid is pretty good for such tests, because it has more than 900 test cases in total, and FlowDroid is heavily performance-sensitive.
Hi! Sorry, I just got around to finally testing this new version, importing the new snapshot takes a bit of time. I've tried it on a few cases where the issue was arising, and it is not anymore. Thank you very much! I'm going to close the issues :)
Hello,
Regarding #1724, following a discussion with Linghui Luo and Goran Piskachev, I've been asked to create a more specific issue. It seems that the issue is due to the
WeakObjectType
only copying the name of the class and not keeping track of the entire SootClass, which then leads to Spark raising an exception issue here. The class causing the exception was properly loaded at theSIGNATURE
level in a previous phase.Citing Goran
And Linghui
Thanks!