From c2ad16ea37be009bf58afb7467f2bdd67b31710a Mon Sep 17 00:00:00 2001 From: Joel Brobecker 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