This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH 01/40] add the cleanup checker


This patch adds the cleanup checker.  This is a Python plugin for GCC
that checks some rules for cleanup handling.  In particular it tries
to notice when cleanups are left dangling at the end of a function.

It does this by applying a few simple rules.

First, it understands that a function whose return type is "struct
cleanup *" is a "cleanup constructor".  Such functions are expected to
return the first cleanup that they make.

Then, it has the notion of a "master cleanup".  The checker keeps a
stack of all cleanups made in a basic block.  The first element is
pushed on the stack is the master cleanup -- the one that must later
be passed to either do_cleanups or discard_cleanups.

It is not perfect -- some constructs confuse it.  So, part of this
series rewrites some code in gdb so that it is analyzable.  I'll note
these spots and you can decide whether or not this is a good idea.

This patch also changes gcc-with-excheck to give it options.  Now you
must use either -Xc (for the cleanup checker) or -Xx (for the
exception checker).

	* contrib/cleanup_check.py: New file.
	* contrib/gcc-with-excheck: Add option parsing.
---
 gdb/contrib/cleanup_check.py | 312 +++++++++++++++++++++++++++++++++++++++++++
 gdb/contrib/gcc-with-excheck |  32 ++++-
 2 files changed, 343 insertions(+), 1 deletion(-)
 create mode 100644 gdb/contrib/cleanup_check.py

