Re: [RFA] inadvertent language switch during breakpoint_re_set_one

[Joel Brobecker <brobecker at adacore dot com>]

> gdb/ChangeLog:
> 
>         * breakpoint.c (breakpoint_re_set): Temporarily force language_mode
>         to language_mode_manual while calling breakpoint_re_set_one.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.ada/bp_fun_addr: New testcase.
> 
> Tested on x86_64-linux.

Friendly 2 weeks ping on this patch :).

Thanks!

> >From 90bb7d41b87c259d09faddae174887defd87cbc9 Mon Sep 17 00:00:00 2001
> From: Joel Brobecker <brobecker@adacore.com>
> Date: Thu, 10 May 2018 13:17:52 -0500
> Subject: [PATCH] inadvertent language switch during breakpoint_re_set_one
> 
> Trying to insert a breakpoint using *FUNC'address with an Ada program
> and then running the program until reaching that breakpoint currently
> yields the following behavior:
> 
>     (gdb) break *a'address
>     Breakpoint 1 at 0x40240c: file a.adb, line 1.
>     (gdb) run
>     [1]  + 27222 suspended (tty output) /[...]/gdb -q simple_main
> 
> Unsuspending GDB then shows it was suspended trying to report
> the following error:
> 
>     Starting program: /home/takamaka.a/brobecke/ex/simple/a
>     Error in re-setting breakpoint 1: Unmatched single quote.
>     Error in re-setting breakpoint 1: Unmatched single quote.
>     Error in re-setting breakpoint 1: Unmatched single quote.
>     [Inferior 1 (process 32470) exited normally]
> 
> The "a'address" is Ada speak for function A's address ("A" by
> itself means the result of calling A with no arguments). The
> transcript above shows that we're having problems trying to
> parse the breakpoint location while re-setting it.  As a result,
> we also fail to stop at the breakpoint.
> 
> Normally, breakpoint locations are evaluated with the current_language
> being set to the language of the breakpoint. But, unfortunately for us,
> what happened in this case is that parse_exp_in_context_1 calls
> get_selected_block which eventually leads to a call to select_frame
> because the current_frame hadn't been set yet.  select_frame then
> finds that our language_mode is auto, and therefore changes the
> current_language to match the language of the frame we just selected.
> In our case, the language chosen was 'c', which of course is not
> able to parse an Ada expression, hence the error.
> 
> This patch prevents this by forcing the language_mode to manual
> before we call breakpoint_re_set_one.
> 
> gdb/ChangeLog:
> 
>         * breakpoint.c (breakpoint_re_set): Temporarily force language_mode
>         to language_mode_manual while calling breakpoint_re_set_one.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.ada/bp_fun_addr: New testcase.
> 
> Tested on x86_64-linux.
> ---
>  gdb/breakpoint.c                        | 13 ++++++++++++
>  gdb/testsuite/gdb.ada/bp_fun_addr.exp   | 35 +++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.ada/bp_fun_addr/a.adb | 19 ++++++++++++++++++
>  3 files changed, 67 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.ada/bp_fun_addr.exp
>  create mode 100644 gdb/testsuite/gdb.ada/bp_fun_addr/a.adb
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index d1955ec..6dfa2d9 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -13881,6 +13881,19 @@ breakpoint_re_set (void)
>      scoped_restore save_input_radix = make_scoped_restore (&input_radix);
>      scoped_restore_current_pspace_and_thread restore_pspace_thread;
>  
> +    /* breakpoint_re_set_one sets the current_language to the language
> +       of the breakpoint it is resetting (see prepare_re_set_context)
> +       before re-evaluating the breakpoint's location.  This change can
> +       unfortunately get undone by accident if the language_mode is set
> +       to auto, and we either switch frames, or more likely in this context,
> +       we select the current frame.
> +
> +       We prevent this by temporarily turning the language_mode to
> +       language_mode_manual.  We we restore it once all breakpoints
> +       have been reset.  */
> +    scoped_restore save_language_mode = make_scoped_restore (&language_mode);
> +    language_mode = language_mode_manual;
> +
>      /* Note: we must not try to insert locations until after all
>         breakpoints have been re-set.  Otherwise, e.g., when re-setting
>         breakpoint 1, we'd insert the locations of breakpoint 2, which
> diff --git a/gdb/testsuite/gdb.ada/bp_fun_addr.exp b/gdb/testsuite/gdb.ada/bp_fun_addr.exp
> new file mode 100644
> index 0000000..aa1261b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/bp_fun_addr.exp
> @@ -0,0 +1,35 @@
> +# Copyright 2016-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"
> +
> +if { [skip_ada_tests] } { return -1 }
> +
> +standard_ada_testfile a
> +
> +if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
> +    return -1
> +}
> +
> +clean_restart ${testfile}
> +
> +gdb_test "break *a'address" \
> +         "Breakpoint \[0-9\]+ at.*: file .*a.adb, line \[0-9\]+."
> +
> +gdb_run_cmd
> +gdb_test "" \
> +         "Breakpoint $decimal, a \\(\\).*" \
> +         "Run until breakpoint at a'address"
> +
> diff --git a/gdb/testsuite/gdb.ada/bp_fun_addr/a.adb b/gdb/testsuite/gdb.ada/bp_fun_addr/a.adb
> new file mode 100644
> index 0000000..00e2e86
> --- /dev/null
> +++ b/gdb/testsuite/gdb.ada/bp_fun_addr/a.adb
> @@ -0,0 +1,19 @@
> +--  Copyright 2016-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 A is
> +begin
> +   null;
> +end A;
> -- 
> 2.1.4
> 


-- 
Joel