Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@redhat.com>
From: Tom Tromey <tromey@redhat.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 36/40] introduce dangling_cleanup attribute and change source to use it
Date: Thu, 09 May 2013 18:52:00 -0000	[thread overview]
Message-ID: <09983ec5522012abad22c2484ede6d58c346cf26.1368124285.git.tromey@redhat.com> (raw)
In-Reply-To: <cover.1368124285.git.tromey@redhat.com>

Some functions leak cleanups on purpose.  However, these functions
aren't "constructors" in the cleanup-checker sense, as they do not
return the cleanup.

This patch adds an attribute that can be used to mark these functions.
This avoids a bunch of false reports from the checker.

This patch is really a huge hack.  It is basically giving up on
cleanly checking the use of cleanups.  You can see this most easily by
noticing that I elected not to have this property propagate through
the call chain.

Still, I figured I'd see what people thought of this patch.  Without
it the checker is harder to use, as you must know which functions are
ignorable.

Maybe some of these are fixable with a bit more thought.

	* ada-lang.c (old_renaming_is_invisible): Use DANGLING_CLEANUP.
	* breakpoint.c (breakpoint_re_set_default): Use DANGLING_CLEANUP.
	* c-exp.y (operator_stoken): Use DANGLING_CLEANUP.
	* coffread.c (coff_locate_sections): Use DANGLING_CLEANUP.
	* contrib/cleanup_check.py (leaks_cleanup): New global.
	(note_dangling_cleanup, leaves_dangling_cleanup): New functions.
	(CleanupChecker.compute_master): Add comment.  Check
	leaves_dangling_cleanup.
	(CleanupChecker.check_cleanups): Don't mention special constructors
	to the user.
	(register_attributes): New function.  Register it with gcc.
	* defs.h (DANGLING_CLEANUP): New define.
	* dwarf2read.c (read_cutu_die_from_dwo, handle_DW_AT_stmt_list)
	(setup_type_unit_groups, dwarf2_add_field, dwarf2_add_typedef)
	(dwarf2_add_member_fn, psymtab_include_file_name)
	(dwarf_decode_lines): Use DANGLING_CLEANUP.
	* linux-nat.c (linux_child_pid_to_exec_file): Use DANGLING_CLEANUP.
	* python/py-prettyprint.c (push_dummy_python_frame): Use
	DANGLING_CLEANUP.
	* stabsread.c (read_member_functions, read_struct_fields)
	(read_baseclasses): Use DANGLING_CLEANUP.
	* utils.h (gdb_bfd_errmsg): Use DANGLING_CLEANUP.
---
 gdb/ada-lang.c               |  1 +
 gdb/breakpoint.c             |  1 +
 gdb/c-exp.y                  |  1 +
 gdb/coffread.c               |  1 +
 gdb/contrib/cleanup_check.py | 25 ++++++++++++++++++++++---
 gdb/defs.h                   |  6 ++++++
 gdb/dwarf2read.c             |  8 ++++++++
 gdb/linux-nat.c              |  1 +
 gdb/python/py-prettyprint.c  |  1 +
 gdb/stabsread.c              |  3 +++
 gdb/utils.h                  |  3 ++-
 11 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 314f4ca..5823022 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4722,6 +4722,7 @@ is_package_name (const char *name)
 /* Return nonzero if SYM corresponds to a renaming entity that is
    not visible from FUNCTION_NAME.  */
 