diff --git a/gdb/contrib/cleanup_check.py b/gdb/contrib/cleanup_check.py
new file mode 100644
index 0000000..17b81f6
--- /dev/null
+++ b/gdb/contrib/cleanup_check.py
@@ -0,0 +1,312 @@
+#   Copyright 2013 Free Software Foundation, Inc.
+#
+#   This is free software: you can redistribute it and/or modify it
+#   under the terms of the GNU General Public License as published by
+#   the Free Software Foundation, either version 3 of the License, or
+#   (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful, but
+#   WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#   General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program.  If not, see
+#   <http://www.gnu.org/licenses/>.
+
+import gcc
+import gccutils
+import sys
+
+want_raii_info = False
+
+logging = False
+
+def log(msg, indent=0):
+    global logging
+    if logging:
+        sys.stderr.write('%s%s\n' % ('  ' * indent, msg))
+        sys.stderr.flush()
+
+def is_cleanup_type(return_type):
+    if not isinstance(return_type, gcc.PointerType):
+        return False
+    if not isinstance(return_type.dereference, gcc.RecordType):
+        return False
+    if str(return_type.dereference.name) == 'cleanup':
+        return True
+    return False
+
+def is_constructor(decl):
+    "Return True if the function DECL is a cleanup constructor; False otherwise"
+    return is_cleanup_type(decl.type.type) and (not decl.name or str(decl.name) != 'make_final_cleanup')
+
+destructor_names = set(['do_cleanups', 'discard_cleanups'])
+
+def is_destructor(decl):
+    return decl.name in destructor_names
+
+# This list is just much too long... we should probably have an
+# attribute instead.
+special_names = set(['do_final_cleanups', 'discard_final_cleanups',
+                     'save_cleanups', 'save_final_cleanups',
+                     'restore_cleanups', 'restore_final_cleanups',
+                     'exceptions_state_mc_init',
+                     'make_my_cleanup2', 'make_final_cleanup', 'all_cleanups',
+                     'save_my_cleanups', 'quit_target'])
+
+def needs_special_treatment(decl):
+    return decl.name in special_names
+
+# Sometimes we need a new placeholder object that isn't the same as
+# anything else.
+class Dummy(object):
+    def __init__(self, location):
+        self.location = location
+
+# A wrapper for a cleanup which has been assigned to a variable.
+# This holds the variable and the location.
+class Cleanup(object):
+    def __init__(self, var, location):
+        self.var = var
+        self.location = location
+
+# A class representing a master cleanup.  This holds a stack of
+# cleanup objects and supports a merging operation.
+class MasterCleanup(object):
+    # Create a new MasterCleanup object.  OTHER, if given, is a
+    # MasterCleanup object to copy.
+    def __init__(self, other = None):
+        # 'cleanups' is a list of cleanups.  Each element is either a
+        # Dummy, for an anonymous cleanup, or a Cleanup, for a cleanup
+        # which was assigned to a variable.
+        if other is None:
+            self.cleanups = []
+        else:
+            self.cleanups = other.cleanups[:]
+
+    def find_base_ssaname(self, arg):
+        while isinstance(arg, gcc.SsaName):
+            if not hasattr(arg, 'def_stmt'):
+                break
+            stmt = arg.def_stmt
+            if isinstance(stmt, gcc.GimpleAssign) and stmt.exprcode is gcc.VarDecl:
+                arg = stmt.rhs[0]
+            else:
+                break
+        return arg
+
+    def compare_vars(self, loc, definition, argument, bb_from):
+        log('[comparing <%s> with <%s>]' % (str(definition), str(argument)), 4)
+        if definition == argument:
+            return True
+        if not isinstance(argument, gcc.SsaName):
+            log('[not an ssaname]', 4)
+            return False
+
+        stmt = argument.def_stmt
+
+        # Maybe ARGUMENT is of the form ARGUMENT = NAME.
+        # In that case, check NAME.
+        while isinstance(stmt, gcc.GimpleAssign) and stmt.exprcode is gcc.VarDecl:
+            argument = stmt.rhs[0]
+            if definition == argument:
+                return True
+            log ('[stripped to %s]' % argument, 4)
+            if not hasattr(argument, 'def_stmt'):
+                return False
+            stmt = argument.def_stmt
+
+        if not isinstance(stmt, gcc.GimplePhi):
+            log('[definition not a Phi: %s]' % repr(stmt), 4)
+            return False
+
+        for (expr, edge) in stmt.args:
+            log('examining edge from %d (bb_from = %d)' % (edge.src.index, bb_from), 4)
+            if edge.src.index == bb_from:
+                argument = expr
+                log('replacing with %s' % str(argument), 4)
+                break
+        else:
+            # gcc.permerror(loc, 'internal error in plugin')
+            return False
+
+        definition = self.find_base_ssaname(definition)
+        argument = self.find_base_ssaname(argument)
+
+        log ('comparing (%s) %s with (%s) %s => %s'
+             % (type(definition), definition, type(argument), argument, definition == argument), 4)
+        return definition == argument
+
+    # Push a new constructor onto our stack.  LHS is the
+    # left-hand-side of the GimpleCall statement.  It may be None,
+    # meaning that this constructor's value wasn't used.
+    def push(self, location, lhs):
+        if lhs is None:
+            obj = Dummy(location)
+        else:
+            obj = Cleanup(lhs, location)
+        self.cleanups.append(obj)
+
+    # Pop constructors until we find one matching BACK_TO.
+    # This is invoked when we see a do_cleanups call.
+    def pop(self, location, back_to, bb_from):
+        log('pop:', 4)
+        for i in range(len(self.cleanups) - 1, -1, -1):
+            if isinstance(self.cleanups[i], Dummy):
+                continue
+            if self.compare_vars(location, self.cleanups[i].var,
+                                 back_to, bb_from):
+                self.cleanups = self.cleanups[0:i]
+                return
+        gcc.permerror(location, 'destructor call with unknown argument')
+
+    # Check whether ARG is the current master cleanup.  Return True if
+    # all is well.
+    def verify(self, location, arg, bb_from):
+        return (len(self.cleanups) > 0
+                and not isinstance(self.cleanups[0], Dummy)
+                and self.compare_vars(location, self.cleanups[0].var,
+                                      arg, bb_from))
+
+    # Check whether SELF is empty.
+    def isempty(self):
+        log('isempty: len = %d' % len(self.cleanups), 4)
+        return len(self.cleanups) == 0
+
+    # Emit informational warnings about the cleanup stack.
+    def inform(self):
+        for item in reversed(self.cleanups):
+            gcc.inform(item.location, 'leaked cleanup')
+
+class CleanupChecker:
+    def __init__(self, fun):
+        self.fun = fun
+        self.seen_edges = set()
+        self.bad_returns = set()
+
+    # Pick a reasonable location for the basic block BB.
+    def guess_bb_location(self, bb):
+        if isinstance(bb.gimple, list):
+            for stmt in bb.gimple:
+                if stmt.loc:
+                    return stmt.loc
+        return self.fun.end
+
+    # Compute the master cleanup list for BB.
+    # Modifies MASTER_CLEANUP in place.
+    def compute_master(self, bb, bb_from, master_cleanup):
+        log('entering BB %d' % bb.index, 1)
+        if not isinstance(bb.gimple, list):
+            return
+        curloc = self.fun.end
+        for stmt in bb.gimple:
+            if stmt.loc:
+                curloc = stmt.loc
+            if isinstance(stmt, gcc.GimpleCall) and stmt.fndecl:
+                if is_constructor(stmt.fndecl):
+                    log('saw constructor %s in bb=%d' % (str(stmt.fndecl), bb.index), 2)
+                    self.cleanup_aware = True
+                    master_cleanup.push(curloc, stmt.lhs)
+                elif is_destructor(stmt.fndecl):
+                    if str(stmt.fndecl.name) != 'do_cleanups':
+                        self.only_do_cleanups_seen = False
+                    log('saw destructor %s in bb=%d, bb_from=%d, argument=%s'
+                        % (str(stmt.fndecl.name), bb.index, bb_from, str(stmt.args[0])),
+                        2)
+                    master_cleanup.pop(curloc, stmt.args[0], bb_from)
+                elif needs_special_treatment(stmt.fndecl):
+                    pass
+                    # gcc.permerror(curloc, 'function needs special treatment')
+            elif isinstance(stmt, gcc.GimpleReturn):
+                if self.is_constructor:
+                    if not master_cleanup.verify(curloc, stmt.retval, bb_from):
+                        gcc.permerror(curloc,
+                                      'constructor does not return master cleanup')
+                elif not self.is_special_constructor:
+                    if not master_cleanup.isempty():
+                        if curloc not in self.bad_returns:
+                            gcc.permerror(curloc, 'cleanup stack is not empty at return')
+                            self.bad_returns.add(curloc)
+                            master_cleanup.inform()
+
+    # Traverse a basic block, updating the master cleanup information
+    # and propagating to other blocks.
+    def traverse_bbs(self, edge, bb, bb_from, entry_master):
+        modified = False
+        # if bb.index in self.master_cleanups:
+        #     # FIXME do some checking
+        #     return
+
+        # EDGE is None for the entry BB.
+        if edge is not None:
+            # FIXME
+            if edge in self.seen_edges:
+                return
+
+            # If merging cleanups caused a change, check to see if we
+            # have a bad loop.
+            if modified and edge in self.seen_edges:
+                gcc.permerror(self.guess_bb_location(bb),
+                              'invalid cleanup use in loop')
+            self.seen_edges.add(edge)
+
+        self.master_cleanups[bb.index] = MasterCleanup(entry_master)
+        self.compute_master(bb, bb_from, self.master_cleanups[bb.index])
+
+        # Now propagate to successor nodes.
+        for edge in bb.succs:
+            self.traverse_bbs(edge, edge.dest, bb.index,
+                              self.master_cleanups[bb.index])
+
+    def check_cleanups(self):
+        if not self.fun.cfg or not self.fun.decl:
+            return 'ignored'
+        if is_destructor(self.fun.decl):
+            return 'destructor'
+        if needs_special_treatment(self.fun.decl):
+            return 'special'
+
+        self.is_constructor = is_constructor(self.fun.decl)
+        self.is_special_constructor = not self.is_constructor and str(self.fun.decl.name).find('with_cleanup') > -1
+        # Yuck.
+        if str(self.fun.decl.name) == 'gdb_xml_create_parser_and_cleanup_1':
+            self.is_special_constructor = True
+
+        if self.is_special_constructor:
+            gcc.inform(self.fun.start, 'function %s is a special constructor' % (self.fun.decl.name))
+
+        # If we only see do_cleanups calls, and this function is not
+        # itself a constructor, then we can convert it easily to RAII.
+        self.only_do_cleanups_seen = not self.is_constructor
+        # If we ever call a constructor, then we are "cleanup-aware".
+        self.cleanup_aware = False
+
+        # This maps BB indices to the BB's master cleanup.
+        self.master_cleanups = {}
+
+        entry_bb = self.fun.cfg.entry
+        self.master_cleanups[entry_bb.index] = MasterCleanup()
+        self.traverse_bbs(None, entry_bb, -1,
+                          self.master_cleanups[entry_bb.index])
+        if want_raii_info and self.only_do_cleanups_seen and self.cleanup_aware:
+            gcc.inform(self.fun.decl.location,
+                       'function %s could be converted to RAII' % (self.fun.decl.name))
+        if self.is_constructor:
+            return 'constructor'
+        return 'OK'
+
+class CheckerPass(gcc.GimplePass):
+    def execute(self, fun):
+        if fun.decl:
+            log("Starting " + fun.decl.name)
+            # dot = gccutils.cfg_to_dot(fun.cfg, fun.decl.name)
+            # gccutils.invoke_dot(dot, name=fun.decl.name)
+        checker = CleanupChecker(fun)
+        what = checker.check_cleanups()
+        if fun.decl:
+            log(fun.decl.name + ': ' + what, 2)
+
+ps = CheckerPass(name = 'check-cleanups')
+# We need the cfg, but we want a relatively high-level Gimple.
+ps.register_after('release_ssa')
diff --git a/gdb/contrib/gcc-with-excheck b/gdb/contrib/gcc-with-excheck
index b810878..da144ac 100755
--- a/gdb/contrib/gcc-with-excheck
+++ b/gdb/contrib/gcc-with-excheck
@@ -17,12 +17,42 @@
 
 # You must set PYTHON_PLUGIN in the environment.
 # It should be the directory holding the "python.so" file.
