RFC: introduce scoped cleanups

Tom Tromey
2013-05-09T18:56:15Z

This adds scoped cleanups to gdb. A scoped cleanup is a more
efficient, and checkable, alternative to the common idiom:

cleanup = make_cleanup (null_cleanup, NULL);
...
do_cleanups (cleanup);

... where the cleanup is always run when exiting the scope.

The efficiency comes from stack allocating the scoped cleanup. This
is probably minor. However, I've noticed myself sometimes avoiding
null cleanups on this basis, and it is nice to erase that bias.

The checking comes from using a GCC extension when available. This
check ensures that the scoped cleanup was either run or discarded when
the scope exits.

I'm curious to know what people think of this.

The patch includes an example use of scoped cleanups -- see the
linespec.c change.

This applies on top of my cleanup-checker series. However it probably
applies just as well to trunk if you drop the cleanup_check.py change.

Tom
---
gdb/cleanups.c | 70 +++++++++++++++++++++++++++++++++++---------
gdb/cleanups.h | 55 ++++++++++++++++++++++++++++++++++
gdb/contrib/cleanup_check.py | 2 +-
gdb/linespec.c | 4 +--
4 files changed, 114 insertions(+), 17 deletions(-)

diff --git a/gdb/cleanups.c b/gdb/cleanups.c
index 02db9f5..76d28c4 100644
--- a/gdb/cleanups.c
+++ b/gdb/cleanups.c
@@ -35,9 +35,10 @@
free that memory. This function will be called both when the cleanup
is executed and when it's discarded. */

