[RFA] (Ada) Add gdb-mi support for stopping at start of exception handler.

  Following my previous commit which add support for stopping at start of
exception handler, this commit adds required gdb-mi support for this
feature.

gdb/ChangeLog:

        * mi-cmd-catch.c (mi_cmd_catch_handlers): New function.
        * mi-cmds.c (struct mi_cmds): Add catch-handlers command.
        * mi-cmds.h (mi_cmd_catch_handlers): Add external declaration.

gdb/testsuite/ChangeLog:
        * gdb.ada/mi_catch_ex_hand.exp: New testcase.

Tested on x86_64-linux.
---
 gdb/mi/mi-cmd-catch.c                          |  73 +++++++++++++
 gdb/mi/mi-cmds.c                               |   2 +
 gdb/mi/mi-cmds.h                               |   1 +
 gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp     | 141 +++++++++++++++++++++++++
 gdb/testsuite/gdb.ada/mi_catch_ex_hand/foo.adb |  33 ++++++
 5 files changed, 250 insertions(+)
 create mode 100644 gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp
 create mode 100644 gdb/testsuite/gdb.ada/mi_catch_ex_hand/foo.adb
  

On 2018-01-03 06:09 AM, Xavier Roirand wrote:
> Following my previous commit which add support for stopping at start of
> exception handler, this commit adds required gdb-mi support for this
> feature.

Hi Xavier,

This patch would need to update the documentation, and probably add a NEWS
entry (I always forget those too).

I would like to try running the test, but I am not able to catch handlers with
my setup, I get the "Your Ada runtime appears to be missing some debugging information"
message.  I am on Ubuntu 16.04, do you know how to set up the environment on that
distro to get it working, is it just a matter of installing the right package?  Right
now I have the gnat-5 package installed, from the xenial-updates official repository.

I have a few comments below, nothing big.

> gdb/ChangeLog:
> 
>         * mi-cmd-catch.c (mi_cmd_catch_handlers): New function.
>         * mi-cmds.c (struct mi_cmds): Add catch-handlers command.
>         * mi-cmds.h (mi_cmd_catch_handlers): Add external declaration.
> 
> gdb/testsuite/ChangeLog:
>         * gdb.ada/mi_catch_ex_hand.exp: New testcase.

There's a new .adb file too, you should mention it.

