From: Tom Tromey <tromey@redhat.com>
From: Tom Tromey <tromey@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 01/40] add the cleanup checker
Date: Thu, 09 May 2013 18:48:00 -0000 [thread overview]
Message-ID: <71e35171076ebf86e6c9317cf02fbbb278e9838f.1368124285.git.tromey@redhat.com> (raw)
In-Reply-To: <cover.1368124285.git.tromey@redhat.com>
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
next prev parent reply other threads:[~2013-05-09 18:48 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-09 18:47 [PATCH 00/40] add cleanup checker and fix cleanup bugs Tom Tromey, Tom Tromey
2013-05-09 18:48 ` Tom Tromey, Tom Tromey [this message]
2013-05-17 16:15 ` [PATCH 01/40] add the cleanup checker Tom Tromey
2013-05-09 18:49 ` [PATCH 02/40] some cleanup checker fixes Tom Tromey, Tom Tromey
2013-05-09 18:49 ` [PATCH 05/40] fix cleanup buglet in ada-lang.c Tom Tromey, Tom Tromey
2013-05-30 16:31 ` Tom Tromey
2013-05-09 18:49 ` [PATCH 04/40] fix cleanups in som_symtab_read Tom Tromey, Tom Tromey
2013-05-09 18:49 ` [PATCH 06/40] remove erroneous return from setup_user_args Tom Tromey, Tom Tromey
2013-05-09 18:49 ` [PATCH 03/40] fix print_command_1 Tom Tromey, Tom Tromey
2013-05-09 18:50 ` [PATCH 08/40] fix up cleanup handling in internal_vproblem Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 24/40] fix mipsread.c Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 10/40] cleanup fixes for inf-ptrace.c Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 12/40] fix cleanup handling in m32r_load Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 17/40] simplify cli-logging.c for analysis Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 18/40] fix py-breakpoint.c Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 09/40] cleanup fixes for remote-mips.c Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 20/40] fix py-prettyprint.c Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 23/40] fix one bug in symfile.c Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 13/40] fix cleanup handling in macho_symfile_read Tom Tromey, Tom Tromey
2013-06-03 13:32 ` Joel Brobecker
2013-06-03 16:09 ` Tom Tromey
2013-06-04 13:30 ` Joel Brobecker
2013-06-11 10:21 ` [patch/Darwin] Fix cleanup leak in machoread.c:macho_symfile_read Joel Brobecker
2013-06-18 15:42 ` Tom Tromey
2013-05-09 18:51 ` [PATCH 11/40] fix list_available_thread_groups Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 22/40] fix symtab.c Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 21/40] fix py-value.c Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 14/40] fix two buglets in breakpoint.c Tom Tromey, Tom Tromey
2013-05-13 2:33 ` Yao Qi
2013-05-13 14:58 ` Tom Tromey
2013-05-13 16:44 ` Tom Tromey
2013-05-14 0:37 ` Yao Qi
2013-05-17 16:19 ` Tom Tromey
2013-05-09 18:51 ` [PATCH 16/40] fix varobj.c Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 19/40] fix py-frame.c Tom Tromey, Tom Tromey
2013-05-09 18:51 ` [PATCH 15/40] make a cleanup unconditionally in tracepoint.c Tom Tromey, Tom Tromey
2013-05-30 9:23 ` Yao Qi
2013-05-30 13:23 ` Tom Tromey
2013-05-09 18:52 ` [PATCH 27/40] fix cp-namespace.c Tom Tromey, Tom Tromey
2013-05-09 18:52 ` [PATCH 28/40] use explicit returns to avoid checker confusion Tom Tromey, Tom Tromey
2013-05-24 0:51 ` asmwarrior
2013-05-24 1:30 ` Tom Tromey
2013-05-24 15:41 ` Tom Tromey
2013-05-26 11:21 ` asmwarrior
2013-05-09 18:52 ` [PATCH 29/40] fix in solib-aix.c Tom Tromey, Tom Tromey
2013-05-09 18:52 ` [PATCH 33/40] fix mi-cmd-stack.c Tom Tromey, Tom Tromey
2013-05-09 18:52 ` [PATCH 31/40] fix source.c Tom Tromey, Tom Tromey
2013-05-09 18:52 ` [PATCH 26/40] fix top.c Tom Tromey, Tom Tromey
2013-05-09 18:52 ` [PATCH 32/40] fix dbxread.c Tom Tromey, Tom Tromey
2013-05-09 18:52 ` [PATCH 35/40] fix mi-cmd-var.c Tom Tromey, Tom Tromey
2013-05-17 16:16 ` Tom Tromey
2013-05-09 18:52 ` [PATCH 36/40] introduce dangling_cleanup attribute and change source to use it Tom Tromey, Tom Tromey
2013-05-17 16:17 ` Tom Tromey
2013-05-09 18:52 ` [PATCH 30/40] fix linux-thread-db.c Tom Tromey, Tom Tromey
2013-05-09 18:52 ` [PATCH 25/40] fix one bug in stabsread.c Tom Tromey, Tom Tromey
2013-05-09 18:52 ` [PATCH 34/40] fix cli-script.c Tom Tromey, Tom Tromey
2013-05-09 18:53 ` [PATCH 39/40] fix compile_rx_or_error Tom Tromey, Tom Tromey
2013-05-13 2:53 ` Yao Qi
2013-05-13 14:59 ` Tom Tromey
2013-05-30 17:39 ` Tom Tromey
2013-05-09 18:53 ` [PATCH 38/40] some fixes to infrun.c Tom Tromey, Tom Tromey
2013-05-17 16:21 ` Tom Tromey
2013-05-09 18:53 ` [PATCH 37/40] fix breakpoint_1 Tom Tromey, Tom Tromey
2013-05-17 16:18 ` Tom Tromey
2013-05-09 18:53 ` [PATCH 40/40] fix up xml-support.c Tom Tromey, Tom Tromey
2013-05-30 17:41 ` Tom Tromey
2013-05-09 19:45 ` [PATCH 07/40] fix linespec bug noticed by the checker Tom Tromey, Tom Tromey
2013-05-09 21:21 ` [PATCH 00/40] add cleanup checker and fix cleanup bugs Phil Muldoon
2013-05-10 14:27 ` Tom Tromey
2013-05-10 8:23 ` Joel Brobecker
2013-05-10 14:34 ` Tom Tromey
2013-05-11 10:41 ` Joel Brobecker
2013-05-14 19:04 ` Tom Tromey
2013-05-30 16:17 ` Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=71e35171076ebf86e6c9317cf02fbbb278e9838f.1368124285.git.tromey@redhat.com \
--to=tromey@redhat.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox