Fix broken call graph for nested lambdas

[matthieu-vergne]

This pull request builds on #21.

Context

When we create a lambda in a method myMethod of a class MyClass, it is identified like this:

MyClass:lambda$myMethod$0()

When this lambda calls another method anotherMethod of a class AnotherClass, the static analysis retrieves the call relationship:

M:MyClass:lambda$myMethod$0() (M)AnotherClass:anotherMethod()

This way, we can see that the method MyClass:myMethod() calls AnotherClass:anotherMethod() through its lambda.

Problem

When having nested lambdas, the parent method is lost. For instance, if in the lambda above we don't call directly AnotherClass:anotherMethod(), but we create another lambda which calls it, then the deepest call is represented as:

M:MyClass:lambda$null$1() (M)AnotherClass:anotherMethod()

Thus, we cannot know anymore that MyClass:myMethod() calls AnotherClass:anotherMethod().

Solution

This pull request provides a Cucumber test which reproduces this behaviour (one commit) and a fix to make it pass (another commit).

Basically, this information is lost because the compiler, for each lambda, introduces an intermediary method (bootstrap) which is resolved at runtime through dynamic linking. Consequently, I have added in the code an analysis of the bootstrap methods to retrieve statically the lambdas they represent. Then, I use this information to complete the method names by replacing their null part with the parent caller. Taking the example above, it replaces this:

M:MyClass:lambda$null$1() (M)AnotherClass:anotherMethod()

by this:

M:MyClass:lambda$lambda$myMethod$0$1() (M)AnotherClass:anotherMethod()

It works recursively (tested until 3 levels of nesting).

[matthieu-vergne]

I just added 2 new commits:

• I used the lib on a JAR which showed that my code was considering too few cases, so I generalized it.
• There was a bug in BCEL lib in 6.0, fixed in 6.1, so I updated to it to the last version (6.2). I was not able to reproduce it so I added the BCEL details in the commit description.

[gousiosg]

Again, fantastic work!

[gousiosg]

I did not have the time to test it thoroughly, but your test code gives me lots of confidence.

[gousiosg]

I was perhaps too quick to merge this. It looks like it cannot run on Java's rt.jar

$ time java -Xmx4096M -jar target/javacg-0.1-SNAPSHOT-static.jar /Library/Java/JavaVirtualMachines/jdk1.8.0_25.jdk/Contents/Home/jre/lib/rt.jar 
[...]
C:apple.applescript.AppleScriptEngine java.util.Iterator
C:apple.applescript.AppleScriptEngine java.util.Map
Exception in thread "main" java.lang.NullPointerException
	at gr.gousiosg.javacg.stat.DynamicCallManager.retrieveCalls(DynamicCallManager.java:94)
	at gr.gousiosg.javacg.stat.ClassVisitor.visitJavaClass(ClassVisitor.java:65)
	at gr.gousiosg.javacg.stat.ClassVisitor.start(ClassVisitor.java:92)
	at gr.gousiosg.javacg.stat.JCallGraph.lambda$main$2(JCallGraph.java:78)
	at gr.gousiosg.javacg.stat.JCallGraph$$Lambda$2/1706377736.apply(Unknown Source)
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:267)
	at java.util.Iterator.forEachRemaining(Iterator.java:116)
	at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:512)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:502)
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.reduce(ReferencePipeline.java:484)
	at gr.gousiosg.javacg.stat.JCallGraph.main(JCallGraph.java:81)

[matthieu-vergne]

This is the problem when there is no automated tests. These issues will need to be considered one after the other. Try to add tests to reproduce them if you happen to work on them, so there is no future regression on them.

I can reproduce it with my own rt.jar (OpenJDK 8 on Ubuntu 16.04). I already had this error when it was trying to play with an interface (there is no code there, so the getCode() returns null). It seems to be the same problem here but for this method:

private static native void nativeObjectInit(Object arg0, Object arg1)

Since a native method should be related to no Java code (source) it makes sense to ignore it too. Just by replacing:

if (method.isAbstract()) {

by this it works for me:

if (method.isAbstract() || method.isNative()) {

Can I let you make the test which can reproduce it?

[matthieu-vergne]

Did not take me long to do, so here is the test you need:

#Author: matthieu.vergne@gmail.com
Feature: Native
  I want to identify all native methods within the analyzed code.

  Scenario: Retrieve native method call
    Given I have the class "NativeTest" with code:
      """
      public class NativeTest {
       public void methodA() {
        methodB();
       }
       
       public native void methodB();
      }
      """
    When I run the analyze
    Then the result should contain:
      """
      M:NativeTest:methodA() (M)NativeTest:methodB()
      """

Just add it, run it to check it fails with the same exception, do the change I suggest above, and check it works.

[matthieu-vergne]

You might as well add a test for abstract method & another for interface method. It would test the isAbstract() condition (if you remove it, both these tests should fail with the same exception).

[gousiosg]

Thanks, works!