Fix source-code locations of return statements

In a function with multiple return statements, they are all combined into
a single gcc.GimpleReturn, with a None for its .loc, which led to the
source-location at a return being reported as fun.end, which is confusing,
see:

  https://fedorahosted.org/gcc-python-plugin/ticket/58

Add a "lastgccloc" attribute to absinterp.State, updating this whenever
building a new state with a new stmt that has a non-None location.  States
with stmt.loc == None inherit the lastgccloc of the previous state, thus
propagating sane locations, and avoiding the need to use fun.end (I hope):
the return statements are reported as the location of the assignment to the
retval.

Manual inspection of the existing testcase results shows that the reported
locations are now sane.  Added a testcase to specifically test for this
behavior:

  tests/cpychecker/refcounts/multiple-returns

Fixes https://fedorahosted.org/gcc-python-plugin/ticket/58

Author: davidmalcolm

libcpychecker/absinterp.py

@@ -1122,15 +1122,6 @@ def describe_stmt(stmt):
     else:
         return str(stmt.loc)
 
-def next_stmt_node(stmtnode):
-    if len(stmtnode.succs) != 1:
-        raise ValueError('len(stmtnode.succs) == %i at %s'
-                         % (len(stmtnode.succs), stmtnode))
-    edge = list(stmtnode.succs)[0]
-    assert edge.srcnode == stmtnode
-    assert edge.dstnode != stmtnode
-    return edge.dstnode
-
 class Region(object):
     __slots__ = ('name', 'parent', 'children', 'fields', )
 
@@ -1336,14 +1327,16 @@ class State(object):
     # We can't use the __slots__ optimization here, as we're adding additional
     # per-facet attributes
 
