Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.

[Andrew Burgess <aburgess@redhat.com>]

Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org> writes:

> When stopped inside an inline function, trying to jump to a different line
> of the same function currently results in a warning about jumping to another
> function.  Fix this by taking inline functions into account.
>
> Before:
>   Breakpoint 1, function_inline (x=510) at jump-inline.cpp:22
>   22        a = a + x;             /* inline-funct */
>   (gdb) j 21
>   Line 21 is not in `function_inline(int)'.  Jump anyway? (y or n)
>
> After:
>   Breakpoint 2, function_inline (x=510) at jump-inline.cpp:22
>   22        a = a + x;            /* inline-funct */
>   (gdb) j 21
>   Continuing at 0x400679.
>
>   Breakpoint 1, function_inline (x=510) at jump-inline.cpp:21
>   21        a += 1020 + a;                /* increment-funct */
>
> This was regression-tested on X86-64 Linux.
>
> Co-Authored-by: Cristian Sandu <cristian.sandu@intel.com>
> ---
>  gdb/infcmd.c                           |  3 +-
>  gdb/testsuite/gdb.base/jump-inline.c   | 30 +++++++++++++++++
>  gdb/testsuite/gdb.base/jump-inline.exp | 45 ++++++++++++++++++++++++++
>  3 files changed, 77 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/testsuite/gdb.base/jump-inline.c
>  create mode 100644 gdb/testsuite/gdb.base/jump-inline.exp
>
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index fd88b8ca328..40414bc9260 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -1091,7 +1091,8 @@ jump_command (const char *arg, int from_tty)
>  
>    /* See if we are trying to jump to another function.  */
>    fn = get_frame_function (get_current_frame ());
> -  sfn = find_pc_function (sal.pc);
> +  sfn = find_pc_sect_containing_function (sal.pc,
> +					  find_pc_mapped_section (sal.pc));

I had a read through the discussion about whether find_pc_function
should return inline functions or not.  I don't know the history of this
code, so I don't know if there's a reason why it does what it does, but
looking at how it's used, I know there are some places in GDB where
find_pc_function is used when it shouldn't be.

For example in edit_command (cli-cmds.c) I'm pretty sure the use of
find_pc_function is incorrect -- GDB will print the name of the
containing function, but then open the editor on the inlined function.

As was said elsewhere, the biggest problem here will be lack of
testing.  What is really needed is an audit of each use of
find_pc_function and to ensure that each use is hit with an inline
function and a non-inline function.  Until then I think it's hard to be
certain about whether find_pc_function can safely be changed or not.
But doing that is no small task.

Given that, I'm inclined to think we should take the patch as
presented.  If anyone ever does get around to sorting out this corner of
GDB then it's easy enough to revert this code back to use
find_pc_function.

I do have one minor nit though...

>    if (fn != nullptr && sfn != fn)
>      {
>        if (!query (_("Line %d is not in `%s'.  Jump anyway? "), sal.line,
> diff --git a/gdb/testsuite/gdb.base/jump-inline.c b/gdb/testsuite/gdb.base/jump-inline.c
> new file mode 100644
> index 00000000000..17447c2d557
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/jump-inline.c
> @@ -0,0 +1,30 @@
> +/* Copyright 2021-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 2 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/>.  */
> +
> +__attribute__((always_inline))
> +static void inline
> +function_inline (int x)
> +{
> +  int a = x;
> +  a += 1020 + a;		/* increment-funct. */
> +  a = a + x;			/* inline-funct. */
> +}
> +
> +int
> +main ()
> +{
> +  function_inline (510);
> +  return 0;			/* out-of-func. */
> +}
> diff --git a/gdb/testsuite/gdb.base/jump-inline.exp b/gdb/testsuite/gdb.base/jump-inline.exp
> new file mode 100644
> index 00000000000..fef29fedb2f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/jump-inline.exp
> @@ -0,0 +1,45 @@
> +# Copyright 2021-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/>.  */
> +#
> +# Tests GDBs support for jump for inline functions.
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> +    return -1
> +}
> +
> +if { ![runto_main] } {
> +    untested "failed to run to main"

This untested line is not needed.  runto_main will emit a FAIL if
anything goes wrong.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

> +    return -1
> +}
> +
> +gdb_breakpoint [gdb_get_line_number "inline-funct"]
> +gdb_continue_to_breakpoint "inline-funct" ".*inline-funct.*"
> +
> +# Test jump to another function - main.
> +set out_func [gdb_get_line_number "out-of-func"]
> +gdb_test "jump $out_func" \
> +    "Not confirmed.*" \
> +    "aborted jump out of current function" \
> +    "Line $out_func is not in `function_inline.*'.  Jump anyway.*y or n. $" \
> +    "n"
> +
> +# Test jump in the same inline function.
> +set increment [gdb_get_line_number "increment-funct"]
> +gdb_breakpoint $increment
> +gdb_test "jump $increment" "Breakpoint .* at .*:$increment.*"
> +gdb_test "next" ".*inline-funct.*"
> +gdb_test "print a" "= 5100"
> -- 
> 2.34.3
>
> 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