+# Usage: gcc-with-excheck [-Xx|-Xc] [--] ARGS
+# -Xx means to invoke the exception checker.
+# -Xc means to invoke the cleanup checker.
+# -- means stop processing -X options.
+# ARGS are passed to gcc.
 
+GCC=${GCC:-gcc}
 exdir=`dirname $0`
 
+pargs=
+while true; do
+    case "$1" in
+	-Xc)
+	    pargs="$pargs -fplugin-arg-python-script=$exdir/cleanup_check.py"
+	    ;;
+	-Xx)
+	    pargs="$pargs -fplugin-arg-python-script=$exdir/excheck.py"
+	    ;;
+	-X*)
+	    echo "unrecognized argument $1" 1>&2
+	    exit 1
+	    ;;
+	--)
+	    shift
+	    break
+	    ;;
+	*)
+	    break
+	    ;;
+    esac
+    shift
+done
+
 # Recent versions of the Python plugin build two .so files in
 # different directories, so we have to set this.  This will be fixed
 # upstream at some point.
 export LD_LIBRARY_PATH=$PYTHON_PLUGIN:$PYTHON_PLUGIN/gcc-c-api
 
-gcc -fplugin=$PYTHON_PLUGIN/python.so -fplugin-arg-python-script=$exdir/excheck.py "$@"
+gcc -fplugin=$PYTHON_PLUGIN/python.so $pargs "$@"
-- 
1.8.1.4


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]