ThrowableSet.catchableAs(...) can be incorrect in case option no_bodies_for_excluded == true

[jpstotz]

If you call ThrowableSet.catchableAs(exceptionType) using an exception that is not a basic class and not an application class the result will be incorrect.

Background: In case the Soot option no_bodies_for_excluded == true and include_all == false Soot will load most of the library classes (from rt.jar or android.jar) and mark them as phantom class if they come from a package that is excluded by default (see Scene.determineExcludedPackages()).

The method ThrowableSet.catchableAs(..) checks if a class is phantom and if it is phantom it does not perform the type check which ends up in incorrect results. I assume the person who wrote the catchableAs method assumed that phantom classes do not have a valid type hierarchy and therefore can't be checked.
However the "phantom library classes" do have a correct type hierarchy!

BTW: Alternatively there is the solution to say that the option no_bodies_for_excluded does not work correctly as it's name would suggest (or is there a documentation of this flag?) because it marks the whole class phantom instead of only marking the methods as phantom (which is what I would have expected by it's name). Also the default not-modifiable exclude list in Scene.java is IMHO not a very clean solution.

[mbenz89]

I agree that no_bodies_for_excludesis a misleading name regarding its effects. Also, having the default excludes non-modifiable in the Scene isn't the nicest solution as well. On the other hand, using -include-all allows for a fully customized exclusion list, while still being very counter-intuitive namewise. I suggest that we rename it to no_default_exclusions or something alike.

Classes can be phantom since you might not have a class source for them, eventually rendering all of its methods as phantom as well. The case where you want to exclude only a few methods of a class but not all is not modeled within Soot.

We might consider renaming the no_bodies_for_excluded option as well, since, as you mentioned, the name does not quite resemble its effect.

I am not quite sure how phantom classes are able to have a correct type hierarchy... How does Soot build the type hierarchy for classes with no class source?

[jpstotz]

I am not quite sure how phantom classes are able to have a correct type hierarchy... How does Soot build the type hierarchy for classes with no class source?

For a real phantom class Soot can not build a class hierarchy (in such a case is superClass of the phantom class null?). However I assume the developer who have implemented this used it as a hack:

The following is partially an assumption based on the outcome I have seen and my general reverse engineering skill how developers usually think:

Situation: no_bodies_for_excluded == true and include_all == false

A library class that is excluded by the default exclude list is loaded and resolved for the level HIERARCHY. To make sure Soot does not try to resolve the class for a higher level like SIGNATURES or BODIES the class is marked phantom. Therefore we end up with classes that have hierarchy info but are marked as phantom.

[mbenz89]

I see. I misread that this is about library classes...

Could we just extend the check in ThrowableSet.catchableAs(exceptionType) to also check if the class has level HIERARCHY or better?

[jpstotz]

That would be an option if a real phantom class would only have a resolved level of DANGLING.

I am not that deep into the Soot phantom class generation to know if this is true or not.

Regarding the no_bodies_for_excluded I think a good description would be sufficient.

BTW: I noticed that the class soot.options.Options does not contain any JavaDocs. I don't know how others are using Soot, I prefer not use soot.Main.main() and therefore I set my Options directly via Options.v().set_no_bodies_for_excluded(true). Hence JavaDoc on the option setter/getter entries would be very welcome.

[jpstotz]

Could we just extend the check in ThrowableSet.catchableAs(exceptionType) to also check if the class has level HIERARCHY or better?

Unfortunately this bug can't be solved that simple. I just changed the implementation to check for catcher.getSootClass().resolvingLevel() < SootClass.HIERARCHY instead of catcher.getSootClass().isPhantom().
In the end my tests worked, however the unit tests ThrowableSettests.test_14_WhichCatchablePhantom0 and ThrowableSettests.test_14_WhichCatchablePhantom1 now fail.

Both tests use "phantom exceptions" which are created this way : Scene.v().forceResolve("de.ecspride.NonExistingExceptionToTestPhantoms1",SootClass.BODIES);

This creates a phantom class that has resolving level BODIES but is a real phantom.

Therefore the only chance I currently see is to use the superClass attribute of the class. Except for java.lang.Object every class have to have a super class therefore this attribute should never be null for a class that has a valid hierarchy. Or did I miss something?

[jpstotz]

OK, I implemented the check that bases on superClass instead of isPhantomClass. I also changed the other methods of ThrowableSet which were using the same approach.

Any comments of pull request #1073 are welcome.