+DANGLING_CLEANUP
 static int
 old_renaming_is_invisible (const struct symbol *sym, const char *function_name)
 {
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 56398a0..1cfed3b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14068,6 +14068,7 @@ addr_string_to_sals (struct breakpoint *b, char *addr_string, int *found)
    breakpoints.  Reevaluate the breakpoint and recreate its
    locations.  */
 
+DANGLING_CLEANUP
 static void
 breakpoint_re_set_default (struct breakpoint *b)
 {
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index dd032d2..a3ca6b8 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1646,6 +1646,7 @@ write_destructor_name (struct stoken token)
 
 /* Returns a stoken of the operator name given by OP (which does not
    include the string "operator").  */ 
+DANGLING_CLEANUP
 static struct stoken
 operator_stoken (const char *op)
 {
diff --git a/gdb/coffread.c b/gdb/coffread.c
index bf39085..886f0b6 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -208,6 +208,7 @@ static void coff_symtab_read (long, unsigned int, struct objfile *);
    section flags to specify what kind of debug section it is
    -kingdon).  */
 
+DANGLING_CLEANUP
 static void
 coff_locate_sections (bfd *abfd, asection *sectp, void *csip)
 {
diff --git a/gdb/contrib/cleanup_check.py b/gdb/contrib/cleanup_check.py
index 17b81f6..dca0ec8 100644
--- a/gdb/contrib/cleanup_check.py
+++ b/gdb/contrib/cleanup_check.py
@@ -58,6 +58,14 @@ special_names = set(['do_final_cleanups', 'discard_final_cleanups',
 def needs_special_treatment(decl):
     return decl.name in special_names
 
+leaks_cleanup = { }
+
+def note_dangling_cleanup(*args):
+    leaks_cleanup[str(args[0].name)] = 1
+
+def leaves_dangling_cleanup(name):
+    return name in leaks_cleanup
+
 # Sometimes we need a new placeholder object that isn't the same as
 # anything else.
 class Dummy(object):
@@ -204,6 +212,10 @@ class CleanupChecker:
             if stmt.loc:
                 curloc = stmt.loc
             if isinstance(stmt, gcc.GimpleCall) and stmt.fndecl:
+                # The below used to also have:
+                #  or leaves_dangling_cleanup(str(stmt.fndecl.name))
+                # but this yields a lot of noise when the point
+                # is really just to pretend that these functions are ok.
                 if is_constructor(stmt.fndecl):
                     log('saw constructor %s in bb=%d' % (str(stmt.fndecl), bb.index), 2)
                     self.cleanup_aware = True
@@ -223,7 +235,7 @@ class CleanupChecker:
                     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:
+                elif not self.is_special_constructor and not leaves_dangling_cleanup(str(self.fun.decl.name)):
                     if not master_cleanup.isempty():
                         if curloc not in self.bad_returns:
                             gcc.permerror(curloc, 'cleanup stack is not empty at return')
@@ -273,8 +285,8 @@ class CleanupChecker:
         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 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.
@@ -307,6 +319,13 @@ class CheckerPass(gcc.GimplePass):
         if fun.decl:
             log(fun.decl.name + ': ' + what, 2)
 
+def register_attributes():
+    gcc.register_attribute('dangling_cleanup', 0, 0, True, False, False,
+                           note_dangling_cleanup)
+    gcc.define_macro('WITH_ATTRIBUTE_DANGLING_CLEANUP')
+
+gcc.register_callback(gcc.PLUGIN_ATTRIBUTES, register_attributes)
+
 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/defs.h b/gdb/defs.h
index d8a1adb..f82fc17 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -98,6 +98,12 @@
 #include "libiberty.h"
 #include "hashtab.h"
 
+#ifdef WITH_ATTRIBUTE_DANGLING_CLEANUP
+#define DANGLING_CLEANUP __attribute__((dangling_cleanup))
+#else
+#define DANGLING_CLEANUP
+#endif
+
 /* Rather than duplicate all the logic in BFD for figuring out what
    types to use (which can be pretty complicated), symply define them
    in terms of the corresponding type from BFD.  */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 2a5acf0..5392842 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -4437,6 +4437,7 @@ init_cu_die_reader (struct die_reader_specs *reader,
    provided an abbrev table to use.
    The result is non-zero if a valid (non-dummy) DIE was found.  */
 
+DANGLING_CLEANUP
 static int
 read_cutu_die_from_dwo (struct dwarf2_per_cu_data *this_cu,
 			struct dwo_unit *dwo_unit,
@@ -8061,6 +8062,7 @@ find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu,
    COMP_DIR is the compilation directory.
    WANT_LINE_INFO is non-zero if the pc/line-number mapping is needed.  */
 
+DANGLING_CLEANUP
 static void
 handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
 			const char *comp_dir) /* ARI: editCase function */
@@ -8177,6 +8179,7 @@ read_file_scope (struct die_info *die, struct dwarf2_cu *cu)
    then restore those symtabs in the line header.
    We don't need the pc/line-number mapping for type units.  */
 
+DANGLING_CLEANUP
 static void
 setup_type_unit_groups (struct die_info *die, struct dwarf2_cu *cu)
 {
@@ -10712,6 +10715,7 @@ handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
 
 /* Add an aggregate field to the field list.  */
 
+DANGLING_CLEANUP
 static void
 dwarf2_add_field (struct field_info *fip, struct die_info *die,
 		  struct dwarf2_cu *cu)
@@ -10899,6 +10903,7 @@ dwarf2_add_field (struct field_info *fip, struct die_info *die,
 
 /* Add a typedef defined in the scope of the FIP's class.  */
 
+DANGLING_CLEANUP
 static void
 dwarf2_add_typedef (struct field_info *fip, struct die_info *die,
 		    struct dwarf2_cu *cu)
@@ -11062,6 +11067,7 @@ dwarf2_is_constructor (struct die_info *die, struct dwarf2_cu *cu)
 
 /* Add a member function to the proper fieldlist.  */
 
+DANGLING_CLEANUP
 static void
 dwarf2_add_member_fn (struct field_info *fip, struct die_info *die,
 		      struct type *type, struct dwarf2_cu *cu)
@@ -15368,6 +15374,7 @@ dwarf_decode_line_header (unsigned int offset, struct dwarf2_cu *cu)
 
    The function creates dangling cleanup registration.  */
 
+DANGLING_CLEANUP
 static const char *
 psymtab_include_file_name (const struct line_header *lh, int file_index,
 			   const struct partial_symtab *pst,
@@ -15769,6 +15776,7 @@ dwarf_decode_lines_1 (struct line_header *lh, const char *comp_dir,
    E.g. expand_line_sal requires this when finding psymtabs to expand.
    A good testcase for this is mb-inline.exp.  */
 
+DANGLING_CLEANUP
 static void
 dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
 		    struct dwarf2_cu *cu, struct partial_symtab *pst,
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 6ba71ba..6859449 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4301,6 +4301,7 @@ linux_nat_thread_name (struct thread_info *thr)
 /* Accepts an integer PID; Returns a string representing a file that
    can be opened to get the symbols for the child process.  */
 
+DANGLING_CLEANUP
 static char *
 linux_child_pid_to_exec_file (int pid)
 {
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index bdb1ac5..7e482bc 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -400,6 +400,7 @@ py_restore_tstate (void *p)
 
 /* Create a dummy PyFrameObject, needed to work around
    a Python-2.4 bug with generators.  */
+DANGLING_CLEANUP
 static PyObject *
 push_dummy_python_frame (void)
 {
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 875541c..506e3b4 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -2246,6 +2246,7 @@ stabs_method_name_from_physname (const char *physname)
 
    Returns 1 for success, 0 for failure.  */
 
+DANGLING_CLEANUP
 static int
 read_member_functions (struct field_info *fip, char **pp, struct type *type,
 		       struct objfile *objfile)
@@ -2972,6 +2973,7 @@ read_one_struct_field (struct field_info *fip, char **pp, char *p,
 
    Returns 1 for success, 0 for failure.  */
 
+DANGLING_CLEANUP
 static int
 read_struct_fields (struct field_info *fip, char **pp, struct type *type,
 		    struct objfile *objfile)
@@ -3070,6 +3072,7 @@ read_struct_fields (struct field_info *fip, char **pp, struct type *type,
 
 
 
+DANGLING_CLEANUP
 static int
 read_baseclasses (struct field_info *fip, char **pp, struct type *type,
 		  struct objfile *objfile)
diff --git a/gdb/utils.h b/gdb/utils.h
index ad5bea4..de232b9 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -52,7 +52,8 @@ extern char *safe_strerror (int);
    MATCHING, if non-NULL, is the corresponding argument to
    bfd_check_format_matches, and will be freed.  */
 
-extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching);
+extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching)
+  DANGLING_CLEANUP;
 
 /* Reset the prompt_for_continue clock.  */
 void reset_prompt_for_continue_wait_time (void);
-- 
1.8.1.4


  parent reply	other threads:[~2013-05-09 18:52 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 ` [PATCH 01/40] add the cleanup checker Tom Tromey, Tom Tromey
2013-05-17 16:15   ` Tom Tromey
2013-05-09 18:49 ` [PATCH 03/40] fix print_command_1 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 04/40] fix cleanups in som_symtab_read 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 02/40] some cleanup checker fixes 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 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: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 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 21/40] fix py-value.c 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 11/40] fix list_available_thread_groups 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 23/40] fix one bug in symfile.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 09/40] cleanup fixes for remote-mips.c 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 17/40] simplify cli-logging.c for analysis 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:52 ` [PATCH 34/40] fix cli-script.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 30/40] fix linux-thread-db.c Tom Tromey, Tom Tromey
2013-05-09 18:52 ` Tom Tromey, Tom Tromey [this message]
2013-05-17 16:17   ` [PATCH 36/40] introduce dangling_cleanup attribute and change source to use it 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 32/40] fix dbxread.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 33/40] fix mi-cmd-stack.c Tom Tromey, Tom Tromey
2013-05-09 18:52 ` [PATCH 29/40] fix in solib-aix.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 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 27/40] fix cp-namespace.c Tom Tromey, 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 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 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 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=09983ec5522012abad22c2484ede6d58c346cf26.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