RE: [PATCHv3 03/13] gdb: include breakpoint number in testing condition error message

["Aktemur, Tankut Baris via Gdb-patches" <gdb-patches@sourceware.org>]

On Tuesday, January 31, 2023 6:27 PM, Andrew Burgess wrote:
> When GDB fails to test the condition of a conditional breakpoint, for
> whatever reason, the error message looks like this:
> 
>   (gdb) break foo if (*(int *) 0) == 1
>   Breakpoint 1 at 0x40111e: file bpcond.c, line 11.
>   (gdb) r
>   Starting program: /tmp/bpcond
>   Error in testing breakpoint condition:
>   Cannot access memory at address 0x0
> 
>   Breakpoint 1, foo () at bpcond.c:11
>   11	  int a = 32;
>   (gdb)
> 
> The line I'm interested in for this commit is this one:
> 
>   Error in testing breakpoint condition:
> 
> In the case above we can figure out that the problematic breakpoint
> was #1 because in the final line of the message GDB reports the stop a
> breakpoint #1.
> 
> However, in the next few patches I plan to change this.  In some cases
> I don't think it makes sense for GDB to report the stop as being at
> breakpoint #1, consider this case:
> 
>   (gdb) list some_func
>   1	int
>   2	some_func ()
>   3	{
>   4	  int *p = 0;
>   5	  return *p;
>   6	}
>   7
>   8	void
>   9	foo ()
>   10	{
>   (gdb) break foo if (some_func ())
>   Breakpoint 1 at 0x40111e: file bpcond.c, line 11.
>   (gdb) r
>   Starting program: /tmp/bpcond
> 
>   Program received signal SIGSEGV, Segmentation fault.
>   0x0000000000401116 in some_func () at bpcond.c:5
>   5	  return *p;
>   Error in testing breakpoint condition:
>   The program being debugged was signaled while in a function called from GDB.
>   GDB remains in the frame where the signal was received.
>   To change this behavior use "set unwindonsignal on".
>   Evaluation of the expression containing the function
>   (some_func) will be abandoned.
>   When the function is done executing, GDB will silently stop.
> 
>   Program received signal SIGSEGV, Segmentation fault.
> 
>   Breakpoint 1, 0x0000000000401116 in some_func () at bpcond.c:5
>   5	  return *p;
>   (gdb)
> 
> Notice that, the final lines of output report the stop as being at
> breakpoint #1, even though we are actually located within some_func.
> 
> I find this behaviour confusing, and propose that this should be
> changed.  However, if I make that change then every reference to
> breakpoint #1 will be lost from the error message.
> 
> So, in this commit, in preparation for the later commits, I propose to
> change the 'Error in testing breakpoint condition:' line to this:
> 
>   Error in testing condition for breakpoint NUMBER:
> 
> where NUMBER will be filled in as appropriate.  Here's the first
> example with the updated error:
> 
>   (gdb) break foo if (*(int *) 0) == 0
>   Breakpoint 1 at 0x40111e: file bpcond.c, line 11.
>   (gdb) r
>   Starting program: /tmp/bpcond
>   Error in testing condition for breakpoint 1:
>   Cannot access memory at address 0x0
> 
>   Breakpoint 1, foo () at bpcond.c:11
>   11	  int a = 32;
>   (gdb)
> 
> The breakpoint number does now appear twice in the output, but I don't
> see that as a negative.
> 
> This commit just changes the one line of the error, and updates the
> few tests that either included the old error in comments, or actually
> checked for the error in the expected output.
> 
> As the only test that checked the line I modified is a Python test,
> I've added a new test that doesn't rely on Python that checks the
> error message in detail.
> 
> While working on the new test, I spotted that it would fail when run
> with native-gdbserver and native-extended-gdbserver target boards.
> This turns out to be due to a gdbserver bug.  To avoid cluttering this
> commit I've added a work around to the new test script so that the
> test passes for the remote boards, in the next few commits I will fix
> gdbserver, and update the test script to remove the work around.
> ---
>  gdb/breakpoint.c                              |   3 +-
>  gdb/testsuite/gdb.base/bp-cond-failure.c      |  30 +++++
>  gdb/testsuite/gdb.base/bp-cond-failure.exp    | 114 ++++++++++++++++++
>  .../gdb.base/catch-signal-siginfo-cond.exp    |   2 +-
>  gdb/testsuite/gdb.base/gnu-ifunc.exp          |   2 +-
>  .../gdb.python/py-finish-breakpoint.exp       |   2 +-
>  6 files changed, 149 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/bp-cond-failure.c
>  create mode 100644 gdb/testsuite/gdb.base/bp-cond-failure.exp
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 00cc2ab401c..eecaeefed3e 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -5542,7 +5542,8 @@ bpstat_check_breakpoint_conditions (bpstat *bs, thread_info *thread)
>  	  catch (const gdb_exception &ex)
>  	    {
>  	      exception_fprintf (gdb_stderr, ex,
> -				 "Error in testing breakpoint condition:\n");
> +				 "Error in testing condition for breakpoint %d:\n",
> +				 b->number);
>  	    }
>  	}
>        else
> diff --git a/gdb/testsuite/gdb.base/bp-cond-failure.c b/gdb/testsuite/gdb.base/bp-cond-
> failure.c
> new file mode 100644
> index 00000000000..2a9974b47ce
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/bp-cond-failure.c
> @@ -0,0 +1,30 @@
> +/* Copyright 2022-2023 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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/>.  */
> +
> +int
> +foo ()
> +{
> +  return 0;	/* Breakpoint here.  */
> +}
> +
> +int
> +main ()
> +{
> +  int res = foo ();
> +
> +  return res;
> +}
> diff --git a/gdb/testsuite/gdb.base/bp-cond-failure.exp b/gdb/testsuite/gdb.base/bp-cond-
> failure.exp
> new file mode 100644
> index 00000000000..9388b8cf582
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/bp-cond-failure.exp
> @@ -0,0 +1,114 @@
> +# Copyright 2022-2023 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/>.
> +
> +# Check the format of the error message given when a breakpoint
> +# condition fails.
> +#
> +# In this case the breakpoint condition does not make use of inferior
> +# function calls, instead, the expression used for the breakpoint
> +# condition will throw an error when evaluated.
> +#
> +# We check that the correct breakpoint number appears in the error
> +# message, and that the error is reported at the correct source
> +# location.
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" ${binfile} "${srcfile}" \
> +	  {debug}] == -1 } {
> +    return
> +}
> +
> +# Run to main so that we connect to the target if using 'target
> +# remote'.  This means that the is_address_zero_readable, and the
> +# 'show breakpoint condition-evaluation' checks below will be
> +# performed with the remote connection in place.
> +if { ![runto_main] } {
> +    fail "run to main"

This fail can be removed.

> +    return -1
> +}
> +
> +# This test relies on reading address zero triggering a SIGSEGV.
> +if { [is_address_zero_readable] } {
> +    return
> +}
> +
> +# Where the breakpoint will be placed.
> +set bp_line [gdb_get_line_number "Breakpoint here"]
> +
> +proc run_test { cond_eval } {
> +    clean_restart ${::binfile}
> +
> +    if { ![runto_main] } {
> +	fail "run to main"

This one, too.

Thanks
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928