FYI: some cleanup fixes

[Tom Tromey <tromey at redhat dot com>]

CC'd to gcc-python-plugin.

I am checking this in on the trunk.

I implemented a GCC plugin that does some checking of cleanup use in
GDB.  This was relatively easy since I used David Malcolm's generic
Python plugin.

The plugin code is attached -- it is not perfect by any means, but it
does an ok job for most of GDB.

This patch fixes a number of problems found by this plugin.  Most of
them were cases where a function that intended to be good about cleanups
instead left some dangling along some path.

The plugin also found a couple of cases of the invalid:

    if (something)
      do_cleanups (something);

anti-idiom.

Finally, the plugin found a real bug, probably a crasher, in
mi-cmd-var.c.

Built and regtested by the buildbot.

Tom


2011-06-27  Tom Tromey  <tromey@redhat.com>

	* valops.c (find_overload_match): Call do_cleanups before early
	return.
	* top.c (execute_command): Call do_cleanups before early return.
	(command_loop): Likewise.
	* stack.c (backtrace_command): Make a null cleanup early.  Don't
	conditionally call do_cleanups.
	* python/py-value.c (TRY_CATCH): Move cleanup handling into
	TRY_CATCH.
	* python/py-breakpoint.c (gdbpy_breakpoint_has_py_cond): Rearrange
	so cleanups are always run.
	* mi/mi-cmd-var.c (mi_cmd_var_delete): Reset old_cleanups.
	* findcmd.c (parse_find_args): Call do_cleanups on early return
	path.
	* dbxread.c (elfstab_build_psymtabs): Make a null cleanup early.
	Don't conditionally call do_cleanups.
	* cli/cli-script.c (execute_user_command): Initialize 'old_chain'
	later.

diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 2d1afe5..c94f7ee 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -317,13 +317,13 @@ execute_user_command (struct cmd_list_element *c, char *args)
   static int user_call_depth = 0;
   extern int max_user_call_depth;
 
-  old_chain = setup_user_args (args);
-
   cmdlines = c->user_commands;
   if (cmdlines == 0)
     /* Null command */
     return;
 
