X10

FinallyEliminator pass creates (unreachable) control flow path in which statically there is no return statement

Details

  • Number of attachments :
    0

Description

The attached code fails to compile with x10c++. Compiles successfully with Managed backend.

public class S {
    class Node {
        var i:Int;
        var previous:Node;
        var next:Node;
        private  atomic def casNext(oldVal:Node, newVal:Node){
            if(next == oldVal){
                newVal.previous = this;
                next = newVal;
                return true;
            }
            return false;
        }
    }
    public static def main(Array[String]) {
        
    }
}

Result of running it:

 ~/x1021/trunk/x10.dist/bin/x10c++ -report postcompile=1 -STATIC_CALLS  -v -O -post  " # # -lpmi # "    -x10rt mpi -MAIN_CLASS=S S.x10 -o stackatomic 
"/opt/ibm/java-x86_64-60/bin/java" -Xmx512m   -classpath "/home/vj/x1021/trunk/x10.dist/lib/x10c.jar:/home/vj/x1021/trunk/x10.dist/classes:/home/vj/x1021/trunk/x10.dist/lib/x10c.jar:/home/vj/x1021/trunk/x10.dist/lib/x10.jar:/home/vj/x1021/trunk/x10.dist/lib/commons-math-2.1.jar:/home/vj/x1021/trunk/x10.dist/lib/lpg.jar:/home/vj/x1021/trunk/x10.dist/lib/com.ibm.wala.cast.java_1.0.0.201012091449.jar:/home/vj/x1021/trunk/x10.dist/lib/com.ibm.wala.cast_1.0.0.201012091449.jar:/home/vj/x1021/trunk/x10.dist/lib/com.ibm.wala.core_1.1.3.201012091449.jar:/home/vj/x1021/trunk/x10.dist/lib/com.ibm.wala.shrike_1.1.3.201012091449.jar:/home/vj/x1021/trunk/x10.dist/lib/x10wala.jar:/home/vj/x1021/trunk/x10.dist/lib/org.eclipse.equinox.common_3.6.0.v20100503.jar"  polyglot.main.Main -extclass x10cuda.ExtensionInfo  -sourcepath "/home/vj/x1021/trunk/x10.dist/lib/x10.jar"  -OPTIMIZE=true  '-report' 'postcompile=1' '-STATIC_CALLS' '-post' ' # # -lpmi # ' '-MAIN_CLASS=S' 'S.x10' '-o' 'stackatomic'
Executing post-compiler mpicxx -g -I/home/vj/x1021/trunk/x10.dist/include -I/home/vj -I. -O2 -DNDEBUG -DNO_PLACE_CHECKS -finline-functions -Wno-long-long -Wno-unused-parameter -Wreturn-type -pthread -msse2 -mfpmath=sse -o /home/vj/stackatomic xxx_main_xxx.cc S__Node.cc S.cc -lpmi -L/home/vj/x1021/trunk/x10.dist/lib -lx10 -DX10_USE_BDWGC -lgc -lx10rt_mpi -ldl -lm -lpthread -Wl,--rpath -Wl,/home/vj/x1021/trunk/x10.dist/lib -Wl,-export-dynamic -lrt 
x10c++: S__Node.cc: In member function 'x10_boolean S__Node::casNext(x10aux::ref<S__Node>, x10aux::ref<S__Node>)':    
     S__Node.cc:101: warning: control reaches end of non-void function    
1 error.
[vj@athos ~]$

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Vijay Saraswat added a comment -

Also get an error with:

public class S {
    class Node {
        var i:Int;
        var previous:Node;
        var next:Node;
        private def casNext(oldVal:Node, newVal:Node){
            atomic {
              if(next == oldVal){
                newVal.previous = this;
                next = newVal;
                return true;
              }
              return false;
             }
        }
    }
    public static def main(Array[String]) {
        
    }
}
Show
Vijay Saraswat added a comment - Also get an error with: public class S { class Node { var i:Int; var previous:Node; var next:Node; private def casNext(oldVal:Node, newVal:Node){ atomic { if (next == oldVal){ newVal.previous = this ; next = newVal; return true ; } return false ; } } } public static def main(Array[ String ]) { } }
Hide
Permalink
David Grove added a comment -

