Re: [PATCH 36/40] introduce dangling_cleanup attribute and change source to use it

Tom Tromey
2013-05-17T16:17:57Z

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

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

I added a comment explaining WITH_ATTRIBUTE_DANGLING_CLEANUP to this
patch. Also, the checker found one more dangling cleanup that was not
readily fixable, so I added an attribute there. (This is the
linux-tdep.c change.)

Tom

* 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.
* linux-tdep.c (linux_make_corefile_notes): 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.

more dangling cleanups
---
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 | 10 ++++++++++
gdb/dwarf2read.c | 8 ++++++++
gdb/linux-nat.c | 1 +
gdb/linux-tdep.c | 1 +
gdb/python/py-prettyprint.c | 1 +
gdb/stabsread.c | 3 +++
gdb/utils.h | 3 ++-
12 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index fa6db0f..19bceb6 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 2f4d4fc..4c34a5c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14066,6 +14066,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 b182d8d..0dd149f 100644
--- a/gdb/contrib/cleanup_check.py
+++ b/gdb/contrib/cleanup_check.py
@@ -59,6 +59,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):
@@ -218,6 +226,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
@@ -240,7 +252,7 @@ class CleanupChecker:
if not master_cleanup.verify(curloc, stmt.retval):
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')
@@ -299,8 +311,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.
@@ -330,6 +342,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('cfg')
diff --git a/gdb/defs.h b/gdb/defs.h
index d8a1adb..fbf6edc 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -98,6 +98,16 @@
#include "libiberty.h"
#include "hashtab.h"

+/* The cleanup checker (see contrib/cleanup_check.py) defines
+ WITH_ATTRIBUTE_DANGLING_CLEANUP. If this is defined then we can
+ use the 'dangling_cleanup' attribute to mark functions which are
+ known to leak a cleanup. */
+#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 a17cd9d..8e9b5a5 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)
{
@@ -10713,6 +10716,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)
@@ -10900,6 +10904,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)
@@ -11063,6 +11068,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)
@@ -15370,6 +15376,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,
@@ -15771,6 +15778,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/linux-tdep.c b/gdb/linux-tdep.c
index bfb6404..b98401d 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1362,6 +1362,7 @@ linux_fill_prpsinfo (struct elf_internal_linux_prpsinfo *p)
/* Fills the "to_make_corefile_note" target vector. Builds the note
section for a corefile, and returns it in a malloc buffer. */

+DANGLING_CLEANUP
char *
linux_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size,
linux_collect_thread_registers_ftype collect)
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 71ce867..146a558 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -53,7 +53,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