soot-oss / SootUp

A new version of Soot with a completely overhauled architecture
https://soot-oss.github.io/SootUp/
GNU Lesser General Public License v2.1
564 stars 71 forks source link

NullPointerException Occurs During Jimple File Generation #556

Closed 75ACOL closed 1 year ago

75ACOL commented 1 year ago

Exception in thread "main" java.lang.NullPointerException at sootup.core.util.printer.JimplePrinter.printLocalsInBody(JimplePrinter.java:409) at sootup.core.util.printer.JimplePrinter.printBody(JimplePrinter.java:300) at sootup.core.util.printer.JimplePrinter.printMethods(JimplePrinter.java:245) at sootup.core.util.printer.JimplePrinter.printTo(JimplePrinter.java:209) at sootup.core.model.SootClass.print(SootClass.java:222)

JonasKlauke commented 1 year ago

@swissiety this problem will be probably solved by the type assigner pull request, or? Because currently the locals are null, or?

swissiety commented 1 year ago

Hi @75ACOL! It would be great if you can provide the input to which you are pointing in the the AnalysisInputLocation (and the signature/classType) or even better just the single compilation unit of the class which fails.

@JonasKlauke - that could help the symptoms but should not solve the underlying issue as there is something null which should not be.

75ACOL commented 1 year ago

One of the set elements in a collection is null, I can change the source code myself, but this should not be null.

swissiety commented 1 year ago

a Local in that Body.locals Set? Yes, there should be no null item - do you use the bytecode or the sourcecodefrontend?

75ACOL commented 1 year ago

bytecode

iscas-zac commented 1 year ago

Hello, it seems that we encountered the same problem, and I dig into the source code to find out some explanation.

I grabbed an apk of Fdroid to test the tool, in which a method gave the following jimple code:

l0 := @this: org.fdroid.fdroid.panic.CalculatorActivity
l1 := @parameter0: double
l4 = staticinvoke <java.lang.String: java.lang.String valueOf(double)>(l1)
l3 = l4
$stack5 = virtualinvoke l4.<java.lang.String: int length()>()
if $stack5 <= 2
l3 = l4
$stack6 = virtualinvoke l4.<java.lang.String: boolean endsWith(java.lang.String)>(".0")
if $stack6 == 0
$stack8 = virtualinvoke l4.<java.lang.String: int length()>()
$stack9 = $stack8 - 2
l3 = virtualinvoke l4.<java.lang.String: java.lang.String substring(int,int)>(0, $stack9)
$stack7 = l3
return $stack7

, and, when I wanted to print its body, reported a null pointer exception as in this issue.

I went on to single step into the sootUp source code. (Actually what I proceeded with is a demo .class file, whose only method gave

l0 := @this: A
l1 := @parameter0: double
l3 = staticinvoke <java.lang.String: java.lang.String valueOf(double)>(l1)
$stack4 = virtualinvoke l3.<java.lang.String: int length()>()
if $stack4 <= 2
$stack5 = virtualinvoke l3.<java.lang.String: boolean endsWith(java.lang.String)>(".0")
if $stack5 == 0
$stack7 = virtualinvoke l3.<java.lang.String: int length()>()
$stack8 = $stack7 - 2
$stack9 = virtualinvoke l3.<java.lang.String: java.lang.String substring(int,int)>(0, $stack8)
return $stack9
$stack6 = l3
return $stack6

, there were some differences, which confused me as the source code were the same.)

It seems that the numbering of the local variables is not sequential, and the AsmMethodSource.locals variable is a NonIndexOutofBoundsArrayList. When sootUp plugged in a l1 local variable, locals's size was actually 2, its content being null and l1, for example.

I'm not sure why the instructions follow an insequential numbering rule, but the root lies in the bytecode. For sootUp, my suggestion is to change the NonIndexOutofBoundsArrayList into a Map.

P.S. Why does the two method compiles differently? Is it simply because the compiler version or some optimization? I once thought the bug lay in the jimple parser.

iscas-zac commented 1 year ago

@JonasKlauke Hello, do you still watch this?

swissiety commented 1 year ago

@iscas-zac did you try replacing the NonIndexOutofBoundsArrayList by a Map? the resulting bytecode could be different due to different compilers/optimizations, but if the bytecode is equal the jimple code should be as well.

btw. these are just the jimple stmts (without the branching target information) but with the current information (ignoring the control flow) it seems the calculated value wich is returned could be the equal in both methods.

iscas-zac commented 1 year ago

@swissiety

these are just the jimple stmts (without the branching target information)

Yes, the branching information is not included, but I don't think it necesssary.

did you try replacing the NonIndexOutofBoundsArrayList by a Map?

No, if you mean cloning soot-up repo and changing its source.

the resulting bytecode could be different

What I really want to say, is that the bytecode can include unsequential numbering and soot-up should deal with that.

swissiety commented 1 year ago

This NPE should be fixed with the next release. please try the current dev branch. problem source: nulls inside body.locals when dword Locals were present - this affected the bytecodefronted.