-    def __init__(self, stmtgraph, stmtnode, facets, region_for_var=None, value_for_region=None,
+    def __init__(self, stmtgraph, stmtnode, lastgccloc,
+                 facets, region_for_var=None, value_for_region=None,
                  return_rvalue=None, has_returned=False, not_returning=False):
         check_isinstance(stmtgraph, StmtGraph)
         check_isinstance(stmtnode, StmtNode)
         check_isinstance(facets, dict)
         self.stmtgraph = stmtgraph
         self.fun = stmtgraph.fun
         self.stmtnode = stmtnode
+        self.lastgccloc = lastgccloc
         self.facets = facets
 
         # Mapping from VarDecl.name to Region:
@@ -1418,6 +1411,7 @@ def log(self, logger):
     def copy(self):
         s_new = State(self.stmtgraph,
                       self.stmtnode,
+                      self.lastgccloc,
                       self.facets,
                       self.region_for_var.copy(),
                       self.value_for_region.copy(),
@@ -1928,18 +1922,29 @@ def mktrans_assignment(self, lhs, rhs, desc):
         log('mktrans_assignment(%r, %r, %r)', lhs, rhs, desc)
         if desc:
             check_isinstance(desc, str)
-        new = self.copy()
-        new.stmtnode = next_stmt_node(self.stmtnode)
+        new = self.use_next_stmt_node()
         if lhs:
             new.assign(lhs, rhs, self.stmtnode.get_gcc_loc())
         return Transition(self, new, desc)
 
     def update_stmt_node(self, new_stmt_node):
         new = self.copy()
         new.stmtnode = new_stmt_node
+        if new.stmtnode.stmt and new.stmtnode.stmt.loc:
+            new.lastgccloc = new.stmtnode.stmt.loc
+        else:
+            new.lastgccloc = self.lastgccloc
         return new
 
     def use_next_stmt_node(self):
+        def next_stmt_node(stmtnode):
+            if len(stmtnode.succs) != 1:
+                raise ValueError('len(stmtnode.succs) == %i at %s'
+                                 % (len(stmtnode.succs), stmtnode))
+            edge = list(stmtnode.succs)[0]
+            assert edge.srcnode == stmtnode
+            assert edge.dstnode != stmtnode
+            return edge.dstnode
         new_stmt_node = next_stmt_node(self.stmtnode)
         return self.update_stmt_node(new_stmt_node)
 
@@ -1959,7 +1964,8 @@ def get_gcc_loc(self, fun):
             # grrr... not all statements have a non-NULL location
             gccloc = self.stmtnode.get_stmt().loc
             if gccloc is None:
-                gccloc = fun.end
+                assert self.lastgccloc
+                return self.lastgccloc
             return gccloc
         else:
             return fun.end
@@ -2081,8 +2087,7 @@ def mkstate_nop(self, stmt):
         Clone this state (at a function call), updating the location, for
         functions with "void" return type
         """
-        newstate = self.copy()
-        newstate.stmtnode = next_stmt_node(self.stmtnode)
+        newstate = self.use_next_stmt_node()
         return newstate
 
     def mkstate_return_of(self, stmt, v_return):
@@ -2091,8 +2096,7 @@ def mkstate_return_of(self, stmt, v_return):
         setting the result of the call to the given AbstractValue
         """
         check_isinstance(v_return, AbstractValue)
-        newstate = self.copy()
-        newstate.stmtnode = next_stmt_node(self.stmtnode)
+        newstate = self.use_next_stmt_node()
         if stmt.lhs:
             newstate.assign(stmt.lhs,
                             v_return,
@@ -2105,8 +2109,7 @@ def mkstate_concrete_return_of(self, stmt, value):
         setting the result of the call to the given concrete value
         """
         check_isinstance(value, numeric_types)
-        newstate = self.copy()
-        newstate.stmtnode = next_stmt_node(self.stmtnode)
+        newstate = self.use_next_stmt_node()
         if stmt.lhs:
             newstate.assign(stmt.lhs,
                             ConcreteValue(stmt.lhs.type, stmt.loc, value),
@@ -2119,8 +2122,7 @@ def mktrans_nop(self, stmt, fnname):
         effect within our simulation (beyond advancing to the next location).
         [We might subsequently modify the destination state, though]
         """
-        newstate = self.copy()
-        newstate.stmtnode = next_stmt_node(self.stmtnode)
+        newstate = self.use_next_stmt_node()
         return Transition(self, newstate, 'calling %s()' % fnname)
 
 
@@ -2641,8 +2643,7 @@ def _get_transitions_for_GimpleAsm(self, stmt):
 
         if stmt.string == '':
             # Empty fragment of inline assembler:
-            s_next = self.copy()
-            s_next.stmtnode = next_stmt_node(self.stmtnode)
+            s_next = self.use_next_stmt_node()
             return [Transition(self, s_next, None)]
 
         raise NotImplementedError('Unable to handle inline assembler: %s'
@@ -2952,6 +2953,7 @@ def iter_traces(stmtgraph, facets, prefix=None, limits=None, depth=0):
         prefix = Trace()
         curstate = State(stmtgraph,
                          stmtgraph.get_entry_nodes()[0],
+                         None,
                          facets,
                          None, None, None)
         #Resources())

libcpychecker/refcounts.py

@@ -149,8 +149,7 @@ def never_returns(self):
     def add_outcome(self, desc, s_new=None):
         assert not self._never_returns
         if s_new is None:
-            s_new = self.s_src.copy()
-            s_new.stmtnode = next_stmt_node(self.s_src.stmtnode)
+            s_new = self.s_src.use_next_stmt_node()
         oc_new = Outcome(self, desc, s_new)
         self.outcomes.append(oc_new)
         return oc_new
@@ -448,8 +447,7 @@ def get_transitions_for_function_call(self, state, stmt):
         result = state.mktrans_assignment(stmt.lhs,
                                        UnknownValue.make(returntype, stmt.loc),
                                           desc)
-        s_new = state.copy()
-        s_new.stmtnode = next_stmt_node(state.stmtnode)
+        s_new = state.use_next_stmt_node()
 
         if region is not None:
             # Mark the region as deallocated:
@@ -824,8 +822,7 @@ def mkstate_new_ref(self, stmt, name, typeobjregion=None):
 
         Returns a pair: (newstate, RegionOnHeap for the new object)
         """
-        newstate = self.state.copy()
-        newstate.stmtnode = next_stmt_node(self.state.stmtnode)
+        newstate = self.state.use_next_stmt_node()
 
         r_nonnull = newstate.cpython.make_sane_object(stmt, name,
                                               RefcountValue.new_ref(stmt.loc, None),
@@ -842,8 +839,7 @@ def mkstate_new_ref(self, stmt, name, typeobjregion=None):
     def mkstate_borrowed_ref(self, stmt, fnmeta, r_typeobj=None):
         """Make a new State, giving a borrowed ref to some object"""
         check_isinstance(fnmeta, FnMeta)
-        newstate = self.state.copy()
-        newstate.stmtnode = next_stmt_node(self.state.stmtnode)
+        newstate = self.state.use_next_stmt_node()
 
         r_nonnull = newstate.cpython.make_sane_object(stmt,
                                               'borrowed reference returned by %s()' % fnmeta.name,
@@ -2484,8 +2480,7 @@ def impl_PyMem_Free(self, stmt, v_ptr):
 
         # FIXME: it's unsafe to call repeatedly, or on the wrong memory region
 
-        s_new = self.state.copy()
-        s_new.stmtnode = next_stmt_node(self.state.stmtnode)
+        s_new = self.state.use_next_stmt_node()
         desc = None
 
         # It's safe to call on NULL

tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/stderr.txt

@@ -1,7 +1,7 @@
 In function 'test':
-tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:58:nn: warning: memory leak: ob_refcnt of '*value' is 1 too high [enabled by default]
+tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:52:nn: warning: memory leak: ob_refcnt of '*value' is 1 too high [enabled by default]
 tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:37:nn: note: '*value' was allocated at:     value = PyLong_FromLong(1000);
-tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:58:nn: note: was expecting final owned ob_refcnt of '*value' to be 0 since nothing references it but final ob_refcnt is refs: 1 owned, 1 borrowed
+tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:52:nn: note: was expecting final owned ob_refcnt of '*value' to be 0 since nothing references it but final ob_refcnt is refs: 1 owned, 1 borrowed
 tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:32:nn: note: when PyDict_New() succeeds at:     dict = PyDict_New();
 tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:33:nn: note: taking False path at:     if (!dict) {
 tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:37:nn: note: reaching:     value = PyLong_FromLong(1000);
@@ -13,5 +13,5 @@ tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:42:nn: note: w
 tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:42:nn: note: ob_refcnt is now refs: 1 owned, 1 borrowed
 tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:42:nn: note: taking False path at:     if (-1 == PyDict_SetItemString(dict, "key", value)) {
 tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:52:nn: note: reaching:     return dict;
-tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:58:nn: note: returning
+tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:52:nn: note: returning
 tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c:28:nn: note: graphical error report for function 'test' written out to 'tests/cpychecker/refcounts/PyDict_SetItemString/incorrect/input.c.test-refcount-errors.html'

tests/cpychecker/refcounts/PyList_Append/incorrect-loop/stderr.txt

@@ -1,7 +1,7 @@
 In function 'test':
-tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:60:nn: warning: memory leak: ob_refcnt of '*item' is 1 too high [enabled by default]
+tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:59:nn: warning: memory leak: ob_refcnt of '*item' is 1 too high [enabled by default]
 tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:41:nn: note: '*item' was allocated at:         item = item_ctor();
-tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:60:nn: note: was expecting final owned ob_refcnt of '*item' to be 1 due to object being referenced by: PyListObject.ob_item[0] but final ob_refcnt is refs: 2 owned
+tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:59:nn: note: was expecting final owned ob_refcnt of '*item' to be 1 due to object being referenced by: PyListObject.ob_item[0] but final ob_refcnt is refs: 2 owned
 tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:34:nn: note: when PyList_New() succeeds at:     list = PyList_New(0);
 tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:35:nn: note: taking False path at:     if (!list) {
 tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:38:nn: note: reaching:     for (i = 0; i < n; i++) {
@@ -20,5 +20,5 @@ tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:38:nn: note: rea
 tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:38:nn: note: when considering n == (int)1 from tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:29 at:     for (i = 0; i < n; i++) {
 tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:38:nn: note: taking False path at:     for (i = 0; i < n; i++) {
 tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:59:nn: note: reaching:     return list;
-tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:60:nn: note: returning
+tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:59:nn: note: returning
 tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c:30:nn: note: graphical error report for function 'test' written out to 'tests/cpychecker/refcounts/PyList_Append/incorrect-loop/input.c.test-refcount-errors.html'

tests/cpychecker/refcounts/PyList_Append/incorrect/stderr.txt

@@ -1,7 +1,7 @@
 In function 'test':
-tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:54:nn: warning: memory leak: ob_refcnt of '*item' is 1 too high [enabled by default]
+tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:53:nn: warning: memory leak: ob_refcnt of '*item' is 1 too high [enabled by default]
 tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:36:nn: note: '*item' was allocated at:     item = PyLong_FromLong(42);
-tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:54:nn: note: was expecting final owned ob_refcnt of '*item' to be 1 due to object being referenced by: PyListObject.ob_item[0] but final ob_refcnt is refs: 2 owned
+tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:53:nn: note: was expecting final owned ob_refcnt of '*item' to be 1 due to object being referenced by: PyListObject.ob_item[0] but final ob_refcnt is refs: 2 owned
 tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:32:nn: note: when PyList_New() succeeds at:     list = PyList_New(0);
 tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:33:nn: note: taking False path at:     if (!list) {
 tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:36:nn: note: reaching:     item = PyLong_FromLong(42);
@@ -14,5 +14,5 @@ tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:42:nn: note: ob_refcn
 tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:42:nn: note: '*item' is now referenced by 1 non-stack value(s): PyListObject.ob_item[0]
 tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:42:nn: note: taking False path at:     if (-1 == PyList_Append(list, item)) {
 tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:53:nn: note: reaching:     return list;
-tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:54:nn: note: returning
+tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:53:nn: note: returning
 tests/cpychecker/refcounts/PyList_Append/incorrect/input.c:28:nn: note: graphical error report for function 'test' written out to 'tests/cpychecker/refcounts/PyList_Append/incorrect/input.c.test-refcount-errors.html'

tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/stderr.txt

@@ -1,7 +1,7 @@
 In function 'handle_SET_ITEM_macro':
-tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:76:nn: warning: memory leak: ob_refcnt of PyLongObject is 1 too high [enabled by default]
+cc1: warning: memory leak: ob_refcnt of PyLongObject is 1 too high [enabled by default]
 tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:50:nn: note: PyLongObject was allocated at:     items[2] = PyLong_FromLong(3000);
-tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:76:nn: note: was expecting final owned ob_refcnt of PyLongObject to be 1 due to object being referenced by: ob_item array for PyListObject[2] but final ob_refcnt is refs: 2 owned
+cc1: note: was expecting final owned ob_refcnt of PyLongObject to be 1 due to object being referenced by: ob_item array for PyListObject[2] but final ob_refcnt is refs: 2 owned
 tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:33:nn: note: when PyList_New() succeeds at:     list = PyList_New(3);
 tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:34:nn: note: taking False path at:     if (!list) {
 tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:39:nn: note: reaching:     items[0] = PyLong_FromLong(1000);
@@ -17,5 +17,5 @@ tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:51:n
 tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:64:nn: note: reaching:     PyList_SET_ITEM(list, 0, items[0]);
 tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:66:nn: note: PyLongObject is now referenced by 1 non-stack value(s): ob_item array for PyListObject[2]
 tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:71:nn: note: ob_refcnt is now refs: 2 owned
-tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:76:nn: note: returning
+cc1: note: returning
 tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c:24:nn: note: graphical error report for function 'handle_SET_ITEM_macro' written out to 'tests/cpychecker/refcounts/PyList_SET_ITEM_macro/incorrect_multiple/input.c.handle_SET_ITEM_macro-refcount-errors.html'

tests/cpychecker/refcounts/PySequence_SetItem/incorrect/stderr.txt

@@ -1,7 +1,7 @@
 In function 'handle_SetItem':
-tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:51:nn: warning: memory leak: ob_refcnt of '*item' is 1 too high [enabled by default]
+tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:50:nn: warning: memory leak: ob_refcnt of '*item' is 1 too high [enabled by default]
 tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:34:nn: note: '*item' was allocated at:     item = PyLong_FromLong(42);
-tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:51:nn: note: was expecting final owned ob_refcnt of '*item' to be 0 since nothing references it but final ob_refcnt is refs: 1 owned
+tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:50:nn: note: was expecting final owned ob_refcnt of '*item' to be 0 since nothing references it but final ob_refcnt is refs: 1 owned
 tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:29:nn: note: when PyList_New() succeeds at:     list = PyList_New(1);
 tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:30:nn: note: taking False path at:     if (!list) {
 tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:34:nn: note: reaching:     item = PyLong_FromLong(42);
@@ -12,5 +12,5 @@ tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:41:nn: note: rea
 tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:41:nn: note: PySequence_SetItem() succeeds at:     rv = PySequence_SetItem(list, 0, item);
 tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:42:nn: note: taking False path at:     if (rv != 0) {
 tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:50:nn: note: reaching:     return list;
-tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:51:nn: note: returning
+tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:50:nn: note: returning
 tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c:24:nn: note: graphical error report for function 'handle_SetItem' written out to 'tests/cpychecker/refcounts/PySequence_SetItem/incorrect/input.c.handle_SetItem-refcount-errors.html'