Incorrect IR for JavaScript method with a finally block

[msridhar]

Code:

function test() {
   try {
     return;
   } finally {
     console.log("finally");
  }
}

When I generate IR for this method, I get:

<Code body of function L/Users/msridhar/tmp/foo.js/test>
CFG:
BB0[-1..-2]
    -> BB1
BB1[0..5]
    -> BB2
BB2[-1..-2]
Instructions:
BB0
BB1
0   v3 = new <JavaScriptLoader,LArray>@0     foo.js [35->135] (line 2) [3=[arguments]]
1   v5 = global:global $$undefined           foo.js [56->133] (line 3) [5=[$$unwind2]]
3   v7 = global:global $$undefined           foo.js [56->133] (line 3) [7=[$$unwind1]]
5   return                                   foo.js [65->72] (line 4)
BB2

The console.log() call in the finally block is missing, though it executes at runtime.

[msridhar]

/cc @MadhuNimmo

[msridhar]

Tried to debug this a bit. Consider this function:

function n() {
  try {
    return;
  } finally {
    m();
  }
}

Here the bug appears, and we do not see a call from n() to m() in a call graph. However, for this version, the call graph is correct:

function n() {
  try {
    // return;
  } finally {
    m();
  }
}

So I think it's the explicit return statement in the try block that is causing problems. The CAst for the body of n() (with the return statement) looks like:

157307626:BLOCK
  BLOCK
    TRY
      SCOPE
        BLOCK
          RETURN
      CATCH
        "$$unwind1"
        ASSIGN
          VAR
            "$$unwind0"
          VAR
            "$$unwind1"
    SCOPE
      BLOCK
        CALL
          VAR
            "m"
          "do"
          VAR
            "__WALA__int3rnal__global"
    IF_STMT
      BINARY_EXPR
        "!="
        VAR
          "$$unwind0"
        VAR
          "$$undefined"
      THROW
        VAR
          "$$unwind0"

I don't know semantics of CAst but I don't see anything indicating the fact that the call to m() in the block after the TRY block is always executed before control returns to the caller. Not immediately obvious to me what's an easy fix here.

[msridhar]

The Java frontend uses an UNWIND statement to handle try/finally:

https://github.com/wala/WALA/blob/338201dc142e081e080669c198e107e48ed75b5e/com.ibm.wala.cast.java.ecj/src/main/java/com/ibm/wala/cast/java/translator/jdt/JDTJava2CAstTranslator.java#L3568-L3579

Not sure if JavaScript could do something similar.

[msridhar]

My WIP branch attempting to fix this bug is here:

https://github.com/msridhar/WALA/tree/try-finally-bug-fix

Unfortunately I couldn't get it to fully work. I changed RhinoToAstTranslator to use UNWIND for try statements, and I'm pretty sure that yields correct CAst trees. But, it exposed a bug inside the translation from CAst to IR that I don't know how to fix. The modified complex_try_finally.js file is the simplest I could make it while still exposing the bug. The code to translate from CAst to IR is quite complex, particularly this findOrCreateCode method for generating copies of finally blocks:

https://github.com/msridhar/WALA/blob/5addb8defa8d3e8deec60259c7ab3d286982365b/com.ibm.wala.cast/src/main/java/com/ibm/wala/cast/ir/translator/AstTranslator.java#L843

There is a bug in there somewhere leading to a block with a goto instruction and multiple successors, but I haven't been able to track it down.