* [PATCH v3 04/17] Use forward_scope_exit for scoped_finish_thread_state
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 17:10 ` Tom Tromey
2019-01-23 15:21 ` [PATCH v3 03/17] Introduce forward_scope_exit Pedro Alves
` (15 subsequent siblings)
16 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
This reimplements the manually-written scoped_finish_thread_state
class as a forward_scope_exit instanciation. forward_scope_exit has
the same interface as scoped_finish_thread_state, so nothing else has
to change.
A forward_scope_exit is preferred over make_scope_exit here because
infrun.c:normal_stop needs to wrap scoped_finish_thread_state in a
gdb::optional. Since we need the type there, might as well use it
everywhere.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
Andrew Burgess <andrew.burgess@embecosm.com>
* gdbthread.h: Include "common/forward-scope-exit.h".
(scoped_finish_thread_state): Redefine custom class in terms of
forward_scope_exit.
---
gdb/gdbthread.h | 28 +++-------------------------
1 file changed, 3 insertions(+), 25 deletions(-)
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 95db69605e..c35a54e75d 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -32,6 +32,7 @@ struct symtab;
#include "cli/cli-utils.h"
#include "common/refcounted-object.h"
#include "common-gdbthread.h"
+#include "common/forward-scope-exit.h"
struct inferior;
@@ -612,31 +613,8 @@ extern void finish_thread_state (ptid_t ptid);
/* Calls finish_thread_state on scope exit, unless release() is called
to disengage. */
-class scoped_finish_thread_state
-{
-public:
- explicit scoped_finish_thread_state (ptid_t ptid)
- : m_ptid (ptid)
- {}
-
- ~scoped_finish_thread_state ()
- {
- if (!m_released)
- finish_thread_state (m_ptid);
- }
-
- /* Disengage. */
- void release ()
- {
- m_released = true;
- }
-
- DISABLE_COPY_AND_ASSIGN (scoped_finish_thread_state);
-
-private:
- bool m_released = false;
- ptid_t m_ptid;
-};
+using scoped_finish_thread_state
+ = FORWARD_SCOPE_EXIT (finish_thread_state);
/* Commands with a prefix of `thread'. */
extern struct cmd_list_element *thread_cmd_list;
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 03/17] Introduce forward_scope_exit
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
2019-01-23 15:21 ` [PATCH v3 04/17] Use forward_scope_exit for scoped_finish_thread_state Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 13/17] Remove cleanup from stop_all_threads Pedro Alves
` (14 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
This adds a template that can be used to automatically instantiate
scope_exit-like types that wrap some cleanup function. The
instantiated type has a ctor that has the same interface as the
wrapped function. While the "magic" is just straight C++11, the
intended use is via the FORWARD_SCOPE_EXIT macro, which is a minimal
macro that avoids spelling out the wrapped function name more than
once:
void some_function (int foo, object *bar);
using some_function_fce = FORWARD_SCOPE_EXIT (some_function);
some_function_fce cleanup (some_int, some_obj_ptr);
The above runs:
some_function (some_int, some_obj_ptr);
at scope exit.
This is mainly useful as opposed to a simpler SCOPE_EXIT when you need
to:
- cancel the scope_exit, in which case you need the object's name
- wrap the scope_exit in a gdb::optional, in which case you need the
scope_exit's type in advance.
More details in the code comments.
gdb/ChangeLog
yyyy-mm-dd Pedro Alves <palves@redhat.com>
Andrew Burgess <andrew.burgess@embecosm.com>
* common/forward-scope-exit.h: New file.
---
gdb/common/forward-scope-exit.h | 123 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 123 insertions(+)
create mode 100644 gdb/common/forward-scope-exit.h
diff --git a/gdb/common/forward-scope-exit.h b/gdb/common/forward-scope-exit.h
new file mode 100644
index 0000000000..8d639151a4
--- /dev/null
+++ b/gdb/common/forward-scope-exit.h
@@ -0,0 +1,123 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef COMMON_FORWARD_SCOPE_EXIT_H
+#define COMMON_FORWARD_SCOPE_EXIT_H
+
+#include "common/scope-exit.h"
+#include <functional>
+
+/* A forward_scope_exit is like scope_exit, but instead of giving it a
+ callable, you instead specialize it for a given cleanup function,
+ and the generated class automatically has a constructor with the
+ same interface as the cleanup function. forward_scope_exit
+ captures the arguments passed to the ctor, and in turn passes those
+ as arguments to the wrapped cleanup function, when it is called at
+ scope exit time, from within the forward_scope_exit dtor. The
+ forward_scope_exit class can take any number of arguments, and is
+ cancelable if needed.
+
+ This allows usage like this:
+
+ void
+ delete_longjmp_breakpoint (int arg)
+ {
+ // Blah, blah, blah...
+ }
+
+ using longjmp_breakpoint_cleanup
+ = FORWARD_SCOPE_EXIT (delete_longjmp_breakpoint);
+
+ This above created a new cleanup class `longjmp_breakpoint_cleanup`
+ than can then be used like this:
+
+ longjmp_breakpoint_cleanup obj (thread);
+
+ // Blah, blah, blah...
+
+ obj.release (); // Optional cancel if needed.
+
+ forward_scope_exit is also handy when you would need to wrap a
+ scope_exit in a gdb::optional:
+
+ gdb::optional<longjmp_breakpoint_cleanup> cleanup;
+ if (some condition)
+ cleanup.emplace (thread);
+ ...
+ if (cleanup)
+ cleanup->release ();
+
+ since with scope exit, you would have to know the scope_exit's
+ callable template type when you create the gdb::optional:
+
+ gdb:optional<scope_exit<what goes here?>>
+
+ The "forward" naming fits both purposes shown above -- the class
+ "forwards" ctor arguments to the wrapped cleanup function at scope
+ exit time, and can also be used to "forward declare"
+ scope_exit-like objects. */
+
+namespace detail
+{
+
+/* Function and Signature are passed in the same type, in order to
+ extract Function's arguments' types in the specialization below.
+ Those are used to generate the constructor. */
+
+template<typename Function, Function *function, typename Signature>
+struct forward_scope_exit;
+
+template<typename Function, Function *function,
+ typename Res, typename... Args>
+class forward_scope_exit<Function, function, Res (Args...)>
+ : public scope_exit_base<forward_scope_exit<Function,
+ function,
+ Res (Args...)>>
+{
+ /* For access to on_exit(). */
+ friend scope_exit_base<forward_scope_exit<Function,
+ function,
+ Res (Args...)>>;
+
+public:
+ explicit forward_scope_exit (Args ...args)
+ : m_bind_function (std::bind (function, args...))
+ {
+ /* Nothing. */
+ }
+
+private:
+ void on_exit ()
+ {
+ m_bind_function ();
+ }
+
+ /* The function and the arguments passed to the ctor, all packed in
+ a std::bind. */
+ decltype (std::bind (function, std::declval<Args> ()...))
+ m_bind_function;
+};
+
+} /* namespace detail */
+
+/* This is the "public" entry point. It's a macro to avoid having to
+ name FUNC more than once. */
+
+#define FORWARD_SCOPE_EXIT(FUNC) \
+ detail::forward_scope_exit<decltype (FUNC), FUNC, decltype (FUNC)>
+
+#endif /* COMMON_FORWARD_SCOPE_EXIT_H */
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 13/17] Remove cleanup from stop_all_threads
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
2019-01-23 15:21 ` [PATCH v3 04/17] Use forward_scope_exit for scoped_finish_thread_state Pedro Alves
2019-01-23 15:21 ` [PATCH v3 03/17] Introduce forward_scope_exit Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 12/17] Remove clear_symtab_users_cleanup Pedro Alves
` (13 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
From: Tom Tromey <tom@tromey.com>
This removes the cleanup from stop_all_threads, replacing it with a
scope_exit.
gdb/ChangeLog
yyyy-mm-dd Tom Tromey <tom@tromey.com>
Pedro Alves <palves@redhat.com>
* infrun.c (disable_thread_events): Delete.
(stop_all_threads): Use SCOPE_EXIT.
---
gdb/infrun.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 76dedd272a..d954eae6ed 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4273,14 +4273,6 @@ save_waitstatus (struct thread_info *tp, struct target_waitstatus *ws)
}
}
-/* A cleanup that disables thread create/exit events. */
-
-static void
-disable_thread_events (void *arg)
-{
- target_thread_events (0);
-}
-
/* See infrun.h. */
void
@@ -4289,7 +4281,6 @@ stop_all_threads (void)
/* We may need multiple passes to discover all threads. */
int pass;
int iterations = 0;
- struct cleanup *old_chain;
gdb_assert (target_is_non_stop_p ());
@@ -4299,7 +4290,7 @@ stop_all_threads (void)
scoped_restore_current_thread restore_thread;
target_thread_events (1);
- old_chain = make_cleanup (disable_thread_events, NULL);
+ SCOPE_EXIT { target_thread_events (0); };
/* Request threads to stop, and then wait for the stops. Because
threads we already know about can spawn more threads while we're
@@ -4484,8 +4475,6 @@ stop_all_threads (void)
}
}
- do_cleanups (old_chain);
-
if (debug_infrun)
fprintf_unfiltered (gdb_stdlog, "infrun: stop_all_threads done\n");
}
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 12/17] Remove clear_symtab_users_cleanup
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (2 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 13/17] Remove cleanup from stop_all_threads Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 15/17] Update an obsolete cleanup comment Pedro Alves
` (12 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
This removes clear_symtab_users_cleanup, replacing it with uses of
forward_scope_exit.
gdb/ChangeLog:
2019-01-08 Tom Tromey <tom@tromey.com>
Pedro Alves <palves@redhat.com>
* symfile.c: Include forward-scope-exit.h.
(clear_symtab_users_cleanup): Replace forward declaration with
a FORWARD_SCOPE_EXIT.
(syms_from_objfile_1): Use the forward_scope_exit and
gdb::optional instead of cleanup_function.
(reread_symbols): Use the forward_scope_exit instead of
cleanup_function.
(clear_symtab_users_cleanup): Remove function.
---
gdb/symfile.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 7f800add8c..4875cbf7f5 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -59,6 +59,7 @@
#include "common/byte-vector.h"
#include "selftest.h"
#include "cli/cli-style.h"
+#include "common/forward-scope-exit.h"
#include <sys/types.h>
#include <fcntl.h>
@@ -79,7 +80,8 @@ void (*deprecated_show_load_progress) (const char *section,
void (*deprecated_pre_add_symbol_hook) (const char *);
void (*deprecated_post_add_symbol_hook) (void);
-static void clear_symtab_users_cleanup (void *ignore);
+using clear_symtab_users_cleanup
+ = FORWARD_SCOPE_EXIT (clear_symtab_users);
/* Global variables owned by this file. */
int readnow_symbol_files; /* Read full symbols immediately. */
@@ -923,7 +925,6 @@ syms_from_objfile_1 (struct objfile *objfile,
symfile_add_flags add_flags)
{
section_addr_info local_addr;
- struct cleanup *old_chain;
const int mainline = add_flags & SYMFILE_MAINLINE;
objfile_set_sym_fns (objfile, find_sym_fns (objfile->obfd));
@@ -945,7 +946,8 @@ syms_from_objfile_1 (struct objfile *objfile,
/* Make sure that partially constructed symbol tables will be cleaned up
if an error occurs during symbol reading. */
- old_chain = make_cleanup (null_cleanup, NULL);
+ gdb::optional<clear_symtab_users_cleanup> defer_clear_users;
+
std::unique_ptr<struct objfile> objfile_holder (objfile);
/* If ADDRS is NULL, put together a dummy address list.
@@ -958,7 +960,7 @@ syms_from_objfile_1 (struct objfile *objfile,
{
/* We will modify the main symbol table, make sure that all its users
will be cleaned up if an error occurs during symbol reading. */
- make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
+ defer_clear_users.emplace ((symfile_add_flag) 0);
/* Since no error yet, throw away the old symbol table. */
@@ -999,7 +1001,8 @@ syms_from_objfile_1 (struct objfile *objfile,
/* Discard cleanups as symbol reading was successful. */
objfile_holder.release ();
- discard_cleanups (old_chain);
+ if (defer_clear_users)
+ defer_clear_users->release ();
}
/* Same as syms_from_objfile_1, but also initializes the objfile
@@ -2441,7 +2444,6 @@ reread_symbols (void)
new_modtime = new_statbuf.st_mtime;
if (new_modtime != objfile->mtime)
{
- struct cleanup *old_cleanups;
struct section_offsets *offsets;
int num_offsets;
@@ -2461,7 +2463,7 @@ reread_symbols (void)
std::unique_ptr<struct objfile> objfile_holder (objfile);
/* We need to do this whenever any symbols go away. */
- old_cleanups = make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
+ clear_symtab_users_cleanup defer_clear_users (0);
if (exec_bfd != NULL
&& filename_cmp (bfd_get_filename (objfile->obfd),
@@ -2615,7 +2617,7 @@ reread_symbols (void)
/* Discard cleanups as symbol reading was successful. */
objfile_holder.release ();
- discard_cleanups (old_cleanups);
+ defer_clear_users.release ();
/* If the mtime has changed between the time we set new_modtime
and now, we *want* this to be out of date, so don't call stat
@@ -2892,12 +2894,6 @@ clear_symtab_users (symfile_add_flags add_flags)
if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0)
breakpoint_re_set ();
}
-
-static void
-clear_symtab_users_cleanup (void *ignore)
-{
- clear_symtab_users (0);
-}
\f
/* OVERLAYS:
The following code implements an abstraction for debugging overlay sections.
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 15/17] Update an obsolete cleanup comment
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (3 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 12/17] Remove clear_symtab_users_cleanup Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 16/17] Update cleanup comment in ui-out.h Pedro Alves
` (11 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
From: Tom Tromey <tom@tromey.com>
This updates a comment in fetch_inferior_event. The comment refers to
a cleanup that is now a scoped_restore_current_thread.
gdb/ChangeLog:
yyyy-mm-dd Tom Tromey <tom@tromey.com>
* infrun.c (fetch_inferior_event): Update comment.
---
gdb/infrun.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 6857467983..cdfdf49070 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3744,7 +3744,7 @@ fetch_inferior_event (void *client_data)
ptid_t finish_ptid = !target_is_non_stop_p () ? minus_one_ptid : ecs->ptid;
scoped_finish_thread_state finish_state (finish_ptid);
- /* Get executed before make_cleanup_restore_current_thread above to apply
+ /* Get executed before scoped_restore_current_thread above to apply
still for the thread which has thrown the exception. */
auto defer_bpstat_clear
= make_scope_exit (bpstat_clear_actions);
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 16/17] Update cleanup comment in ui-out.h
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (4 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 15/17] Update an obsolete cleanup comment Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 01/17] Rename ESC -> ESC_PARENS Pedro Alves
` (10 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
From: Tom Tromey <tom@tromey.com>
ui-out.h refers to some cleanup functions that no longer exist. This
updates the reference.
gdb/ChangeLog:
yyyy-mm-dd Tom Tromey <tom@tromey.com>
* ui-out.h (class ui_out_emit_type): Update comment.
---
gdb/ui-out.h | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 5f4eea5491..8d183060b5 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -195,11 +195,9 @@ class ui_out
ui_out_level *current_level () const;
};
-/* This is similar to make_cleanup_ui_out_tuple_begin_end and
- make_cleanup_ui_out_list_begin_end, but written as an RAII template
- class. It takes the ui_out_type as a template parameter. Normally
- this is used via the typedefs ui_out_emit_tuple and
- ui_out_emit_list. */
+/* Start a new tuple or list on construction, and end it on
+ destruction. Normally this is used via the typedefs
+ ui_out_emit_tuple and ui_out_emit_list. */
template<ui_out_type Type>
class ui_out_emit_type
{
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 01/17] Rename ESC -> ESC_PARENS
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (5 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 16/17] Update cleanup comment in ui-out.h Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 10/17] Remove cleanup_delete_std_terminate_breakpoint Pedro Alves
` (9 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
A following patch will include common/preprocessor.h in some .c file
that also includes readline.h, and that revealed a conflict -- ESC is
defined by readline.h as well (actually readline's chardefs.h) with a
completely unrelated meaning:
#define ESC CTRL('[')
Rename our version to avoid the conflict.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* common/preprocessor.h (ESC): Rename to ...
(ESC_PARENS): ... this.
* common/valid-expr.h (CHECK_VALID_EXPR_1, CHECK_VALID_EXPR_2)
(CHECK_VALID_EXPR_3, CHECK_VALID_EXPR_4): Adjust.
---
gdb/common/preprocessor.h | 2 +-
gdb/common/valid-expr.h | 18 +++++++++---------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/gdb/common/preprocessor.h b/gdb/common/preprocessor.h
index 647568ede6..a7c33d0978 100644
--- a/gdb/common/preprocessor.h
+++ b/gdb/common/preprocessor.h
@@ -30,6 +30,6 @@
/* Escape parens out. Useful if you need to pass an argument that
includes commas to another macro. */
-#define ESC(...) __VA_ARGS__
+#define ESC_PARENS(...) __VA_ARGS__
#endif /* COMMON_PREPROC */
diff --git a/gdb/common/valid-expr.h b/gdb/common/valid-expr.h
index 9289448365..603c76e8f1 100644
--- a/gdb/common/valid-expr.h
+++ b/gdb/common/valid-expr.h
@@ -85,24 +85,24 @@
another variant. */
#define CHECK_VALID_EXPR_1(T1, VALID, EXPR_TYPE, EXPR) \
- CHECK_VALID_EXPR_INT (ESC (typename T1), \
- ESC (T1), \
+ CHECK_VALID_EXPR_INT (ESC_PARENS (typename T1), \
+ ESC_PARENS (T1), \
VALID, EXPR_TYPE, EXPR)
#define CHECK_VALID_EXPR_2(T1, T2, VALID, EXPR_TYPE, EXPR) \
- CHECK_VALID_EXPR_INT (ESC (typename T1, typename T2), \
- ESC (T1, T2), \
+ CHECK_VALID_EXPR_INT (ESC_PARENS(typename T1, typename T2), \
+ ESC_PARENS (T1, T2), \
VALID, EXPR_TYPE, EXPR)
#define CHECK_VALID_EXPR_3(T1, T2, T3, VALID, EXPR_TYPE, EXPR) \
- CHECK_VALID_EXPR_INT (ESC (typename T1, typename T2, typename T3), \
- ESC (T1, T2, T3), \
+ CHECK_VALID_EXPR_INT (ESC_PARENS (typename T1, typename T2, typename T3), \
+ ESC_PARENS (T1, T2, T3), \
VALID, EXPR_TYPE, EXPR)
#define CHECK_VALID_EXPR_4(T1, T2, T3, T4, VALID, EXPR_TYPE, EXPR) \
- CHECK_VALID_EXPR_INT (ESC (typename T1, typename T2, \
- typename T3, typename T4), \
- ESC (T1, T2, T3, T4), \
+ CHECK_VALID_EXPR_INT (ESC_PARENS (typename T1, typename T2, \
+ typename T3, typename T4), \
+ ESC_PARENS (T1, T2, T3, T4), \
VALID, EXPR_TYPE, EXPR)
#endif /* COMMON_VALID_EXPR_H */
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 10/17] Remove cleanup_delete_std_terminate_breakpoint
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (6 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 01/17] Rename ESC -> ESC_PARENS Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 14/17] Remove remaining cleanup from fetch_inferior_event Pedro Alves
` (8 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
From: Tom Tromey <tom@tromey.com>
This removes cleanup_delete_std_terminate_breakpoint, replacing it
with a use of SCOPE_EXIT.
gdb/ChangeLog:
yyyy-mm-dd Tom Tromey <tom@tromey.com>
Pedro Alves <palves@redhat.com>
* infcall.c (cleanup_delete_std_terminate_breakpoint): Remove.
(call_function_by_hand_dummy): Use SCOPE_EXIT.
---
gdb/infcall.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 14b0cbc716..6ca479ac1f 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -40,6 +40,7 @@
#include "interps.h"
#include "thread-fsm.h"
#include <algorithm>
+#include "common/scope-exit.h"
/* If we can't find a function's name from its address,
we print this instead. */
@@ -675,13 +676,6 @@ run_inferior_call (struct call_thread_fsm *sm,
return caught_error;
}
-/* A cleanup function that calls delete_std_terminate_breakpoint. */
-static void
-cleanup_delete_std_terminate_breakpoint (void *ignore)
-{
- delete_std_terminate_breakpoint ();
-}
-
/* See infcall.h. */
struct value *
@@ -727,7 +721,6 @@ call_function_by_hand_dummy (struct value *function,
struct frame_id dummy_id;
struct frame_info *frame;
struct gdbarch *gdbarch;
- struct cleanup *terminate_bp_cleanup;
ptid_t call_thread_ptid;
struct gdb_exception e;
char name_buf[RAW_FUNCTION_ADDRESS_SIZE];
@@ -1122,8 +1115,7 @@ call_function_by_hand_dummy (struct value *function,
dummy_dtor, dummy_dtor_data);
/* Register a clean-up for unwind_on_terminating_exception_breakpoint. */
- terminate_bp_cleanup = make_cleanup (cleanup_delete_std_terminate_breakpoint,
- NULL);
+ SCOPE_EXIT { delete_std_terminate_breakpoint (); };
/* - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP - SNIP -
If you're looking to implement asynchronous dummy-frames, then
@@ -1184,7 +1176,6 @@ call_function_by_hand_dummy (struct value *function,
maybe_remove_breakpoints ();
- do_cleanups (terminate_bp_cleanup);
gdb_assert (retval != NULL);
return retval;
}
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 14/17] Remove remaining cleanup from fetch_inferior_event
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (7 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 10/17] Remove cleanup_delete_std_terminate_breakpoint Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 06/17] Replace delete_longjmp_breakpoint_cleanup with a forward_scope_exit type Pedro Alves
` (7 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
From: Tom Tromey <tom@tromey.com>
This removes the remaining cleanup from fetch_inferior_event,
replacing it with a SCOPE_EXIT. This required introducing a new scope
and reindenting.
gdb/ChangeLog:
yyyy-mm-dd Tom Tromey <tom@tromey.com>
Pedro Alves <palves@redhat.com>
* infrun.c (reinstall_readline_callback_handler_cleanup): Remove
parameter.
(fetch_inferior_event): Use SCOPE_EXIT.
---
gdb/infrun.c | 202 ++++++++++++++++++++++++++++++-----------------------------
1 file changed, 102 insertions(+), 100 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d954eae6ed..6857467983 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3585,7 +3585,7 @@ wait_for_inferior (void)
input. */
static void
-reinstall_readline_callback_handler_cleanup (void *arg)
+reinstall_readline_callback_handler_cleanup ()
{
struct ui *ui = current_ui;
@@ -3687,7 +3687,6 @@ fetch_inferior_event (void *client_data)
{
struct execution_control_state ecss;
struct execution_control_state *ecs = &ecss;
- struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
int cmd_done = 0;
ptid_t waiton_ptid = minus_one_ptid;
@@ -3699,116 +3698,119 @@ fetch_inferior_event (void *client_data)
scoped_restore save_ui = make_scoped_restore (¤t_ui, main_ui);
/* End up with readline processing input, if necessary. */
- make_cleanup (reinstall_readline_callback_handler_cleanup, NULL);
-
- /* We're handling a live event, so make sure we're doing live
- debugging. If we're looking at traceframes while the target is
- running, we're going to need to get back to that mode after
- handling the event. */
- gdb::optional<scoped_restore_current_traceframe> maybe_restore_traceframe;
- if (non_stop)
- {
- maybe_restore_traceframe.emplace ();
- set_current_traceframe (-1);
- }
-
- gdb::optional<scoped_restore_current_thread> maybe_restore_thread;
-
- if (non_stop)
- /* In non-stop mode, the user/frontend should not notice a thread
- switch due to internal events. Make sure we reverse to the
- user selected thread and frame after handling the event and
- running any breakpoint commands. */
- maybe_restore_thread.emplace ();
-
- overlay_cache_invalid = 1;
- /* Flush target cache before starting to handle each event. Target
- was running and cache could be stale. This is just a heuristic.
- Running threads may modify target memory, but we don't get any
- event. */
- target_dcache_invalidate ();
-
- scoped_restore save_exec_dir
- = make_scoped_restore (&execution_direction, target_execution_direction ());
-
- ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws,
- target_can_async_p () ? TARGET_WNOHANG : 0);
-
- if (debug_infrun)
- print_target_wait_results (waiton_ptid, ecs->ptid, &ecs->ws);
-
- /* If an error happens while handling the event, propagate GDB's
- knowledge of the executing state to the frontend/user running
- state. */
- ptid_t finish_ptid = !target_is_non_stop_p () ? minus_one_ptid : ecs->ptid;
- scoped_finish_thread_state finish_state (finish_ptid);
-
- /* Get executed before make_cleanup_restore_current_thread above to apply
- still for the thread which has thrown the exception. */
- auto defer_bpstat_clear
- = make_scope_exit (bpstat_clear_actions);
- auto defer_delete_threads
- = make_scope_exit (delete_just_stopped_threads_infrun_breakpoints);
-
- /* Now figure out what to do with the result of the result. */
- handle_inferior_event (ecs);
+ {
+ SCOPE_EXIT { reinstall_readline_callback_handler_cleanup (); };
+
+ /* We're handling a live event, so make sure we're doing live
+ debugging. If we're looking at traceframes while the target is
+ running, we're going to need to get back to that mode after
+ handling the event. */
+ gdb::optional<scoped_restore_current_traceframe> maybe_restore_traceframe;
+ if (non_stop)
+ {
+ maybe_restore_traceframe.emplace ();
+ set_current_traceframe (-1);
+ }
- if (!ecs->wait_some_more)
- {
- struct inferior *inf = find_inferior_ptid (ecs->ptid);
- int should_stop = 1;
- struct thread_info *thr = ecs->event_thread;
+ gdb::optional<scoped_restore_current_thread> maybe_restore_thread;
+
+ if (non_stop)
+ /* In non-stop mode, the user/frontend should not notice a thread
+ switch due to internal events. Make sure we reverse to the
+ user selected thread and frame after handling the event and
+ running any breakpoint commands. */
+ maybe_restore_thread.emplace ();
+
+ overlay_cache_invalid = 1;
+ /* Flush target cache before starting to handle each event. Target
+ was running and cache could be stale. This is just a heuristic.
+ Running threads may modify target memory, but we don't get any
+ event. */
+ target_dcache_invalidate ();
+
+ scoped_restore save_exec_dir
+ = make_scoped_restore (&execution_direction,
+ target_execution_direction ());
+
+ ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws,
+ target_can_async_p () ? TARGET_WNOHANG : 0);
+
+ if (debug_infrun)
+ print_target_wait_results (waiton_ptid, ecs->ptid, &ecs->ws);
+
+ /* If an error happens while handling the event, propagate GDB's
+ knowledge of the executing state to the frontend/user running
+ state. */
+ ptid_t finish_ptid = !target_is_non_stop_p () ? minus_one_ptid : ecs->ptid;
+ scoped_finish_thread_state finish_state (finish_ptid);
+
+ /* Get executed before make_cleanup_restore_current_thread above to apply
+ still for the thread which has thrown the exception. */
+ auto defer_bpstat_clear
+ = make_scope_exit (bpstat_clear_actions);
+ auto defer_delete_threads
+ = make_scope_exit (delete_just_stopped_threads_infrun_breakpoints);
+
+ /* Now figure out what to do with the result of the result. */
+ handle_inferior_event (ecs);
+
+ if (!ecs->wait_some_more)
+ {
+ struct inferior *inf = find_inferior_ptid (ecs->ptid);
+ int should_stop = 1;
+ struct thread_info *thr = ecs->event_thread;
- delete_just_stopped_threads_infrun_breakpoints ();
+ delete_just_stopped_threads_infrun_breakpoints ();
- if (thr != NULL)
- {
- struct thread_fsm *thread_fsm = thr->thread_fsm;
+ if (thr != NULL)
+ {
+ struct thread_fsm *thread_fsm = thr->thread_fsm;
- if (thread_fsm != NULL)
- should_stop = thread_fsm_should_stop (thread_fsm, thr);
- }
+ if (thread_fsm != NULL)
+ should_stop = thread_fsm_should_stop (thread_fsm, thr);
+ }
- if (!should_stop)
- {
- keep_going (ecs);
- }
- else
- {
- int should_notify_stop = 1;
- int proceeded = 0;
+ if (!should_stop)
+ {
+ keep_going (ecs);
+ }
+ else
+ {
+ int should_notify_stop = 1;
+ int proceeded = 0;
- clean_up_just_stopped_threads_fsms (ecs);
+ clean_up_just_stopped_threads_fsms (ecs);
- if (thr != NULL && thr->thread_fsm != NULL)
- {
- should_notify_stop
- = thread_fsm_should_notify_stop (thr->thread_fsm);
- }
+ if (thr != NULL && thr->thread_fsm != NULL)
+ {
+ should_notify_stop
+ = thread_fsm_should_notify_stop (thr->thread_fsm);
+ }
- if (should_notify_stop)
- {
- /* We may not find an inferior if this was a process exit. */
- if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
- proceeded = normal_stop ();
- }
+ if (should_notify_stop)
+ {
+ /* We may not find an inferior if this was a process exit. */
+ if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY)
+ proceeded = normal_stop ();
+ }
- if (!proceeded)
- {
- inferior_event_handler (INF_EXEC_COMPLETE, NULL);
- cmd_done = 1;
- }
- }
- }
+ if (!proceeded)
+ {
+ inferior_event_handler (INF_EXEC_COMPLETE, NULL);
+ cmd_done = 1;
+ }
+ }
+ }
- defer_delete_threads.release ();
- defer_bpstat_clear.release ();
+ defer_delete_threads.release ();
+ defer_bpstat_clear.release ();
- /* No error, don't finish the thread states yet. */
- finish_state.release ();
+ /* No error, don't finish the thread states yet. */
+ finish_state.release ();
- /* Revert thread and frame. */
- do_cleanups (old_chain);
+ /* This scope is used to ensure that readline callbacks are
+ reinstalled here. */
+ }
/* If a UI was in sync execution mode, and now isn't, restore its
prompt (a synchronous execution command has finished, and we're
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 06/17] Replace delete_longjmp_breakpoint_cleanup with a forward_scope_exit type
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (8 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 14/17] Remove remaining cleanup from fetch_inferior_event Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 08/17] Remove delete_just_stopped_threads_infrun_breakpoints_cleanup Pedro Alves
` (6 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
From: Tom Tromey <tom@tromey.com>
This removes delete_longjmp_breakpoint_cleanup in favor of forward_scope_exit.
gdb/ChangeLog:
yyyy-mm-dd Tom Tromey <tom@tromey.com>
Andrew Burgess <andrew.burgess@embecosm.com>
Pedro Alves <palves@redhat.com>
* breakpoint.c (until_break_command): Use
delete_longjmp_breakpoint_cleanup class.
* infcmd.c (delete_longjmp_breakpoint_cleanup): Remove function.
(until_next_command): Use delete_longjmp_breakpoint_cleanup class.
* inferior.h: Include forward-scope-exit.h.
(delete_longjmp_breakpoint_cleanup): Replace function declaration
with FORWARD_SCOPE_EXIT type.
---
gdb/breakpoint.c | 11 ++++++-----
gdb/infcmd.c | 12 ++----------
gdb/inferior.h | 4 +++-
3 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 3166fa0129..999809c312 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -11073,7 +11073,6 @@ until_break_command (const char *arg, int from_tty, int anywhere)
struct gdbarch *frame_gdbarch;
struct frame_id stack_frame_id;
struct frame_id caller_frame_id;
- struct cleanup *old_chain;
int thread;
struct thread_info *tp;
struct until_break_fsm *sm;
@@ -11106,8 +11105,6 @@ until_break_command (const char *arg, int from_tty, int anywhere)
tp = inferior_thread ();
thread = tp->global_num;
- old_chain = make_cleanup (null_cleanup, NULL);
-
/* Note linespec handling above invalidates the frame chain.
Installing a breakpoint also invalidates the frame chain (as it
may need to switch threads), so do any frame handling before
@@ -11122,6 +11119,9 @@ until_break_command (const char *arg, int from_tty, int anywhere)
one. */
breakpoint_up caller_breakpoint;
+
+ gdb::optional<delete_longjmp_breakpoint_cleanup> lj_deleter;
+
if (frame_id_p (caller_frame_id))
{
struct symtab_and_line sal2;
@@ -11136,7 +11136,7 @@ until_break_command (const char *arg, int from_tty, int anywhere)
bp_until);
set_longjmp_breakpoint (tp, caller_frame_id);
- make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
+ lj_deleter.emplace (thread);
}
/* set_momentary_breakpoint could invalidate FRAME. */
@@ -11159,7 +11159,8 @@ until_break_command (const char *arg, int from_tty, int anywhere)
std::move (caller_breakpoint));
tp->thread_fsm = &sm->thread_fsm;
- discard_cleanups (old_chain);
+ if (lj_deleter)
+ lj_deleter->release ();
proceed (-1, GDB_SIGNAL_DEFAULT);
}
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 3c3add89ab..fafb7e2ec4 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -947,13 +947,6 @@ nexti_command (const char *count_string, int from_tty)
step_1 (1, 1, count_string);
}
-void
-delete_longjmp_breakpoint_cleanup (void *arg)
-{
- int thread = * (int *) arg;
- delete_longjmp_breakpoint (thread);
-}
-
/* Data for the FSM that manages the step/next/stepi/nexti
commands. */
@@ -1517,7 +1510,6 @@ until_next_command (int from_tty)
struct symtab_and_line sal;
struct thread_info *tp = inferior_thread ();
int thread = tp->global_num;
- struct cleanup *old_chain;
struct until_next_fsm *sm;
clear_proceed_status (0);
@@ -1556,11 +1548,11 @@ until_next_command (int from_tty)
tp->control.step_over_calls = STEP_OVER_ALL;
set_longjmp_breakpoint (tp, get_frame_id (frame));
- old_chain = make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
+ delete_longjmp_breakpoint_cleanup lj_deleter (thread);
sm = new_until_next_fsm (command_interp (), tp->global_num);
tp->thread_fsm = &sm->thread_fsm;
- discard_cleanups (old_chain);
+ lj_deleter.release ();
proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT);
}
diff --git a/gdb/inferior.h b/gdb/inferior.h
index a82df1a52a..cadaaedf22 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -51,6 +51,7 @@ struct thread_info;
#include "symfile-add-flags.h"
#include "common/refcounted-object.h"
+#include "common/forward-scope-exit.h"
#include "common-inferior.h"
#include "gdbthread.h"
@@ -198,7 +199,8 @@ extern void continue_1 (int all_threads);
extern void interrupt_target_1 (int all_threads);
-extern void delete_longjmp_breakpoint_cleanup (void *arg);
+using delete_longjmp_breakpoint_cleanup
+ = FORWARD_SCOPE_EXIT (delete_longjmp_breakpoint);
extern void detach_command (const char *, int);
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 08/17] Remove delete_just_stopped_threads_infrun_breakpoints_cleanup
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (9 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 06/17] Replace delete_longjmp_breakpoint_cleanup with a forward_scope_exit type Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 07/17] Remove remaining cleanup from gdb/breakpoint.c Pedro Alves
` (5 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
From: Tom Tromey <tom@tromey.com>
v3: sorted earlier in the series, and replaces the cleanup in
fetch_inferior_event here too instead of in some other patch.
This removes delete_just_stopped_threads_infrun_breakpoints_cleanup,
replacing it with uses of scope_exit.
gdb/ChangeLog:
yyyy-mm-dd Tom Tromey <tom@tromey.com>
Pedro Alves <palves@redhat.com>
* infrun.c: Include "common/scope-exit.h"
(delete_just_stopped_threads_infrun_breakpoints_cleanup): Remove.
(wait_for_inferior): Use SCOPE_EXIT.
(fetch_inferior_event): Use scope_exit.
---
gdb/infrun.c | 21 +++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 150288264f..065fbf06d8 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -67,6 +67,7 @@
#include "progspace-and-thread.h"
#include "common/gdb_optional.h"
#include "arch-utils.h"
+#include "common/scope-exit.h"
/* Prototypes for local functions */
@@ -3247,14 +3248,6 @@ delete_just_stopped_threads_single_step_breakpoints (void)
for_each_just_stopped_thread (delete_single_step_breakpoints);
}
-/* A cleanup wrapper. */
-
-static void
-delete_just_stopped_threads_infrun_breakpoints_cleanup (void *arg)
-{
- delete_just_stopped_threads_infrun_breakpoints ();
-}
-
/* See infrun.h. */
void
@@ -3538,15 +3531,11 @@ prepare_for_detach (void)
void
wait_for_inferior (void)
{
- struct cleanup *old_cleanups;
-
if (debug_infrun)
fprintf_unfiltered
(gdb_stdlog, "infrun: wait_for_inferior ()\n");
- old_cleanups
- = make_cleanup (delete_just_stopped_threads_infrun_breakpoints_cleanup,
- NULL);
+ SCOPE_EXIT { delete_just_stopped_threads_infrun_breakpoints (); };
/* If an error happens while handling the event, propagate GDB's
knowledge of the executing state to the frontend/user running
@@ -3583,8 +3572,6 @@ wait_for_inferior (void)
/* No error, don't finish the state yet. */
finish_state.release ();
-
- do_cleanups (old_cleanups);
}
/* Cleanup that reinstalls the readline callback handler, if the
@@ -3760,7 +3747,8 @@ fetch_inferior_event (void *client_data)
still for the thread which has thrown the exception. */
struct cleanup *ts_old_chain = make_bpstat_clear_actions_cleanup ();
- make_cleanup (delete_just_stopped_threads_infrun_breakpoints_cleanup, NULL);
+ auto defer_delete_threads
+ = make_scope_exit (delete_just_stopped_threads_infrun_breakpoints);
/* Now figure out what to do with the result of the result. */
handle_inferior_event (ecs);
@@ -3813,6 +3801,7 @@ fetch_inferior_event (void *client_data)
}
}
+ defer_delete_threads.release ();
discard_cleanups (ts_old_chain);
/* No error, don't finish the thread states yet. */
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 07/17] Remove remaining cleanup from gdb/breakpoint.c
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (10 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 08/17] Remove delete_just_stopped_threads_infrun_breakpoints_cleanup Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 05/17] Use SCOPE_EXIT in gdbarch-selftest.c Pedro Alves
` (4 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
From: Tom Tromey <tom@tromey.com>
In v3: remove the "have BKPT_CHAIN already discarded" comment too.
The remaining null cleanup in breakpoint.c does not seem to protect
anything, so remove it.
gdb/ChangeLog:
yyyy-mm-dd Tom Tromey <tom@tromey.com>
Pedro Alves <palves@redhat.com>
* breakpoint.c (create_breakpoint): Remove cleanup.
---
gdb/breakpoint.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 999809c312..33c5bfef43 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9230,7 +9230,6 @@ create_breakpoint (struct gdbarch *gdbarch,
unsigned flags)
{
struct linespec_result canonical;
- struct cleanup *bkpt_chain = NULL;
int pending = 0;
int task = 0;
int prev_bkpt_count = breakpoint_count;
@@ -9280,12 +9279,6 @@ create_breakpoint (struct gdbarch *gdbarch,
if (!pending && canonical.lsals.empty ())
return 0;
- /* ----------------------------- SNIP -----------------------------
- Anything added to the cleanup chain beyond this point is assumed
- to be part of a breakpoint. If the breakpoint create succeeds
- then the memory is not reclaimed. */
- bkpt_chain = make_cleanup (null_cleanup, 0);
-
/* Resolve all line numbers to PC's and verify that the addresses
are ok for the target. */
if (!pending)
@@ -9384,11 +9377,6 @@ create_breakpoint (struct gdbarch *gdbarch,
prev_breakpoint_count = prev_bkpt_count;
}
- /* That's it. Discard the cleanups for data inserted into the
- breakpoint. */
- discard_cleanups (bkpt_chain);
-
- /* error call may happen here - have BKPT_CHAIN already discarded. */
update_global_location_list (UGLL_MAY_INSERT);
return 1;
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 05/17] Use SCOPE_EXIT in gdbarch-selftest.c
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (11 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 07/17] Remove remaining cleanup from gdb/breakpoint.c Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 02/17] Introduce scope_exit Pedro Alves
` (3 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
Replace the custom local class with a SCOPE_EXIT.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
* gdbarch-selftests.c (struct on_exit): Use SCOPE_EXIT.
---
gdb/gdbarch-selftests.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index 50062abe8a..64d1e543e4 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -104,13 +104,7 @@ register_to_value_test (struct gdbarch *gdbarch)
push_target (&mock_target);
/* Pop it again on exit (return/exception). */
- struct on_exit
- {
- ~on_exit ()
- {
- pop_all_targets_at_and_above (process_stratum);
- }
- } pop_targets;
+ SCOPE_EXIT { pop_all_targets_at_and_above (process_stratum); };
/* Switch to the mock thread. */
scoped_restore restore_inferior_ptid
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 02/17] Introduce scope_exit
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (12 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 05/17] Use SCOPE_EXIT in gdbarch-selftest.c Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:21 ` [PATCH v3 17/17] Use scope_exit in regcache.c Pedro Alves
` (2 subsequent siblings)
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
This add a new template class scope_exit. scope_exit is a
general-purpose scope guard that calls its exit function at the end of
the current scope. A scope_exit may be canceled by calling the
"release" method. The API is modeled on P0052R5 - Generic Scope Guard
and RAII Wrapper for the Standard Library, which is itself based on
Andrej Alexandrescu's ScopeGuard/SCOPE_EXIT.
The main advantage of scope_exit is avoiding writing single-use RAII
classes and its boilerplate. Following patches will remove a few of
such classes.
There are two forms available:
- The "make_scope_exit" form allows canceling the scope guard. Use
it like this:
auto cleanup = make_scope_exit ( <function, function object, lambda> );
...
cleanup.release (); // cancel
- If you don't need to cancel the guard, you can use the SCOPE_EXIT
macro, like this:
SCOPE_EXIT { /* any code you like here. */ }
Note: scope_exit instances do not allocate anything on the heap.
gdb/ChangeLog:
yyyy-mm-dd Pedro Alves <palves@redhat.com>
Andrew Burgess <andrew.burgess@embecosm.com>
Tom Tromey <tom@tromey.com>
* common/scope-exit.h: New file.
---
gdb/common/scope-exit.h | 186 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 186 insertions(+)
create mode 100644 gdb/common/scope-exit.h
diff --git a/gdb/common/scope-exit.h b/gdb/common/scope-exit.h
new file mode 100644
index 0000000000..8cdbec305a
--- /dev/null
+++ b/gdb/common/scope-exit.h
@@ -0,0 +1,186 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef COMMON_SCOPE_EXIT_H
+#define COMMON_SCOPE_EXIT_H
+
+#include <functional>
+#include <type_traits>
+#include "common/preprocessor.h"
+
+/* scope_exit is a general-purpose scope guard that calls its exit
+ function at the end of the current scope. A scope_exit may be
+ canceled by calling the "release" method. The API is modeled on
+ P0052R5 - Generic Scope Guard and RAII Wrapper for the Standard
+ Library, which is itself based on Andrej Alexandrescu's
+ ScopeGuard/SCOPE_EXIT.
+
+ There are two forms available:
+
+ - The "make_scope_exit" form allows canceling the scope guard. Use
+ it like this:
+
+ auto cleanup = make_scope_exit ( <function, function object, lambda> );
+ ...
+ cleanup.release (); // cancel
+
+ - If you don't need to cancel the guard, you can use the SCOPE_EXIT
+ macro, like this:
+
+ SCOPE_EXIT
+ {
+ // any code you like here.
+ }
+
+ See also forward_scope_exit.
+*/
+
+/* CRTP base class for cancelable scope_exit-like classes. Implements
+ the common call-custom-function-from-dtor functionality. Classes
+ that inherit this implement the on_exit() method, which is called
+ from scope_exit_base's dtor. */
+
+template <typename CRTP>
+class scope_exit_base
+{
+public:
+ scope_exit_base () = default;
+
+ ~scope_exit_base ()
+ {
+ if (!m_released)
+ {
+ auto *self = static_cast<CRTP *> (this);
+ self->on_exit ();
+ }
+ }
+
+ /* This is needed for make_scope_exit because copy elision isn't
+ guaranteed until C++17. An optimizing compiler will usually skip
+ calling this, but it must exist. */
+ scope_exit_base (const scope_exit_base &other)
+ : m_released (other.m_released)
+ {
+ other.m_released = true;
+ }
+
+ void operator= (const scope_exit_base &) = delete;
+
+ /* If this is called, then the wrapped function will not be called
+ on destruction. */
+ void release () noexcept
+ {
+ m_released = true;
+ }
+
+private:
+
+ /* True if released. Mutable because of the copy ctor hack
+ above. */
+ mutable bool m_released = false;
+};
+
+/* The scope_exit class. */
+
+template<typename EF>
+class scope_exit : public scope_exit_base<scope_exit<EF>>
+{
+ /* For access to on_exit(). */
+ friend scope_exit_base<scope_exit<EF>>;
+
+public:
+
+ template<typename EFP,
+ typename = gdb::Requires<std::is_constructible<EF, EFP>>>
+ scope_exit (EFP &&f)
+ try : m_exit_function ((!std::is_lvalue_reference<EFP>::value
+ && std::is_nothrow_constructible<EF, EFP>::value)
+ ? std::move (f)
+ : f)
+ {
+ }
+ catch (...)
+ {
+ /* "If the initialization of exit_function throws an exception,
+ calls f()." */
+ f ();
+ }
+
+ template<typename EFP,
+ typename = gdb::Requires<std::is_constructible<EF, EFP>>>
+ scope_exit (scope_exit &&rhs)
+ noexcept (std::is_nothrow_move_constructible<EF>::value
+ || std::is_nothrow_copy_constructible<EF>::value)
+ : m_exit_function (std::is_nothrow_constructible<EFP>::value
+ ? std::move (rhs)
+ : rhs)
+ {
+ rhs.release ();
+ }
+
+ /* This is needed for make_scope_exit because copy elision isn't
+ guaranteed until C++17. An optimizing compiler will usually skip
+ calling this, but it must exist. */
+ scope_exit (const scope_exit &other)
+ : scope_exit_base<scope_exit<EF>> (other),
+ m_exit_function (other.m_exit_function)
+ {
+ }
+
+ void operator= (const scope_exit &) = delete;
+ void operator= (scope_exit &&) = delete;
+
+private:
+ void on_exit ()
+ {
+ m_exit_function ();
+ }
+
+ /* The function to call on scope exit. */
+ EF m_exit_function;
+};
+
+template <typename EF>
+scope_exit<typename std::decay<EF>::type>
+make_scope_exit (EF &&f)
+{
+ return scope_exit<typename std::decay<EF>::type> (std::forward<EF> (f));
+}
+
+namespace detail
+{
+
+enum class scope_exit_lhs {};
+
+template<typename EF>
+scope_exit<typename std::decay<EF>::type>
+operator+ (scope_exit_lhs, EF &&rhs)
+{
+ return scope_exit<typename std::decay<EF>::type> (std::forward<EF> (rhs));
+}
+
+}
+
+/* Register a block of code to run on scope exit. Note that the local
+ context is captured by reference, which means you should be careful
+ to avoid inadvertently changing a captured local's value before the
+ scope exit runs. */
+
+#define SCOPE_EXIT \
+ auto CONCAT(scope_exit_, __LINE__) = ::detail::scope_exit_lhs () + [&] ()
+
+#endif /* COMMON_SCOPE_EXIT_H */
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 17/17] Use scope_exit in regcache.c
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (13 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 02/17] Introduce scope_exit Pedro Alves
@ 2019-01-23 15:21 ` Pedro Alves
2019-01-23 15:28 ` [PATCH v3 09/17] Remove make_bpstat_clear_actions_cleanup Pedro Alves
2019-01-23 17:33 ` [PATCH v3 00/17] Remove some cleanups using scope_exit Tom Tromey
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
From: Tom Tromey <tom@tromey.com>
This removes the regcache_invalidator class in favor of a scope_exit.
This seems like an improvement (albeit a minor one) because
regcache_invalidator is only used in a single spot.
gdb/ChangeLog
2019-01-22 Tom Tromey <tom@tromey.com>
Pedro Alves <palves@redhat.com>
* regcache.c (class regcache_invalidator): Remove.
(regcache::raw_write): Use make_scope_exit.
---
gdb/regcache.c | 34 ++--------------------------------
1 file changed, 2 insertions(+), 32 deletions(-)
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 4a68390c5f..a354e15d2d 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -219,37 +219,6 @@ reg_buffer::arch () const
return m_descr->gdbarch;
}
-/* Cleanup class for invalidating a register. */
-
-class regcache_invalidator
-{
-public:
-
- regcache_invalidator (struct regcache *regcache, int regnum)
- : m_regcache (regcache),
- m_regnum (regnum)
- {
- }
-
- ~regcache_invalidator ()
- {
- if (m_regcache != nullptr)
- m_regcache->invalidate (m_regnum);
- }
-
- DISABLE_COPY_AND_ASSIGN (regcache_invalidator);
-
- void release ()
- {
- m_regcache = nullptr;
- }
-
-private:
-
- struct regcache *m_regcache;
- int m_regnum;
-};
-
/* Return a pointer to register REGNUM's buffer cache. */
gdb_byte *
@@ -769,7 +738,8 @@ regcache::raw_write (int regnum, const gdb_byte *buf)
/* Invalidate the register after it is written, in case of a
failure. */
- regcache_invalidator invalidator (this, regnum);
+ auto invalidator
+ = make_scope_exit ([&] { this->invalidate (regnum); });
target_store_registers (this, regnum);
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 09/17] Remove make_bpstat_clear_actions_cleanup
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (14 preceding siblings ...)
2019-01-23 15:21 ` [PATCH v3 17/17] Use scope_exit in regcache.c Pedro Alves
@ 2019-01-23 15:28 ` Pedro Alves
2019-01-23 17:33 ` [PATCH v3 00/17] Remove some cleanups using scope_exit Tom Tromey
16 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2019-01-23 15:28 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess
From: Tom Tromey <tom@tromey.com>
This removes make_bpstat_clear_actions_cleanup, replacing it with uses
of scope_exit.
gdb/ChangeLog:
yyyy-mm-dd Tom Tromey <tom@tromey.com>
Andrew Burgess <andrew.burgess@embecosm.com>
Pedro Alves <palves@redhat.com>
* infrun.c (fetch_inferior_event): Use scope_exit.
* utils.h (make_bpstat_clear_actions_cleanup): Don't declare.
* top.c (execute_command): Use scope_exit.
* breakpoint.c (bpstat_do_actions): Use scope_exit.
* utils.c (do_bpstat_clear_actions_cleanup)
(make_bpstat_clear_actions_cleanup): Remove.
---
gdb/breakpoint.c | 4 ++--
gdb/infrun.c | 6 +++---
gdb/top.c | 8 ++++----
gdb/utils.c | 17 -----------------
gdb/utils.h | 1 -
5 files changed, 9 insertions(+), 27 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 33c5bfef43..f9dd194487 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4468,7 +4468,7 @@ get_bpstat_thread ()
void
bpstat_do_actions (void)
{
- struct cleanup *cleanup_if_error = make_bpstat_clear_actions_cleanup ();
+ auto cleanup_if_error = make_scope_exit (bpstat_clear_actions);
thread_info *tp;
/* Do any commands attached to breakpoint we are stopped at. */
@@ -4482,7 +4482,7 @@ bpstat_do_actions (void)
break;
}
- discard_cleanups (cleanup_if_error);
+ cleanup_if_error.release ();
}
/* Print out the (old or new) value associated with a watchpoint. */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 065fbf06d8..76dedd272a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3745,8 +3745,8 @@ fetch_inferior_event (void *client_data)
/* Get executed before make_cleanup_restore_current_thread above to apply
still for the thread which has thrown the exception. */
- struct cleanup *ts_old_chain = make_bpstat_clear_actions_cleanup ();
-
+ auto defer_bpstat_clear
+ = make_scope_exit (bpstat_clear_actions);
auto defer_delete_threads
= make_scope_exit (delete_just_stopped_threads_infrun_breakpoints);
@@ -3802,7 +3802,7 @@ fetch_inferior_event (void *client_data)
}
defer_delete_threads.release ();
- discard_cleanups (ts_old_chain);
+ defer_bpstat_clear.release ();
/* No error, don't finish the thread states yet. */
finish_state.release ();
diff --git a/gdb/top.c b/gdb/top.c
index 900e78aaec..cf6a6abc7d 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -52,6 +52,7 @@
#include "frame.h"
#include "buffer.h"
#include "gdb_select.h"
+#include "common/scope-exit.h"
/* readline include files. */
#include "readline/readline.h"
@@ -539,12 +540,11 @@ set_repeat_arguments (const char *args)
void
execute_command (const char *p, int from_tty)
{
- struct cleanup *cleanup_if_error;
struct cmd_list_element *c;
const char *line;
const char *cmd_start = p;
- cleanup_if_error = make_bpstat_clear_actions_cleanup ();
+ auto cleanup_if_error = make_scope_exit (bpstat_clear_actions);
scoped_value_mark cleanup = prepare_execute_command ();
/* Force cleanup of any alloca areas if using C alloca instead of
@@ -554,7 +554,7 @@ execute_command (const char *p, int from_tty)
/* This can happen when command_line_input hits end of file. */
if (p == NULL)
{
- discard_cleanups (cleanup_if_error);
+ cleanup_if_error.release ();
return;
}
@@ -649,7 +649,7 @@ execute_command (const char *p, int from_tty)
if (has_stack_frames () && inferior_thread ()->state != THREAD_RUNNING)
check_frame_language_change ();
- discard_cleanups (cleanup_if_error);
+ cleanup_if_error.release ();
}
/* Run execute_command for P and FROM_TTY. Capture its output into the
diff --git a/gdb/utils.c b/gdb/utils.c
index ed8d60fa7b..4af75e3480 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3057,23 +3057,6 @@ parse_pid_to_attach (const char *args)
return pid;
}
-/* Helper for make_bpstat_clear_actions_cleanup. */
-
-static void
-do_bpstat_clear_actions_cleanup (void *unused)
-{
- bpstat_clear_actions ();
-}
-
-/* Call bpstat_clear_actions for the case an exception is throw. You should
- discard_cleanups if no exception is caught. */
-
-struct cleanup *
-make_bpstat_clear_actions_cleanup (void)
-{
- return make_cleanup (do_bpstat_clear_actions_cleanup, NULL);
-}
-
/* Substitute all occurences of string FROM by string TO in *STRINGP. *STRINGP
must come from xrealloc-compatible allocator and it may be updated. FROM
needs to be delimited by IS_DIR_SEPARATOR or DIRNAME_SEPARATOR (or be
diff --git a/gdb/utils.h b/gdb/utils.h
index f2fe1da832..896feb973c 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -286,7 +286,6 @@ private:
int m_save_batch_flag;
};
-extern struct cleanup *make_bpstat_clear_actions_cleanup (void);
\f
/* Path utilities. */
--
2.14.4
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v3 00/17] Remove some cleanups using scope_exit
2019-01-23 15:21 [PATCH v3 00/17] Remove some cleanups using scope_exit Pedro Alves
` (15 preceding siblings ...)
2019-01-23 15:28 ` [PATCH v3 09/17] Remove make_bpstat_clear_actions_cleanup Pedro Alves
@ 2019-01-23 17:33 ` Tom Tromey
2019-01-23 19:18 ` Pedro Alves
16 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2019-01-23 17:33 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches, Tom Tromey, Andrew Burgess
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> Here's the new version of Tromey's "cleanup function" series, updated
Pedro> to use the new scope_exit instead.
Pedro> v1 was here:
Pedro> https://sourceware.org/ml/gdb-patches/2019-01/msg00159.html
Pedro> I'm calling Andrew's patches the "v2":
Pedro> https://sourceware.org/ml/gdb-patches/2019-01/msg00273.html
Pedro> The discussion around "v2" led to this version.
Thanks for doing this. I read through the patches again and everything
looks good to me. There was one tiny nit in a commit message.
Tom
^ permalink raw reply [flat|nested] 20+ messages in thread