This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Make mi_cmd_break_insert exception-safe.
On Friday 01 February 2008 17:24:39 Daniel Jacobowitz wrote:
> On Fri, Feb 01, 2008 at 09:53:46AM +0300, Vladimir Prus wrote:
> > So, why bother trying to make gdb_breakpoint non-throwing? I believe
> > any such change will be at least as complex as making mi_cmd_break_insert
> > exception-safe? If your concern is about gdb_ prefix, how about renaming
> > gdb_exception into 'set_breakpoint'?
>
> That's fine too.
What about the following, then?
- Volodya
From d2a24e0c16437cbad9a8ad79ab22899b3749f5ca Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Sun, 27 Jan 2008 17:06:05 +0300
Subject: [RFA] Make mi_cmd_break_insert exception-safe.
To: gdb-patches@sources.redhat.com
X-KMail-Transport: CodeSourcery
X-KMail-Identity: 901867920
[gdb]
* breakpoint.c (break_command_1): Return void.
(break_command_really): Return void. Rethrow
exceptions instead of returning.
(gdb_breakpoint): Remove the error_message parameter.
Return void. Rename to set_breakpoint.
* gdb.h (gdb_breakpoint): Rename and move to...
* breakpoint.h (set_breakpoint): ...here.
* mi/mi-cmb-break.c (mi_cmd_break_insert): Restore
event hooks even if exception is thrown. Adjust to
gdb_breakpoint interface changes.
[gdb/testsuite]
* gdb.mi/basic.c (return_1): New function.
* gdb.mi/mi-break.exp: Make sure that failed -break-insert
don't cause future evaluations of function to report
creation of internal breakpoints.
---
gdb/breakpoint.c | 51 +++++++++++++----------------
gdb/breakpoint.h | 5 +++
gdb/gdb.h | 7 ----
gdb/mi/mi-cmd-break.c | 62 +++++++++++++++++++------------------
gdb/testsuite/gdb.mi/basics.c | 5 +++
gdb/testsuite/gdb.mi/mi-break.exp | 12 +++++++
6 files changed, 77 insertions(+), 65 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f69002c..c2bf50a 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -90,7 +90,7 @@ static void watch_command (char *, int);
static int can_use_hardware_watchpoint (struct value *);
-static int break_command_1 (char *, int, int);
+static void break_command_1 (char *, int, int);
static void mention (struct breakpoint *);
@@ -5285,7 +5285,7 @@ find_condition_and_thread (char *tok, CORE_ADDR pc,
and thread specified by the COND_STRING and THREAD
parameters. */
-static int
+static void
break_command_really (char *arg, char *cond_string, int thread,
int parse_condition_and_thread,
int tempflag, int hardwareflag,
@@ -5323,8 +5323,7 @@ break_command_really (char *arg, char *cond_string, int thread,
switch (e.reason)
{
case RETURN_QUIT:
- exception_print (gdb_stderr, e);
- return e.reason;
+ throw_exception (e);
case RETURN_ERROR:
switch (e.error)
{
@@ -5342,7 +5341,7 @@ break_command_really (char *arg, char *cond_string, int thread,
selects no, then simply return the error code. */
if (pending_break_support == AUTO_BOOLEAN_AUTO &&
!nquery ("Make breakpoint pending on future shared library load? "))
- return e.reason;
+ return;
/* At this point, either the user was queried about setting
a pending breakpoint and selected yes, or pending
@@ -5356,12 +5355,11 @@ break_command_really (char *arg, char *cond_string, int thread,
pending = 1;
break;
default:
- exception_print (gdb_stderr, e);
- return e.reason;
+ throw_exception (e);
}
default:
if (!sals.nelts)
- return GDB_RC_FAIL;
+ return;
}
/* Create a chain of things that always need to be cleaned up. */
@@ -5457,8 +5455,6 @@ break_command_really (char *arg, char *cond_string, int thread,
discard_cleanups (breakpoint_chain);
/* But cleanup everything else. */
do_cleanups (old_chain);
-
- return GDB_RC_OK;
}
/* Set a breakpoint.
@@ -5468,34 +5464,33 @@ break_command_really (char *arg, char *cond_string, int thread,
and if breakpoint is temporary, using BP_HARDWARE_FLAG
and BP_TEMPFLAG. */
-static int
+static void
break_command_1 (char *arg, int flag, int from_tty)
{
int hardwareflag = flag & BP_HARDWAREFLAG;
int tempflag = flag & BP_TEMPFLAG;
- return break_command_really (arg,
- NULL, 0, 1 /* parse arg */,
- tempflag, hardwareflag,
- 0 /* Ignore count */,
- pending_break_support, from_tty);
+ break_command_really (arg,
+ NULL, 0, 1 /* parse arg */,
+ tempflag, hardwareflag,
+ 0 /* Ignore count */,
+ pending_break_support, from_tty);
}
-enum gdb_rc
-gdb_breakpoint (char *address, char *condition,
+void
+set_breakpoint (char *address, char *condition,
int hardwareflag, int tempflag,
int thread, int ignore_count,
- int pending,
- char **error_message)
-{
- return break_command_really (address, condition, thread,
- 0 /* condition and thread are valid. */,
- tempflag, hardwareflag,
- ignore_count,
- pending
- ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
- 0);
+ int pending)
+{
+ break_command_really (address, condition, thread,
+ 0 /* condition and thread are valid. */,
+ tempflag, hardwareflag,
+ ignore_count,
+ pending
+ ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
+ 0);
}
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 7b72e19..41730c0 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -710,6 +710,11 @@ extern void awatch_command_wrapper (char *, int);
extern void rwatch_command_wrapper (char *, int);
extern void tbreak_command (char *, int);
+extern void set_breakpoint (char *address, char *condition,
+ int hardwareflag, int tempflag,
+ int thread, int ignore_count,
+ int pending);
+
extern void insert_breakpoints (void);
extern int remove_breakpoints (void);
diff --git a/gdb/gdb.h b/gdb/gdb.h
index c7c405c..fcd3e3b 100644
--- a/gdb/gdb.h
+++ b/gdb/gdb.h
@@ -47,13 +47,6 @@ enum gdb_rc {
enum gdb_rc gdb_breakpoint_query (struct ui_out *uiout, int bnum,
char **error_message);
-/* Create a breakpoint at ADDRESS (a GDB source and line). */
-enum gdb_rc gdb_breakpoint (char *address, char *condition,
- int hardwareflag, int tempflag,
- int thread, int ignore_count,
- int pending,
- char **error_message);
-
/* Switch thread and print notification. */
enum gdb_rc gdb_thread_select (struct ui_out *uiout, char *tidstr,
char **error_message);
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index c77b0dc..3a5faf3 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -26,6 +26,7 @@
#include "mi-getopt.h"
#include "gdb-events.h"
#include "gdb.h"
+#include "exceptions.h"
enum
{
@@ -69,7 +70,7 @@ mi_cmd_break_insert (char *command, char **argv, int argc)
int ignore_count = 0;
char *condition = NULL;
int pending = 0;
- enum gdb_rc rc;
+ struct gdb_exception e;
struct gdb_events *old_hooks;
enum opt
{
@@ -132,41 +133,42 @@ mi_cmd_break_insert (char *command, char **argv, int argc)
/* Now we have what we need, let's insert the breakpoint! */
old_hooks = deprecated_set_gdb_event_hooks (&breakpoint_hooks);
- switch (type)
+ /* Make sure we restore hooks even if exception is thrown. */
+ TRY_CATCH (e, RETURN_MASK_ALL)
{
- case REG_BP:
- rc = gdb_breakpoint (address, condition,
- 0 /*hardwareflag */ , temp_p,
- thread, ignore_count,
- pending,
- &mi_error_message);
- break;
- case HW_BP:
- rc = gdb_breakpoint (address, condition,
- 1 /*hardwareflag */ , temp_p,
- thread, ignore_count,
- pending,
- &mi_error_message);
- break;
+ switch (type)
+ {
+ case REG_BP:
+ set_breakpoint (address, condition,
+ 0 /*hardwareflag */ , temp_p,
+ thread, ignore_count,
+ pending);
+ break;
+ case HW_BP:
+ set_breakpoint (address, condition,
+ 1 /*hardwareflag */ , temp_p,
+ thread, ignore_count,
+ pending);
+ break;
#if 0
- case REGEXP_BP:
- if (temp_p)
- error (_("mi_cmd_break_insert: Unsupported tempoary regexp breakpoint"));
- else
- rbreak_command_wrapper (address, FROM_TTY);
- return MI_CMD_DONE;
- break;
+ case REGEXP_BP:
+ if (temp_p)
+ error (_("mi_cmd_break_insert: Unsupported tempoary regexp breakpoint"));
+ else
+ rbreak_command_wrapper (address, FROM_TTY);
+ return MI_CMD_DONE;
+ break;
#endif
- default:
- internal_error (__FILE__, __LINE__,
- _("mi_cmd_break_insert: Bad switch."));
+ default:
+ internal_error (__FILE__, __LINE__,
+ _("mi_cmd_break_insert: Bad switch."));
+ }
}
deprecated_set_gdb_event_hooks (old_hooks);
+ if (e.reason < 0)
+ throw_exception (e);
- if (rc == GDB_RC_FAIL)
- return MI_CMD_ERROR;
- else
- return MI_CMD_DONE;
+ return MI_CMD_DONE;
}
enum wp_type
diff --git a/gdb/testsuite/gdb.mi/basics.c b/gdb/testsuite/gdb.mi/basics.c
index 5b52112..e4be7b2 100644
--- a/gdb/testsuite/gdb.mi/basics.c
+++ b/gdb/testsuite/gdb.mi/basics.c
@@ -51,6 +51,11 @@ void callme (int i)
printf ("callme\n");
}
+int return_1 ()
+{
+ return 1;
+}
+
main ()
{
callee1 (2, "A string argument.", 3.5);
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index 6933a34..9f10364 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -181,6 +181,18 @@ proc test_error {} {
mi_gdb_test "-break-insert function_that_does_not_exist" \
".*\\^error,msg=\"Function \\\\\"function_that_does_not_exist\\\\\" not defined.\"" \
"breakpoint at nonexistent function"
+
+ # We used to have a bug whereby -break-insert that failed would not
+ # clear some event hooks. As result, whenever we evaluate expression
+ # containing function call, the internal breakpoint created to handle
+ # function call would be reported, messing up MI output.
+ mi_gdb_test "-var-create V * return_1()" \
+ "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\"" \
+ "create varobj for function call"
+
+ mi_gdb_test "-var-update *" \
+ "\\^done,changelist=\\\[\\\]" \
+ "update varobj for function call"
}
test_tbreak_creation_and_listing
--
1.5.3.5