> 
> Tested on x86_64-linux.
> ---
>  gdb/mi/mi-cmd-catch.c                          |  73 +++++++++++++
>  gdb/mi/mi-cmds.c                               |   2 +
>  gdb/mi/mi-cmds.h                               |   1 +
>  gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp     | 141 +++++++++++++++++++++++++
>  gdb/testsuite/gdb.ada/mi_catch_ex_hand/foo.adb |  33 ++++++
>  5 files changed, 250 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp
>  create mode 100644 gdb/testsuite/gdb.ada/mi_catch_ex_hand/foo.adb
> 
> diff --git a/gdb/mi/mi-cmd-catch.c b/gdb/mi/mi-cmd-catch.c
> index 9c103d2..964b1a8 100644
> --- a/gdb/mi/mi-cmd-catch.c
> +++ b/gdb/mi/mi-cmd-catch.c
> @@ -167,6 +167,79 @@ mi_cmd_catch_exception (const char *cmd, char *argv[], int argc)
>  				   temp, enabled, 0);
>  }
>  
> +/* Handler for the -catch-handlers command.  */
> +
> +void
> +mi_cmd_catch_handlers (const char *cmd, char *argv[], int argc)
> +{
> +  struct gdbarch *gdbarch = get_current_arch();

Space before ().

> +  char *condition = NULL;
> +  int enabled = 1;
> +  char *exception_name = NULL;
> +  int temp = 0;
> +  enum ada_exception_catchpoint_kind ex_kind = ada_catch_handlers;
> +
> +  int oind = 0;
> +  char *oarg;
> +
> +  enum opt
> +    {
> +      OPT_CONDITION, OPT_DISABLED, OPT_EXCEPTION_NAME, OPT_TEMP,
> +      OPT_UNHANDLED,

I think unhandled is unused in the context of this function.

> +    };
> +  static const struct mi_opt opts[] =
> +    {
> +      { "c", OPT_CONDITION, 1},
> +      { "d", OPT_DISABLED, 0 },
> +      { "e", OPT_EXCEPTION_NAME, 1 },
> +      { "t", OPT_TEMP, 0 },
> +      { "u", OPT_UNHANDLED, 0},> +      { 0, 0, 0 }
> +    };
> +
> +  for (;;)
> +    {
> +      int opt = mi_getopt ("-catch-handlers", argc, argv, opts,
> +			   &oind, &oarg);
> +
> +      if (opt < 0)
> +        break;
> +
> +      switch ((enum opt) opt)
> +        {
> +	case OPT_CONDITION:
> +	  condition = oarg;
> +	  break;
> +	case OPT_DISABLED:
> +	  enabled = 0;
> +	  break;
> +	case OPT_EXCEPTION_NAME:
> +	  exception_name = oarg;
> +	  break;
> +	case OPT_TEMP:
> +	  temp = 1;
> +	  break;
> +        }
> +    }
> +
> +  /* This command does not accept any argument.  Make sure the user
> +     did not provide any.  */
> +  if (oind != argc)
> +    error (_("Invalid argument: %s"), argv[oind]);
> +
> +  scoped_restore restore_breakpoint_reporting
> +    = setup_breakpoint_reporting ();
> +  /* create_ada_exception_catchpoint needs EXCEPTION_NAME and CONDITION
> +     to be xstrdup'ed, and will assume control of their lifetime.  */
> +  if (exception_name != NULL)
> +    exception_name = xstrdup (exception_name);
> +  if (condition != NULL)
> +    condition = xstrdup (condition);
> +  create_ada_exception_catchpoint (gdbarch, ex_kind,
> +				   exception_name, condition,
> +				   temp, enabled, 0);
> +}
> +
>  /* Common path for the -catch-load and -catch-unload.  */
>  
>  static void
> diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
> index 18f6653..310be51 100644
> --- a/gdb/mi/mi-cmds.c
> +++ b/gdb/mi/mi-cmds.c
> @@ -69,6 +69,8 @@ static struct mi_cmd mi_cmds[] =
>                     &mi_suppress_notification.breakpoint),
>    DEF_MI_CMD_MI_1 ("catch-exception", mi_cmd_catch_exception,
>                     &mi_suppress_notification.breakpoint),
> +  DEF_MI_CMD_MI_1 ("catch-handlers", mi_cmd_catch_handlers,
> +                   &mi_suppress_notification.breakpoint),
>    DEF_MI_CMD_MI_1 ("catch-load", mi_cmd_catch_load,
>                     &mi_suppress_notification.breakpoint),
>    DEF_MI_CMD_MI_1 ("catch-unload", mi_cmd_catch_unload,
> diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
> index 03a11c0..52785e9 100644
> --- a/gdb/mi/mi-cmds.h
> +++ b/gdb/mi/mi-cmds.h
> @@ -41,6 +41,7 @@ extern mi_cmd_argv_ftype mi_cmd_break_passcount;
>  extern mi_cmd_argv_ftype mi_cmd_break_watch;
>  extern mi_cmd_argv_ftype mi_cmd_catch_assert;
>  extern mi_cmd_argv_ftype mi_cmd_catch_exception;
> +extern mi_cmd_argv_ftype mi_cmd_catch_handlers;
>  extern mi_cmd_argv_ftype mi_cmd_catch_load;
>  extern mi_cmd_argv_ftype mi_cmd_catch_unload;
>  extern mi_cmd_argv_ftype mi_cmd_disassemble;
> diff --git a/gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp b/gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp
> new file mode 100644
> index 0000000..544e0d8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/mi_catch_ex_hand.exp
> @@ -0,0 +1,141 @@
> +# Copyright 2011-2018 Free Software Foundation, Inc.
> +#
> +# 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/>.
> +
> +load_lib "ada.exp"
> +
> +standard_ada_testfile foo
> +
> +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug additional_flags=-gnata ]] != "" } {
> +  return -1
> +}
> +
> +# Some global variables used to simplify the maintenance of some of
> +# the regular expressions below.
> +set any_nb "\[0-9\]+"