+  old_chain = setup_user_args (args);
+
   if (++user_call_depth > max_user_call_depth)
     error (_("Max user call depth exceeded -- command aborted."));
 
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index 0c5dc02..33c776f 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -3433,7 +3433,7 @@ elfstab_build_psymtabs (struct objfile *objfile, asection *stabsect,
   bfd *sym_bfd = objfile->obfd;
   char *name = bfd_get_filename (sym_bfd);
   struct dbx_symfile_info *info;
-  struct cleanup *back_to = NULL;
+  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
 
   /* There is already a dbx_symfile_info allocated by our caller.
      It might even contain some info from the ELF symtab to help us.  */
@@ -3477,7 +3477,7 @@ elfstab_build_psymtabs (struct objfile *objfile, asection *stabsect,
   symbuf_left = bfd_section_size (objfile->obfd, stabsect);
   stabs_data = symfile_relocate_debug_section (objfile, stabsect, NULL);
   if (stabs_data)
-    back_to = make_cleanup (free_current_contents, (void *) &stabs_data);
+    make_cleanup (free_current_contents, (void *) &stabs_data);
 
   /* In an elf file, we've already installed the minimal symbols that came
      from the elf (non-stab) symbol table, so always act like an
@@ -3487,8 +3487,7 @@ elfstab_build_psymtabs (struct objfile *objfile, asection *stabsect,
      case it does, it will install them itself.  */
   dbx_symfile_read (objfile, 0);
 
-  if (back_to)
-    do_cleanups (back_to);
+  do_cleanups (back_to);
 }
 
 /* Scan and build partial symbols for a file with special sections for stabs
diff --git a/gdb/findcmd.c b/gdb/findcmd.c
index c21c028..0255613 100644
--- a/gdb/findcmd.c
+++ b/gdb/findcmd.c
@@ -132,6 +132,7 @@ parse_find_args (char *args, ULONGEST *max_countp,
       len = value_as_long (v);
       if (len == 0)
 	{
+	  do_cleanups (old_cleanups);
 	  printf_filtered (_("Empty search range.\n"));
 	  return;
 	}
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index a401846..aee6224 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -196,7 +196,7 @@ mi_cmd_var_delete (char *command, char **argv, int argc)
       children_only_p = 1;
       do_cleanups (old_cleanups);
       name = xstrdup (argv[1]);
-      make_cleanup (free_current_contents, &name);
+      old_cleanups = make_cleanup (free_current_contents, &name);
     }
 
   /* If we didn't error out, now NAME contains the name of the
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index e73dc24..294e648 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -771,10 +771,9 @@ gdbpy_breakpoint_has_py_cond (struct breakpoint_object *bp_obj)
     get_current_arch ();
   struct cleanup *cleanup = ensure_python_env (garch, current_language);
   
-  if (py_bp == NULL)
-    return 0;
+  if (py_bp != NULL)
+    has_func = PyObject_HasAttrString (py_bp, stop_func);
 
-  has_func = PyObject_HasAttrString (py_bp, stop_func);
   do_cleanups (cleanup);
 
   return has_func;
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 4381d52..fc5053a 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -553,8 +553,6 @@ static PyObject *
 valpy_str (PyObject *self)
 {
   char *s = NULL;
-  struct ui_file *stb;
-  struct cleanup *old_chain;
   PyObject *result;
   struct value_print_options opts;
   volatile struct gdb_exception except;
@@ -562,19 +560,19 @@ valpy_str (PyObject *self)
   get_user_print_options (&opts);
   opts.deref_ref = 0;
 
-  stb = mem_fileopen ();
-  old_chain = make_cleanup_ui_file_delete (stb);
-
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
+      struct ui_file *stb = mem_fileopen ();
+      struct cleanup *old_chain = make_cleanup_ui_file_delete (stb);
+
       common_val_print (((value_object *) self)->value, stb, 0,
 			&opts, python_language);
       s = ui_file_xstrdup (stb, NULL);
+
+      do_cleanups (old_chain);
     }
   GDB_PY_HANDLE_EXCEPTION (except);
 
-  do_cleanups (old_chain);
-
   result = PyUnicode_Decode (s, strlen (s), host_charset (), NULL);
   xfree (s);
 
diff --git a/gdb/stack.c b/gdb/stack.c
index 0888b69..1e44367 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1425,7 +1425,7 @@ backtrace_command_stub (void *data)
 static void
 backtrace_command (char *arg, int from_tty)
 {
-  struct cleanup *old_chain = NULL;
+  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
   int fulltrace_arg = -1, arglen = 0, argc = 0;
   struct backtrace_command_args btargs;
 
@@ -1435,7 +1435,7 @@ backtrace_command (char *arg, int from_tty)
       int i;
 
       argv = gdb_buildargv (arg);
-      old_chain = make_cleanup_freeargv (argv);
+      make_cleanup_freeargv (argv);
       argc = 0;
       for (i = 0; argv[i]; i++)
 	{
@@ -1481,8 +1481,7 @@ backtrace_command (char *arg, int from_tty)
   if (fulltrace_arg >= 0 && arglen > 0)
     xfree (arg);
 
-  if (old_chain)
-    do_cleanups (old_chain);
+  do_cleanups (old_chain);
 }
 
 static void
diff --git a/gdb/top.c b/gdb/top.c
index 58c5075..ecb91f2 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -378,7 +378,10 @@ execute_command (char *p, int from_tty)
 
   /* This can happen when command_line_input hits end of file.  */
   if (p == NULL)
-    return;
+    {
+      do_cleanups (cleanup);
+      return;
+    }
 
   target_log_command (p);
 
@@ -542,7 +545,10 @@ command_loop (void)
 				    get_prompt () : (char *) NULL,
 				    instream == stdin, "prompt");
       if (command == 0)
-	return;
+	{
+	  do_cleanups (old_chain);
+	  return;
+	}
 
       make_command_stats_cleanup (1);
 
diff --git a/gdb/valops.c b/gdb/valops.c
index f5458ef..803b73c 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2670,6 +2670,7 @@ find_overload_match (struct type **arg_types, int nargs,
       if (func_name == NULL)
         {
 	  *symp = fsym;
+	  do_cleanups (all_cleanups);
           return 0;
         }

Attachment: check_cleanups.py
Description: check_cleanups.py