WALA icon indicating copy to clipboard operation
WALA copied to clipboard

DefaultIRFactory crashes on dex input

Open reddr opened this issue 6 years ago • 2 comments

com.ibm.wala.ssa.DefaultIRFactory crashes if the input is dex code (i.e. DexIMethod). It suffices to call sth like this

IAnalysisCacheView cache = new AnalysisCacheImpl(SSAOptions.defaultOptions());
IR ir = cache.getIR(dexIMethod, Everywhere.EVERYWHERE);

The obvious fix would be to add another instanceof check to the makeIR/makeCFG methods and enable processing through a DexIRFactory. However, this does not work since it is in the com.ibm.wala.dalvik package while the rest of the code is in the core package. Simply adding the dependency dalvik to the core package doesn't work either due to cyclic dependency.

I see different problems here:

  • Probably the right way would be to move DexIRFactory into the core package, similar to ShrikeIRFactory.

  • Why does the dalvik package depend on the cast package (spoiler: because Unary-/Binary-/BInaryLiteralInstruction import com.ibm.wala.cast.ir.ssa.CAstUnaryOp). Probably this is something that should move as well, as there are now multiple frontends using it.

One workaround to the above-mentioned problem is

IAnalysisCacheView cache = new AnalysisCacheImpl(new DexIRFactory(), SSAOptions.defaultOptions());

as long as you statically link the cast package as well. Interestingly, this also works for non-dex input as DexIRFactory seems to have a fallback mechanism as a workaround (cf. lines 47+48)

reddr avatar Jan 23 '19 07:01 reddr

Hi @reddr, not clear to me this is a bug. We've had discussions for a while about cleanly separating a "core" package from code that specifically supports Java bytecode, but no one has had the cycles to do that. So in the meantime, "core" covers both language-independent stuff and the Java bytecode frontend. I don't think moving support for other frontends into core is a good idea (/cc @juliandolby for further thoughts).

It seems like your workaround is in fact the right solution, and what we need is to improve the documentation. What do you think?

Regarding the dependence from dalvik to cast, that is gross. I am working on a PR to remove that.

msridhar avatar Jan 23 '19 18:01 msridhar

I see your point that moving the class to the core package somewhat kills the strict separation between frontends. However, it feels strange that the DefaultIRFactory can only handle a subset of cases and crashes hard on the unsupported ones. At the same time, DexIRFactory can handle all cases although its name suggests specialisation.

I'm fine with keeping the separation of code. But probably it should "crash" more gracefully, e.g. with an UnsupportedOperationException pointing to the use of DexIRFactory.

Btw. thanks for the removal of the cast dependency.

reddr avatar Jan 24 '19 07:01 reddr