There is $decimal that is defined to the same thing as any_nb by Dejagnu.  I would suggest
using that instead (especially since you already use it in this test).

> +set eol "\[\r\n\]+"
> +
> +# Before going any further, verify that we can insert exception
> +# handlers catchpoints...  That way, we won't have to do this while
> +# doing the actual GDB/MI testing.
> +
> +clean_restart ${testfile}
> +
> +if ![runto_main] then {
> +   fail "cannot run to main, testcase aborted"
> +   return 0
> +}
> +
> +set msg "insert catchpoint on all Ada exceptions handlers"
> +gdb_test_multiple "catch handlers" $msg {
> +    -re "Catchpoint $any_nb: all Ada exceptions handlers$eol$gdb_prompt $" {
> +	pass $msg
> +    }
> +    -re "Your Ada runtime appears to be missing some debugging information.*\[\r\n\]+$gdb_prompt $" {
> +	# If the runtime was not built with enough debug information,
> +	# or if it was stripped, we can not test exception
> +	# catchpoints.
> +	unsupported $msg
> +	return -1
> +    }
> +}
> +
> +# Now, we can start the GDB/MI testing itself...
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +gdb_exit
> +if [mi_gdb_start] {
> +    continue
> +}
> +
> +mi_delete_breakpoints
> +mi_gdb_reinitialize_dir $srcdir/$subdir
> +mi_gdb_load ${binfile}

These last three actions are done in mi_run_to_main, so you can remove them.

> +
> +#############################################
> +# 1. Try catching all exceptions handlers.  #
> +#############################################
> +
> +if ![mi_run_to_main] then {
> +   fail "cannot run to main, testcase aborted"
> +   return 0
> +}
> +
> +mi_gdb_test "-catch-handlers" \
> +            "\\^done,bkptno=\"$decimal\",bkpt={.*disp=\"keep\",enabled=\"y\",addr=\"$hex\",what=\"all Ada exceptions handlers\",.*}" \
> +            "catch all exceptions handlers"
> +
> +# Continue to exception handler.
> +
> +proc continue_to_exception_handler { test line } {
> +    global hex any_nb
> +
> +    mi_send_resuming_command "exec-continue" "$test"
> +
> +    # Now MI stream output.
> +    mi_expect_stop \
> +	"breakpoint-hit\",disp=\"keep\",bkptno=\"$any_nb\",exception-name=\"exception\"?" \
> +	"foo" "" ".*" "$line" \
> +	".*" \
> +        $test
> +}
> +
> +# We don't have the exception name info when stopping at the exception handlers
> +# breakpoint so we use source line to check if the inferior stops at the
> +# right location.
> +
> +set bp_ce_location [gdb_get_line_number "BREAK1" ${testdir}/foo.adb]
> +continue_to_exception_handler \
> +    "continue until CE handling caught by all-exceptions handlers catchpoint" \
> +    "$bp_ce_location"
> +
> +set bp_pe_location [gdb_get_line_number "BREAK2" ${testdir}/foo.adb]
> +continue_to_exception_handler \
> +    "continue until PE handling caught by all-exceptions handlers catchpoint" \
> +    "$bp_pe_location"
> +
> +##########################################################
> +# 2. Try catching only some of the exceptions handlers.  #
> +##########################################################
> +
> +# Here is the scenario:
> +#  - Restart the debugger from scratch, runto_main
> +#  - We'll catch only "Constraint_Error handlers"
> +#  - continue, we should stop at the Constraint_Error exception handler
> +#  - continue, we should not stop at the Program_Error exception handler
> +#    but exits instead.

