From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17001 invoked by alias); 5 Mar 2013 21:44:44 -0000 Mailing-List: contact archer-commits-help@sourceware.org; run by ezmlm Sender: Precedence: bulk List-Post: List-Help: List-Subscribe: Received: (qmail 16949 invoked by uid 306); 5 Mar 2013 21:44:41 -0000 Date: Tue, 05 Mar 2013 21:44:00 -0000 Message-ID: <20130305214441.16934.qmail@sourceware.org> From: tromey@sourceware.org To: archer-commits@sourceware.org Subject: [SCM] tromey/cleanup-checker: fix cp-namespace.c X-Git-Refname: refs/heads/tromey/cleanup-checker X-Git-Reftype: branch X-Git-Oldrev: ffcb4744bbd7bcea6c966d2c547c0fb3b3746b4e X-Git-Newrev: a07800bcf88394a3b48aa8f6853a431220f706a7 X-SW-Source: 2013-q1/txt/msg00237.txt.bz2 List-Id: The branch, tromey/cleanup-checker has been updated via a07800bcf88394a3b48aa8f6853a431220f706a7 (commit) via b552d480e10c99e896f9fd60c87b119228a9d77f (commit) via 4d3275bdb0de18c7893f1764f829e2328d6087c9 (commit) via d8cf936981cf3e79ba6a73ef551cb22a9c0697d2 (commit) via e3d900bc5c808ae0f99e89631da7cf2debbbcc05 (commit) via fda335a762299b95b91d59313f36453f2f98f5dc (commit) via 63da9c2b9b3a60fa3c3d45fcb1869a1b409b4116 (commit) via e70a3b2e05e782b0842fde229f079d2bfa313061 (commit) via 2fe15b1b63712d575e0516bd07be0477521346b8 (commit) via 58fac0e51ab259387f99d74784b76d99c3a657a1 (commit) via 2292402df24046861246fdb2869c51d48f3536ee (commit) via f970534071b749ddba5ef069dce31351e28be1aa (commit) via e11490ffcecb0af48282da1019608928f5a5e8b3 (commit) via be67e32329bff6bc848795f18e20d53ba7f5f16f (commit) via 3f04f735a0e17c3448c798a14793ca68db7996d4 (commit) via a7f27eca4a87ffd4a3faad646635b0a346ea1833 (commit) from ffcb4744bbd7bcea6c966d2c547c0fb3b3746b4e (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email. - Log ----------------------------------------------------------------- commit a07800bcf88394a3b48aa8f6853a431220f706a7 Author: Tom Tromey Date: Tue Mar 5 14:43:27 2013 -0700 fix cp-namespace.c commit b552d480e10c99e896f9fd60c87b119228a9d77f Author: Tom Tromey Date: Tue Mar 5 14:36:21 2013 -0700 fix top.c commit 4d3275bdb0de18c7893f1764f829e2328d6087c9 Author: Tom Tromey Date: Tue Mar 5 14:33:18 2013 -0700 fix one bug in stabsread.c commit d8cf936981cf3e79ba6a73ef551cb22a9c0697d2 Author: Tom Tromey Date: Tue Mar 5 14:31:14 2013 -0700 fix mipsread.c commit e3d900bc5c808ae0f99e89631da7cf2debbbcc05 Author: Tom Tromey Date: Tue Mar 5 14:27:46 2013 -0700 fix event-top.c commit fda335a762299b95b91d59313f36453f2f98f5dc Author: Tom Tromey Date: Tue Mar 5 14:22:40 2013 -0700 fix one bug in symfile.c commit 63da9c2b9b3a60fa3c3d45fcb1869a1b409b4116 Author: Tom Tromey Date: Tue Mar 5 14:21:16 2013 -0700 fix symtab.c commit e70a3b2e05e782b0842fde229f079d2bfa313061 Author: Tom Tromey Date: Tue Mar 5 14:19:32 2013 -0700 fix a bug in breakpoint.c commit 2fe15b1b63712d575e0516bd07be0477521346b8 Author: Tom Tromey Date: Tue Mar 5 14:13:35 2013 -0700 fix py-value.c commit 58fac0e51ab259387f99d74784b76d99c3a657a1 Author: Tom Tromey Date: Tue Mar 5 14:11:25 2013 -0700 fix py-prettyprint.c commit 2292402df24046861246fdb2869c51d48f3536ee Author: Tom Tromey Date: Tue Mar 5 14:10:29 2013 -0700 fix py-frame.c commit f970534071b749ddba5ef069dce31351e28be1aa Author: Tom Tromey Date: Tue Mar 5 14:08:39 2013 -0700 fix py-breakpoint.c commit e11490ffcecb0af48282da1019608928f5a5e8b3 Author: Tom Tromey Date: Tue Mar 5 14:05:33 2013 -0700 simplify cli-logging.c for analysis commit be67e32329bff6bc848795f18e20d53ba7f5f16f Author: Tom Tromey Date: Tue Mar 5 14:04:14 2013 -0700 fix varobj.c commit 3f04f735a0e17c3448c798a14793ca68db7996d4 Author: Tom Tromey Date: Tue Mar 5 14:00:20 2013 -0700 make a cleanup unconditionally commit a7f27eca4a87ffd4a3faad646635b0a346ea1833 Author: Tom Tromey Date: Tue Mar 5 13:14:28 2013 -0700 initial rewrite ----------------------------------------------------------------------- Summary of changes: gdb/breakpoint.c | 4 +- gdb/cli/cli-logging.c | 9 ++- gdb/contrib/cleanup_check.py | 162 +++++++++++++++++++++++++++++++++--------- gdb/cp-namespace.c | 5 +- gdb/event-top.c | 1 + gdb/mipsread.c | 25 +++++-- gdb/python/py-breakpoint.c | 6 ++- gdb/python/py-frame.c | 7 ++- gdb/python/py-prettyprint.c | 2 - gdb/python/py-value.c | 10 ++- gdb/stabsread.c | 5 +- gdb/symfile.c | 10 ++- gdb/symtab.c | 2 +- gdb/top.c | 1 + gdb/tracepoint.c | 8 +- gdb/varobj.c | 4 +- 16 files changed, 197 insertions(+), 64 deletions(-) First 500 lines of diff: diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 73b3b6a..93b3c1b 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2251,6 +2251,8 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd) fpieces, nargs, argvec); } + do_cleanups (old_cleanups); + if (ex.reason < 0) { /* If we got here, it means the command could not be parsed to a valid @@ -2259,8 +2261,6 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd) return NULL; } - do_cleanups (old_cleanups); - /* We have a valid agent expression, return it. */ return aexpr; } diff --git a/gdb/cli/cli-logging.c b/gdb/cli/cli-logging.c index 9f1477d..5704179 100644 --- a/gdb/cli/cli-logging.c +++ b/gdb/cli/cli-logging.c @@ -79,7 +79,7 @@ static struct ui_file *logging_no_redirect_file; static void set_logging_redirect (char *args, int from_tty, struct cmd_list_element *c) { - struct cleanup *cleanups = NULL; + struct cleanup *cleanups; struct ui_file *output, *new_logging_no_redirect_file; struct ui_out *uiout = current_uiout; @@ -88,13 +88,15 @@ set_logging_redirect (char *args, int from_tty, struct cmd_list_element *c) || (logging_redirect == 0 && logging_no_redirect_file != NULL)) return; + cleanups = make_cleanup (null_cleanup, NULL); + if (logging_redirect != 0) { gdb_assert (logging_no_redirect_file != NULL); /* ui_out_redirect still has not been called for next gdb_stdout. */ - cleanups = make_cleanup_ui_file_delete (gdb_stdout); + make_cleanup_ui_file_delete (gdb_stdout); output = logging_no_redirect_file; new_logging_no_redirect_file = NULL; @@ -139,8 +141,7 @@ set_logging_redirect (char *args, int from_tty, struct cmd_list_element *c) || ui_out_redirect (uiout, output) < 0) warning (_("Current output protocol does not support redirection")); - if (logging_redirect != 0) - do_cleanups (cleanups); + do_cleanups (cleanups); } static void diff --git a/gdb/contrib/cleanup_check.py b/gdb/contrib/cleanup_check.py index 1fa4bfd..542e658 100644 --- a/gdb/contrib/cleanup_check.py +++ b/gdb/contrib/cleanup_check.py @@ -20,8 +20,11 @@ import sys want_raii_info = False +logging = False + def log(msg, indent=0): - if False: + global logging + if logging: sys.stderr.write('%s%s\n' % (' ' * indent, msg)) sys.stderr.flush() @@ -60,9 +63,27 @@ def needs_special_treatment(decl): class Dummy(object): pass -class CleanupChecker: - def __init__(self, fun): - self.fun = fun +# 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): + 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) @@ -71,6 +92,7 @@ class CleanupChecker: 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. @@ -98,11 +120,72 @@ class CleanupChecker: # 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: + lhs = Dummy() + self.cleanups.append((location, lhs)) + + # 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][1], Dummy): + continue + if self.compare_vars(location, self.cleanups[i][1], + 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][1], Dummy) + and self.compare_vars(location, self.cleanups[0][1], + 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[0], '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 None + return curloc = self.fun.end for stmt in bb.gimple: if stmt.loc: @@ -111,52 +194,57 @@ class CleanupChecker: if is_constructor(stmt.fndecl): log('saw constructor %s in bb=%d' % (str(stmt.fndecl), bb.index), 2) self.cleanup_aware = True - if master_cleanup is None: - master_cleanup = stmt.lhs - if master_cleanup is None: - # Maybe trigger another error later. - master_cleanup = Dummy() - log('...installed new master cleanup %s' % str(master_cleanup), 2) + 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) - # See if we're cleaning up the master cleanup. - if self.compare_vars(curloc, master_cleanup, stmt.args[0], - bb_from): - master_cleanup = None + 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 self.compare_vars(curloc, master_cleanup, stmt.retval, bb_from): + 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: - log('GOT HERE: ' + str(master_cleanup), 2) - if master_cleanup is not None: - gcc.permerror(curloc, - 'cleanup stack is not empty at return') - # sys.stderr.write('master cleanup = %s, bb_from = %d\n' % (str(master_cleanup), bb_from)) - # gccutils.write_dot_image(gccutils.cfg_to_dot(self.fun.cfg), - # self.fun.decl.name) - log('noting master cleanup %s in bb %d' % (str(master_cleanup), - bb.index), 2) - return master_cleanup - - def traverse_bbs(self, bb, bb_from, entry_master): - if bb.index in self.master_cleanups: - # FIXME do some checking - return + 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]) - self.master_cleanups[bb.index] = self.compute_master(bb, bb_from, entry_master) # Now propagate to successor nodes. for edge in bb.succs: - self.traverse_bbs(edge.dest, bb.index, self.master_cleanups[bb.index]) - return + 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: @@ -184,7 +272,10 @@ class CleanupChecker: # This maps BB indices to the BB's master cleanup. self.master_cleanups = {} - self.traverse_bbs(self.fun.cfg.entry, -1, None) + 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)) @@ -204,4 +295,5 @@ class CheckerPass(gcc.GimplePass): 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/cp-namespace.c b/gdb/cp-namespace.c index 279021e..19abb1e 100644 --- a/gdb/cp-namespace.c +++ b/gdb/cp-namespace.c @@ -499,7 +499,10 @@ cp_lookup_symbol_imports_or_template (const char *scope, TYPE_N_TEMPLATE_ARGUMENTS (context), TYPE_TEMPLATE_ARGUMENTS (context)); if (result != NULL) - return result; + { + do_cleanups (cleanups); + return result; + } } do_cleanups (cleanups); diff --git a/gdb/event-top.c b/gdb/event-top.c index 6f3dfab..6b99bf2 100644 --- a/gdb/event-top.c +++ b/gdb/event-top.c @@ -268,6 +268,7 @@ display_gdb_prompt (char *new_prompt) rl_callback_handler_remove(), does the job. */ rl_callback_handler_remove (); + do_cleanups (old_chain); return; } else diff --git a/gdb/mipsread.c b/gdb/mipsread.c index e9f0402..b425780b 100644 --- a/gdb/mipsread.c +++ b/gdb/mipsread.c @@ -227,16 +227,28 @@ read_alphacoff_dynamic_symtab (struct section_offsets *section_offsets, if (!bfd_get_section_contents (abfd, si.sym_sect, sym_secptr, (file_ptr) 0, sym_secsize)) - return; + { + do_cleanups (cleanups); + return; + } if (!bfd_get_section_contents (abfd, si.str_sect, str_secptr, (file_ptr) 0, str_secsize)) - return; + { + do_cleanups (cleanups); + return; + } if (!bfd_get_section_contents (abfd, si.dyninfo_sect, dyninfo_secptr, (file_ptr) 0, dyninfo_secsize)) - return; + { + do_cleanups (cleanups); + return; + } if (!bfd_get_section_contents (abfd, si.got_sect, got_secptr, (file_ptr) 0, got_secsize)) - return; + { + do_cleanups (cleanups); + return; + } /* Find the number of local GOT entries and the index for the first dynamic symbol in the GOT. */ @@ -264,7 +276,10 @@ read_alphacoff_dynamic_symtab (struct section_offsets *section_offsets, } } if (dt_mips_local_gotno < 0 || dt_mips_gotsym < 0) - return; + { + do_cleanups (cleanups); + return; + } /* Scan all dynamic symbols and enter them into the minimal symbol table if appropriate. */ diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c index 5e5f9b3..d099892 100644 --- a/gdb/python/py-breakpoint.c +++ b/gdb/python/py-breakpoint.c @@ -489,7 +489,11 @@ bppy_get_commands (PyObject *self, void *closure) print_command_lines (current_uiout, breakpoint_commands (bp), 0); } ui_out_redirect (current_uiout, NULL); - GDB_PY_HANDLE_EXCEPTION (except); + if (except.reason < 0) + { + do_cleanups (chain); + GDB_PY_HANDLE_EXCEPTION (except); + } cmdstr = ui_file_xstrdup (string_file, &length); make_cleanup (xfree, cmdstr); diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c index e2eb9c5..33d0bd0 100644 --- a/gdb/python/py-frame.c +++ b/gdb/python/py-frame.c @@ -456,6 +456,7 @@ frapy_read_var (PyObject *self, PyObject *args) { PyErr_SetString (PyExc_RuntimeError, _("Second argument must be block.")); + do_cleanups (cleanup); return NULL; } } @@ -468,7 +469,11 @@ frapy_read_var (PyObject *self, PyObject *args) block = get_frame_block (frame, NULL); var = lookup_symbol (var_name, block, VAR_DOMAIN, NULL); } - GDB_PY_HANDLE_EXCEPTION (except); + if (except.reason < 0) + { + do_cleanups (cleanup); + GDB_PY_HANDLE_EXCEPTION (except); + } if (!var) { diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c index dbf6c22..9b7a0dd 100644 --- a/gdb/python/py-prettyprint.c +++ b/gdb/python/py-prettyprint.c @@ -629,8 +629,6 @@ print_children (PyObject *printer, const char *hint, local_opts.addressprint = 0; val_print_string (type, encoding, addr, (int) length, stream, &local_opts); - - do_cleanups (inner_cleanup); } else if (gdbpy_is_string (py_v)) { diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c index 11cc038..74087cd 100644 --- a/gdb/python/py-value.c +++ b/gdb/python/py-value.c @@ -775,11 +775,17 @@ valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other) a gdb.Value object and need to convert it from python as well. */ arg1 = convert_value_from_python (self); if (arg1 == NULL) - break; + { + do_cleanups (cleanup); + break; + } arg2 = convert_value_from_python (other); if (arg2 == NULL) - break; + { + do_cleanups (cleanup); + break; + } switch (opcode) { diff --git a/gdb/stabsread.c b/gdb/stabsread.c index a38ead1..732df5e 100644 --- a/gdb/stabsread.c +++ b/gdb/stabsread.c @@ -3534,7 +3534,10 @@ read_struct_type (char **pp, struct type *type, enum type_code type_code, TYPE_LENGTH (type) = read_huge_number (pp, 0, &nbits, 0); if (nbits != 0) - return error_type (pp, objfile); + { + do_cleanups (back_to); + return error_type (pp, objfile); + } set_length_in_type_chain (type); } diff --git a/gdb/symfile.c b/gdb/symfile.c index 59b7b43..eab2603 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -1528,7 +1528,10 @@ find_separate_debug_file (const char *dir, strcat (debugfile, debuglink); if (separate_debug_file_exists (debugfile, crc32, objfile)) - return debugfile; + { + do_cleanups (back_to); + return debugfile; + } /* If the file is in the sysroot, try using its base path in the global debugfile directory. */ @@ -1543,7 +1546,10 @@ find_separate_debug_file (const char *dir, strcat (debugfile, debuglink); if (separate_debug_file_exists (debugfile, crc32, objfile)) - return debugfile; + { + do_cleanups (back_to); + return debugfile; + } } } diff --git a/gdb/symtab.c b/gdb/symtab.c index 57441c1..1331c49 100644 --- a/gdb/symtab.c hooks/post-receive -- Repository for Project Archer.