-struct cleanup
+struct real_cleanup
{
- struct cleanup *next;
+ struct cleanup base;
+
void (*function) (void *);
void (*free_arg) (void *);
void *arg;
@@ -53,7 +54,7 @@ struct cleanup
This is const for a bit of extra robustness.
It is initialized to coax gcc into putting it into .rodata.
All fields are initialized to survive -Wextra. */
-static const struct cleanup sentinel_cleanup = { 0, 0, 0, 0 };
+static const struct real_cleanup sentinel_cleanup = { { 0, 0, 0 }, 0, 0, 0 };

/* Handy macro to use when referring to sentinel_cleanup. */
#define SENTINEL_CLEANUP ((struct cleanup *) &sentinel_cleanup)
@@ -79,15 +80,15 @@ static struct cleanup *
make_my_cleanup2 (struct cleanup **pmy_chain, make_cleanup_ftype *function,
void *arg, void (*free_arg) (void *))
{
- struct cleanup *new
- = (struct cleanup *) xmalloc (sizeof (struct cleanup));
+ struct real_cleanup *new = XNEW (struct real_cleanup);
struct cleanup *old_chain = *pmy_chain;

- new->next = *pmy_chain;
+ new->base.next = *pmy_chain;
+ new->base.scoped = 0;
new->function = function;
new->free_arg = free_arg;
new->arg = arg;
- *pmy_chain = new;
+ *pmy_chain = &new->base;

gdb_assert (old_chain != NULL);
return old_chain;
@@ -152,10 +153,17 @@ do_my_cleanups (struct cleanup **pmy_chain,
while ((ptr = *pmy_chain) != old_chain)
{
*pmy_chain = ptr->next; /* Do this first in case of recursion. */
- (*ptr->function) (ptr->arg);
- if (ptr->free_arg)
- (*ptr->free_arg) (ptr->arg);
- xfree (ptr);
+ if (ptr->scoped)
+ ptr->cleaned_up = 1;
+ else
+ {
+ struct real_cleanup *rc = (struct real_cleanup *) ptr;
+
+ (*rc->function) (rc->arg);
+ if (rc->free_arg)
+ (*rc->free_arg) (rc->arg);
+ xfree (ptr);
+ }
}
}

@@ -200,9 +208,16 @@ discard_my_cleanups (struct cleanup **pmy_chain,
while ((ptr = *pmy_chain) != old_chain)
{
*pmy_chain = ptr->next;
- if (ptr->free_arg)
- (*ptr->free_arg) (ptr->arg);
- xfree (ptr);
+ if (ptr->scoped)
+ ptr->cleaned_up = 1;
+ else
+ {
+ struct real_cleanup *rc = (struct real_cleanup *) ptr;
+
+ if (rc->free_arg)
+ (*rc->free_arg) (rc->arg);
+ xfree (ptr);
+ }
}
}

@@ -292,3 +307,30 @@ void
null_cleanup (void *arg)
{
}
+
+/* Initialize a scoped cleanup. */
+
+struct cleanup *
+init_scoped_cleanup (struct scoped_cleanup *cl)
+{
+ struct cleanup *old_chain = cleanup_chain;
+
+ cl->base.next = cleanup_chain;
+ cleanup_chain = &cl->base;
+
+ cl->base.scoped = 1;
+ cl->base.cleaned_up = 0;
+
+ return old_chain;
+}
+
+/* Verify that a scoped cleanup was in fact handled. */
+
+void
+cleanup_close_scope (struct scoped_cleanup *cl)
+{
+#ifdef SCOPED_CLEANUP_CHECKING
+ if (!cl->base.cleaned_up)
+ internal_warning (__FILE__, __LINE__, "scoped cleanup leaked");
+#endif /* SCOPED_CLEANUP_CHECKING */
+}
diff --git a/gdb/cleanups.h b/gdb/cleanups.h
index 463b923..9f8cbd4 100644
--- a/gdb/cleanups.h
+++ b/gdb/cleanups.h
@@ -66,4 +66,59 @@ extern void restore_final_cleanups (struct cleanup *);
to pass to do_cleanups. */
extern void null_cleanup (void *);

+
+/* You should continue to treat this as opaque. It is defined here
+ so that scoped cleanups can be stack-allocated and specially
+ treated. */
+
+struct cleanup
+{
+ struct cleanup *next;
+
+ /* True if this is a scoped cleanup. */
+ unsigned scoped : 1;
+
+ /* True if this is scoped cleanup has been cleaned up or discarded.
+ Not used for ordinary cleanups. */
+
+ unsigned cleaned_up : 1;
+};
+
+/* This is used for scoped cleanups. It should be treated as
+ opaque. */
+
+struct scoped_cleanup
+{
+ struct cleanup base;
+};
+
+extern struct cleanup *init_scoped_cleanup (struct scoped_cleanup *);
+
+extern void cleanup_close_scope (struct scoped_cleanup *);
+
+#if defined (__GNUC__) && __GNUC__ >= 4
+
+/* Define this to consolidate #if checking with the
+ implementation. */
+
+#define SCOPED_CLEANUP_CHECKING
+
+#define SCOPED_CLEANUP_ATTRIBUTE \
+ __attribute__ ((cleanup (cleanup_close_scope)))
+
+#else
+
+#define SCOPED_CLEANUP_ATTRIBUTE
+
+#endif
+
+/* Use this to declare a scoped cleanup. A scoped cleanup must be
+ cleaned up or discarded whenever the scope is exited. When
+ possible, this is checked at runtime. */
+
+#define SCOPED_CLEANUP(name) \
+ struct scoped_cleanup name ## __LINE__ \
+ SCOPED_CLEANUP_ATTRIBUTE; \
+ struct cleanup *name = init_scoped_cleanup (& (name ## __LINE__))
+
#endif /* CLEANUPS_H */
diff --git a/gdb/contrib/cleanup_check.py b/gdb/contrib/cleanup_check.py
index dca0ec8..a8f4fa1 100644
--- a/gdb/contrib/cleanup_check.py
+++ b/gdb/contrib/cleanup_check.py
@@ -53,7 +53,7 @@ special_names = set(['do_final_cleanups', 'discard_final_cleanups',
'restore_cleanups', 'restore_final_cleanups',
'exceptions_state_mc_init',
'make_my_cleanup2', 'make_final_cleanup', 'all_cleanups',
- 'save_my_cleanups', 'quit_target'])
+ 'save_my_cleanups', 'quit_target', 'init_scoped_cleanup'])

def needs_special_treatment(decl):
return decl.name in special_names
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 4bbd6e4..c6603d0 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -2821,7 +2821,7 @@ find_superclass_methods (VEC (typep) *superclasses,
{
int old_len = VEC_length (const_char_ptr, *result_names);
VEC (typep) *iter_classes;
- struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
+ SCOPED_CLEANUP (cleanup);

iter_classes = superclasses;
while (1)
@@ -2855,7 +2855,7 @@ find_method (struct linespec_state *self, VEC (symtab_p) *file_symtabs,
VEC (minsym_and_objfile_d) **minsyms)
{
struct symbol *sym;
- struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
+ SCOPED_CLEANUP (cleanup);
int ix;
int last_result_len;
VEC (typep) *superclass_vec;
--
1.8.1.4