exits -> exit

> +
> +if ![mi_run_to_main] then {
> +   fail "cannot run to main, testcase aborted"
> +   return 0
> +}
> +
> +mi_gdb_test "-catch-handlers -e Constraint_Error" \
> +            "\\^done,bkptno=\"$decimal\",bkpt={.*disp=\"keep\",enabled=\"y\",addr=\"$hex\",what=\"`Constraint_Error' Ada exception handlers\",.*}" \
> +            "catch Constraint_Error"
> +
> +mi_execute_to "exec-continue" \
> +              "breakpoint-hit\",disp=\"keep\",bkptno=\"$any_nb\",exception-name=\"exception\"?" \
> +              "foo" "" ".*" "$bp_ce_location" \
> +              ".*" \
> +              "continue to exception catchpoint hit"
> +
> +# Exit the inferior.
> +mi_send_resuming_command "exec-continue" "continuing to inferior exit"
> +mi_expect_stop "exited-normally" "" "" "" "" "" "exit normally"
> +
> +mi_gdb_exit
> +return 0
> diff --git a/gdb/testsuite/gdb.ada/mi_catch_ex_hand/foo.adb b/gdb/testsuite/gdb.ada/mi_catch_ex_hand/foo.adb
> new file mode 100644
> index 0000000..da701a3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/mi_catch_ex_hand/foo.adb
> @@ -0,0 +1,33 @@
> +--  Copyright 2007-2018 Free Software Foundation, Inc.
> +--
> +--  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/>.
> +
> +procedure Foo is
> +begin
> +
> +   begin
> +      raise Constraint_Error;
> +   exception
> +      when Constraint_Error => -- BREAK1
> +         null;
> +   end;
> +
> +   begin
> +      raise Program_Error;
> +   exception
> +      when Program_Error => -- BREAK2
> +         null;
> +   end;
> +
> +end Foo;
> 

Thanks,

Simon
  

On 2018-01-03 06:09 AM, Xavier Roirand wrote:
> +  /* This command does not accept any argument.  Make sure the user
> +     did not provide any.  */
> +  if (oind != argc)
> +    error (_("Invalid argument: %s"), argv[oind]);
> +
> +  scoped_restore restore_breakpoint_reporting
> +    = setup_breakpoint_reporting ();
> +  /* create_ada_exception_catchpoint needs EXCEPTION_NAME and CONDITION
> +     to be xstrdup'ed, and will assume control of their lifetime.  */
> +  if (exception_name != NULL)
> +    exception_name = xstrdup (exception_name);
> +  if (condition != NULL)
> +    condition = xstrdup (condition);
> +  create_ada_exception_catchpoint (gdbarch, ex_kind,
> +				   exception_name, condition,
> +				   temp, enabled, 0);
> +}

create_ada_exception_catchpoint claims that it takes ownership of the condition
string and will deallocate it, but I don't think that's true.  It looks like a
memory leak to me.  Could you take a look?  Converting it to use std::string
or gdb::unique_xmalloc_ptr<char> instead would probably be the way to go eventually.

Simon
  

Hi Simon,

Thanks for the review!

> I would like to try running the test, but I am not able to catch
> handlers with my setup, I get the "Your Ada runtime appears to be
> missing some debugging information" message.  I am on Ubuntu 16.04, do
> you know how to set up the environment on that distro to get it
> working, is it just a matter of installing the right package?  Right
> now I have the gnat-5 package installed, from the xenial-updates
> official repository.

I think you need to add one of the "debug" packages. I know that
some GNU/Linux distribution providers have a policy of providing
everything stripped by default, with the option of installing
debug packages when needed. I recommend the debug package be installed
when debugging Ada, as GDB needs the debugging information on a small
fraction of them in order to implement some of the functionality.
Exception handling is one of them.

Users of GNAT built from sources do not suffer from this issue
(of the stripped runtime), because the default mode for building
ensures that the few units we need are indeed built with debug info.