The problem is that the finally-block rewriting pass leaves a (dynamically unreachable) control flow path in which control flow exits the procedure without executing a return. This code will never actually be taken, but g++ can't understand that.

The workaround is to disable -Wreturn-type from the post compilation command until finally rewriting can be fixed. Will commit this shortly.

Show
David Grove added a comment - The problem is that the finally-block rewriting pass leaves a (dynamically unreachable) control flow path in which control flow exits the procedure without executing a return. This code will never actually be taken, but g++ can't understand that. The workaround is to disable -Wreturn-type from the post compilation command until finally rewriting can be fixed. Will commit this shortly.
Hide
Permalink
Igor Peshansky added a comment -

An alternative is to add a throw of an exception that will never be executed. That way we don't have to disable a useful warning.

Show
Igor Peshansky added a comment - An alternative is to add a throw of an exception that will never be executed. That way we don't have to disable a useful warning.
Hide
Permalink
David Grove added a comment -

disabled warning in 18978. No time to do anything else before the release.

We need to get exception throwing off the return path for performance anyways, so I want to leave the warning disabled until we apply replication of finally blocks for returns. Then this becomes a moot point. Don't want to spend anytime fixing something that we actually don't want to do in the first place.

Show
David Grove added a comment - disabled warning in 18978. No time to do anything else before the release. We need to get exception throwing off the return path for performance anyways, so I want to leave the warning disabled until we apply replication of finally blocks for returns. Then this becomes a moot point. Don't want to spend anytime fixing something that we actually don't want to do in the first place.
Hide
Permalink
Bowen Alpern added a comment -

There are four ways to enter a finally block:

  1. Normal termination of the try-catch code,
  2. Throw of an uncaught throwable,
  3. Return, and
  4. Branch (Break or Continue) to a containing statement.

The existing synthesized code keeps track of ways 3 and 4 but blithely assumes that ways 1 and 2 could always happen.

The problem reported here occurs when there is no normal termination (way 1 cannot happen). This creates an unreachable code-flow that doesn't do the right thing. (The reported code also results in an unreachable code-flow to handle uncaught exceptions, way 2. But, since this path ends in a throw, it is less problematic.)

I don't believe that inlining finally blocks for ways 3 and 4 will solve the problem. Reachability analysis is needed to deduce when normal termination is impossible.

Show
Bowen Alpern added a comment - There are four ways to enter a finally block: Normal termination of the try-catch code, Throw of an uncaught throwable, Return, and Branch (Break or Continue) to a containing statement. The existing synthesized code keeps track of ways 3 and 4 but blithely assumes that ways 1 and 2 could always happen. The problem reported here occurs when there is no normal termination (way 1 cannot happen). This creates an unreachable code-flow that doesn't do the right thing. (The reported code also results in an unreachable code-flow to handle uncaught exceptions, way 2. But, since this path ends in a throw, it is less problematic.) I don't believe that inlining finally blocks for ways 3 and 4 will solve the problem. Reachability analysis is needed to deduce when normal termination is impossible.
Hide
Permalink
Igor Peshansky added a comment -

Bowen, inlining finally blocks in all cases would not alter the control flow, so this becomes a non-issue.

Show
Igor Peshansky added a comment - Bowen, inlining finally blocks in all cases would not alter the control flow, so this becomes a non-issue.
Hide
Permalink
David Grove added a comment -

defer all non-critical 2.1.2 issues to 2.2.

Show
David Grove added a comment - defer all non-critical 2.1.2 issues to 2.2.
Hide
Permalink
David Grove added a comment -

bulk defer of open issues to 2.2.2.

Show
David Grove added a comment - bulk defer of open issues to 2.2.2.
Hide
Permalink
David Grove added a comment -

bulk defer of issues to 2.2.3.

Show
David Grove added a comment - bulk defer of issues to 2.2.3.
Hide
Permalink
David Grove added a comment -

bulk defer of 2.3.0 open issues to 2.3.1.

Show
David Grove added a comment - bulk defer of 2.3.0 open issues to 2.3.1.
Hide
Permalink
David Grove added a comment -

bulk defer to 2.3.2

Show
David Grove added a comment - bulk defer to 2.3.2

People

  • Assignee:
    David Grove
    Reporter:
    Vijay Saraswat
Vote (0)
Watch (1)

Dates

  • Created:
    Updated: