This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 5/6] Change Ada exceptions to use std::string
[really attached, this time!]
> > This changes the Ada exception code to use std::string, allowing the
> > removal of some more cleanups.
> >
> > ChangeLog
> > 2017-11-29 Tom Tromey <tom@tromey.com>
> >
> > * mi/mi-cmd-catch.c (mi_cmd_catch_assert)
> > (mi_cmd_catch_exception): Update.
> > * ada-lang.h (create_ada_exception_catchpoint): Update.
> > * ada-lang.c (struct ada_catchpoint) <excep_string>: Now a
> > std::string.
> > (create_excep_cond_exprs, ~ada_catchpoint)
> > (should_stop_exception, print_one_exception)
> > (print_mention_exception, print_recreate_exception): Update.
> > (ada_get_next_arg): Remove.
> > (catch_ada_exception_command_split): Change two arguments to
> > "std::string *". Remove cleanups.
> > (ada_exception_catchpoint_cond_string): Change "string" to
> > std::string.
> > (ada_exception_sal): Remove excep_string parameter.
> > (create_ada_exception_catchpoint): Change two arguments to
> > "std::string &&".
> > (catch_ada_exception_command): Update.
>
> As hinted in my answer to the cover email, I found a few issues.
> Attached is a commit that explains and fixes them. If the fixes
> look good to you, a new commit combining the two is preapproved.
>
> (I tested my patch on x86_64-linux, limiting myself to gdb.ada)
--
Joel
>From c2ad16ea37be009bf58afb7467f2bdd67b31710a Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Thu, 30 Nov 2017 16:15:09 -0500
Subject: [PATCH] Fix a couple of regressions introduced by the previous patch
Pb #1: Crash while trying to insert an assert catchpoint:
With any program built with "gnatmake -g -gnata ...", trying
to insert a catchpoint on failed assertions would cause an internal
error:
(gdb) catch assert
/[...]/common/cleanups.c:264: internal-warning: restore_my_cleanups has found a stale cleanup
I tracked this down to an issue in the call to
create_ada_exception_catchpoint, where we pass NULL for at least
one of the parameters declared as a "std::string &&". I don't know
C++ well enough, but instruction-by-instruction leads me to believe
that the constructor doesn't like NULL as a char *, so we get an
exception, and that exception eventually somehow leads to the
cleanup error above (not sure how, unfortunately). The fix was
to pass an std::string() instead.
And by the time I understood this for the cond string parameter
(on which I had target fixation), I had C++-ifed
catch_ada_assert_command_split.
Pb #2: Same kind of crash insertin an exception in GDB/MI mode.
In this case, it was just a case of getting confused between
a couple of temporary "char *" variables, and the corresponding
std::string parameters that Tom introduced.
I first fixed the confusion, but then I wondered why using
intermediate "char *" variables at all. So I changed this function
to only use std::string variables.
Pb #3: condition handling in exception catchpoints is broken
(gdb) catch exception constraint_error if True
Junk at end of expression
Tom removed a call to "skip_spaces" before checking for the next
argument, and I am not sure I understand why. But debugging
the problem showed by arg was equal to...
"constraint_error if True"
... and that once extract_arg is called, args is now equal to...
" if True"
... so the condition identifying an "if" block no longer works.
I restored the call to skip_spaces.
---
gdb/ada-lang.c | 9 +++++----
gdb/mi/mi-cmd-catch.c | 12 +++---------
2 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 5e9aebf..1caa3dc 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12744,6 +12744,7 @@ catch_ada_exception_command_split (const char *args,
/* Check to see if we have a condition. */
+ args = skip_spaces (args);
if (startswith (args, "if")
&& (isspace (args[2]) || args[2] == '\0'))
{
@@ -12991,7 +12992,7 @@ catch_ada_exception_command (const char *arg_entry, int from_tty,
(the memory needs to be deallocated after use). */
static void
-catch_ada_assert_command_split (const char *args, char **cond_string)
+catch_ada_assert_command_split (const char *args, std::string *cond_string)
{
args = skip_spaces (args);
@@ -13003,7 +13004,7 @@ catch_ada_assert_command_split (const char *args, char **cond_string)
args = skip_spaces (args);
if (args[0] == '\0')
error (_("condition missing after `if' keyword"));
- *cond_string = xstrdup (args);
+ *cond_string = args;
}
/* Otherwise, there should be no other argument at the end of
@@ -13021,7 +13022,7 @@ catch_assert_command (const char *arg_entry, int from_tty,
const char *arg = arg_entry;
struct gdbarch *gdbarch = get_current_arch ();
int tempflag;
- char *cond_string = NULL;
+ std::string cond_string;
tempflag = get_cmd_context (command) == CATCH_TEMPORARY;
@@ -13029,7 +13030,7 @@ catch_assert_command (const char *arg_entry, int from_tty,
arg = "";
catch_ada_assert_command_split (arg, &cond_string);
create_ada_exception_catchpoint (gdbarch, ada_catch_assert,
- NULL, cond_string,
+ std::string (), std::move (cond_string),
tempflag, 1 /* enabled */,
from_tty);
}
diff --git a/gdb/mi/mi-cmd-catch.c b/gdb/mi/mi-cmd-catch.c
index f69cdaa..70939ee 100644
--- a/gdb/mi/mi-cmd-catch.c
+++ b/gdb/mi/mi-cmd-catch.c
@@ -93,9 +93,9 @@ void
mi_cmd_catch_exception (const char *cmd, char *argv[], int argc)
{
struct gdbarch *gdbarch = get_current_arch();
- char *condition = NULL;
+ std::string condition;
int enabled = 1;
- char *exception_name = NULL;
+ std::string exception_name;
int temp = 0;
enum ada_exception_catchpoint_kind ex_kind = ada_catch_exception;
@@ -152,16 +152,10 @@ mi_cmd_catch_exception (const char *cmd, char *argv[], int argc)
/* Specifying an exception name does not make sense when requesting
an unhandled exception breakpoint. */
- if (ex_kind == ada_catch_exception_unhandled && exception_name != NULL)
+ if (ex_kind == ada_catch_exception_unhandled && ! exception_name.empty())
error (_("\"-e\" and \"-u\" are mutually exclusive"));
scoped_restore restore_breakpoint_reporting = setup_breakpoint_reporting ();
- std::string except_str;
- if (exception_name != NULL)
- except_str = exception_name;
- std::string cond_str;
- if (condition != NULL)
- cond_str = condition;
create_ada_exception_catchpoint (gdbarch, ex_kind,
std::move (exception_name),
std::move (condition),
--
2.1.4