wala / WALA

T.J. Watson Libraries for Analysis, with frontends for Java, Android, and JavaScript, and may common static program analyses
http://github.com/wala/WALA
Eclipse Public License 2.0
762 stars 223 forks source link

Question about control flow graph for synchronized statement #1370

Closed ljcppp closed 8 months ago

ljcppp commented 8 months ago

Here is the control flow graph generated by WALA for the following Java code that contains synchronized.

I don't understand why there are paths in the graph where the number of "monitorentry" and "monitorexit" nodes are not consistent, specifically the paths next to the red and green lines in the graph.

WALA_main

Java code

public class SynchronizedDemo2 {
    public static void main(String[] args) {
        Object object = new Object();
        synchronized (object) {
        }
        method2();
    }

    private static void method2() {
    }
}

Dot generate code using WALA cgnode is an object of CGNode that represents the main method.

try {
    String dotFile = "xxxxx";
    DotUtil.writeDotFile(cgnode.getIR().getControlFlowGraph(), makeIRDecorator(cgnode.getIR()), null, dotFile);
} catch (WalaException e) {
    e.printStackTrace();
}

According to JVM Specification, paragraph 3.14 Synchronization, JVM call monitorexit instruction twice. But that is the case for whether the method invocation completes normally (§2.6.4) or abruptly (§2.6.5). The number of "monitorentry" and "monitorexit" nodes along each path should be equal.

I would like to inquire if there is a mistake in my method of generating the CFG dot, or if there is any other issue that I am not aware of.

msridhar commented 8 months ago

I think the red edge corresponds to the case where the monitorenter does not succeed due to an NPE. I'm not sure about the green path, need to dig a bit more. Thanks for the report!

msridhar commented 8 months ago

You can see that there are two monitorexit instructions in the generated bytecode for your example:

https://gcc.godbolt.org/z/ffvjWEEMb

So I think the green path corresponds to bytecode that tries to catch any exception that gets thrown in the synchronized block and then run monitorexit before re-throwing the exception. It makes more sense in this example:

https://gcc.godbolt.org/z/9aKsx9xnK

So, in short, both the red and green cases correspond to exceptions where the corresponding monitorenter or monitorexit either did not execute or did not complete successfully. Does that make sense?

msridhar commented 8 months ago

I'm going to go ahead and close this as I'm pretty confident in the explanation above, but if you have further questions, please add another comment.