public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
@ 2023-01-24 15:19 Felix Willgerodt
  2023-02-20 12:50 ` [PING] " Willgerodt, Felix
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Felix Willgerodt @ 2023-01-24 15:19 UTC (permalink / raw)
  To: gdb-patches; +Cc: Felix Willgerodt, Cristian Sandu

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));
   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"
+    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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PING] [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-01-24 15:19 [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function Felix Willgerodt
@ 2023-02-20 12:50 ` Willgerodt, Felix
  2023-03-06  9:03 ` Willgerodt, Felix
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Willgerodt, Felix @ 2023-02-20 12:50 UTC (permalink / raw)
  To: gdb-patches

*Ping*

Thanks,
Felix

> -----Original Message-----
> From: Willgerodt, Felix <felix.willgerodt@intel.com>
> Sent: Dienstag, 24. Januar 2023 16:20
> To: gdb-patches@sourceware.org
> Cc: Willgerodt, Felix <felix.willgerodt@intel.com>; Cristian Sandu
> <cristian.sandu@intel.com>
> Subject: [PATCH 1/1] gdb: Avoid warning for the jump command inside an
> inline function.
> 
> 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));
>    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"
> +    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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PING] [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-01-24 15:19 [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function Felix Willgerodt
  2023-02-20 12:50 ` [PING] " Willgerodt, Felix
@ 2023-03-06  9:03 ` Willgerodt, Felix
  2023-03-16 15:08 ` Willgerodt, Felix
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Willgerodt, Felix @ 2023-03-06  9:03 UTC (permalink / raw)
  To: gdb-patches

*Ping* v2

Thanks,
Felix

> -----Original Message-----
> From: Willgerodt, Felix
> Sent: Montag, 20. Februar 2023 13:51
> To: gdb-patches@sourceware.org
> Subject: [PING] [PATCH 1/1] gdb: Avoid warning for the jump command
> inside an inline function.
> 
> *Ping*
> 
> Thanks,
> Felix
> 
> > -----Original Message-----
> > From: Willgerodt, Felix <felix.willgerodt@intel.com>
> > Sent: Dienstag, 24. Januar 2023 16:20
> > To: gdb-patches@sourceware.org
> > Cc: Willgerodt, Felix <felix.willgerodt@intel.com>; Cristian Sandu
> > <cristian.sandu@intel.com>
> > Subject: [PATCH 1/1] gdb: Avoid warning for the jump command inside an
> > inline function.
> >
> > 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));
> >    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"
> > +    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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PING] [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-01-24 15:19 [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function Felix Willgerodt
  2023-02-20 12:50 ` [PING] " Willgerodt, Felix
  2023-03-06  9:03 ` Willgerodt, Felix
@ 2023-03-16 15:08 ` Willgerodt, Felix
  2023-03-17 10:13 ` Bruno Larsen
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Willgerodt, Felix @ 2023-03-16 15:08 UTC (permalink / raw)
  To: gdb-patches

*Ping* v3

Thanks,
Felix

> -----Original Message-----
> From: Willgerodt, Felix
> Sent: Montag, 6. März 2023 10:04
> To: gdb-patches@sourceware.org
> Subject: RE: [PING] [PATCH 1/1] gdb: Avoid warning for the jump command
> inside an inline function.
> 
> *Ping* v2
> 
> Thanks,
> Felix
> 
> > -----Original Message-----
> > From: Willgerodt, Felix
> > Sent: Montag, 20. Februar 2023 13:51
> > To: gdb-patches@sourceware.org
> > Subject: [PING] [PATCH 1/1] gdb: Avoid warning for the jump command
> > inside an inline function.
> >
> > *Ping*
> >
> > Thanks,
> > Felix
> >
> > > -----Original Message-----
> > > From: Willgerodt, Felix <felix.willgerodt@intel.com>
> > > Sent: Dienstag, 24. Januar 2023 16:20
> > > To: gdb-patches@sourceware.org
> > > Cc: Willgerodt, Felix <felix.willgerodt@intel.com>; Cristian Sandu
> > > <cristian.sandu@intel.com>
> > > Subject: [PATCH 1/1] gdb: Avoid warning for the jump command inside an
> > > inline function.
> > >
> > > 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));
> > >    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"
> > > +    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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-01-24 15:19 [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function Felix Willgerodt
                   ` (2 preceding siblings ...)
  2023-03-16 15:08 ` Willgerodt, Felix
@ 2023-03-17 10:13 ` Bruno Larsen
  2023-03-17 12:56   ` Willgerodt, Felix
  2023-04-24 14:28 ` [PING] " Willgerodt, Felix
  2023-04-25 14:09 ` Andrew Burgess
  5 siblings, 1 reply; 16+ messages in thread
From: Bruno Larsen @ 2023-03-17 10:13 UTC (permalink / raw)
  To: Felix Willgerodt, gdb-patches; +Cc: Cristian Sandu

On 24/01/2023 16:19, Felix Willgerodt via Gdb-patches wrote:
> 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));

Hi Felix,

Thanks for doing this, it is a good improvement, but I don't know if 
this is the best way to go about it. Is there a reason why 
find_pc_function should not return inlined functions?

I feel like most of the time we want to know the function, knowing if 
we're in an inlined one would be desirable, but I might be wrong. Does 
anyone know?

-- 
Cheers,
Bruno

>     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"
> +    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"


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-03-17 10:13 ` Bruno Larsen
@ 2023-03-17 12:56   ` Willgerodt, Felix
  2023-03-17 13:33     ` Bruno Larsen
  0 siblings, 1 reply; 16+ messages in thread
From: Willgerodt, Felix @ 2023-03-17 12:56 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches

> -----Original Message-----
> From: Bruno Larsen <blarsen@redhat.com>
> Sent: Freitag, 17. März 2023 11:14
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> patches@sourceware.org
> Cc: Cristian Sandu <cristian.sandu@intel.com>
> Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside
> an inline function.
> 
> On 24/01/2023 16:19, Felix Willgerodt via Gdb-patches wrote:
> > 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));
> 
> Hi Felix,
> 
> Thanks for doing this, it is a good improvement, but I don't know if
> this is the best way to go about it. Is there a reason why
> find_pc_function should not return inlined functions?
> 
> I feel like most of the time we want to know the function, knowing if
> we're in an inlined one would be desirable, but I might be wrong. Does
> anyone know?
> 

Hi Bruno,

I don't know the details, but the comments in symtab.h are rather explicit
about it, so I assume there is a reason:


/* lookup the function symbol corresponding to the address.  The
   return value will not be an inlined function; the containing
   function will be returned instead.  */

extern struct symbol *find_pc_function (CORE_ADDR);

/* lookup the function symbol corresponding to the address and
   section.  The return value will be the closest enclosing function,
   which might be an inline function.  */

extern struct symbol *find_pc_sect_containing_function
  (CORE_ADDR pc, struct obj_section *section);


Thanks,
Felix
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-03-17 12:56   ` Willgerodt, Felix
@ 2023-03-17 13:33     ` Bruno Larsen
  2023-03-21 14:05       ` Willgerodt, Felix
  2023-04-11 13:08       ` Willgerodt, Felix
  0 siblings, 2 replies; 16+ messages in thread
From: Bruno Larsen @ 2023-03-17 13:33 UTC (permalink / raw)
  To: Willgerodt, Felix, gdb-patches

On 17/03/2023 13:56, Willgerodt, Felix wrote:
>> -----Original Message-----
>> From: Bruno Larsen <blarsen@redhat.com>
>> Sent: Freitag, 17. März 2023 11:14
>> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
>> patches@sourceware.org
>> Cc: Cristian Sandu <cristian.sandu@intel.com>
>> Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside
>> an inline function.
>>
>> On 24/01/2023 16:19, Felix Willgerodt via Gdb-patches wrote:
>>> 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));
>> Hi Felix,
>>
>> Thanks for doing this, it is a good improvement, but I don't know if
>> this is the best way to go about it. Is there a reason why
>> find_pc_function should not return inlined functions?
>>
>> I feel like most of the time we want to know the function, knowing if
>> we're in an inlined one would be desirable, but I might be wrong. Does
>> anyone know?
>>
> Hi Bruno,
>
> I don't know the details, but the comments in symtab.h are rather explicit
> about it, so I assume there is a reason:
>
>
> /* lookup the function symbol corresponding to the address.  The
>     return value will not be an inlined function; the containing
>     function will be returned instead.  */
>
> extern struct symbol *find_pc_function (CORE_ADDR);
>
> /* lookup the function symbol corresponding to the address and
>     section.  The return value will be the closest enclosing function,
>     which might be an inline function.  */
>
> extern struct symbol *find_pc_sect_containing_function
>    (CORE_ADDR pc, struct obj_section *section);

Hi Felix,

I thought it was mostly a descriptive comment, rather than prescriptive. 
I tested changing find_pc_function locally and there were only 2 
regressions, which might just be broken assumptions, but our testsuite 
is probably not very comprehensive on inlined functions, so I don't know 
how representative this test actually is.

-- 
Cheers,
Bruno

>
> Thanks,
> Felix
> 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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-03-17 13:33     ` Bruno Larsen
@ 2023-03-21 14:05       ` Willgerodt, Felix
  2023-03-21 14:44         ` Bruno Larsen
  2023-04-11 13:08       ` Willgerodt, Felix
  1 sibling, 1 reply; 16+ messages in thread
From: Willgerodt, Felix @ 2023-03-21 14:05 UTC (permalink / raw)
  To: Bruno Larsen, gdb-patches; +Cc: Pedro Alves

> -----Original Message-----
> From: Bruno Larsen <blarsen@redhat.com>
> Sent: Freitag, 17. März 2023 14:34
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside
> an inline function.
> 
> On 17/03/2023 13:56, Willgerodt, Felix wrote:
> >> -----Original Message-----
> >> From: Bruno Larsen <blarsen@redhat.com>
> >> Sent: Freitag, 17. März 2023 11:14
> >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> >> patches@sourceware.org
> >> Cc: Cristian Sandu <cristian.sandu@intel.com>
> >> Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command
> inside
> >> an inline function.
> >>
> >> On 24/01/2023 16:19, Felix Willgerodt via Gdb-patches wrote:
> >>> 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));
> >> Hi Felix,
> >>
> >> Thanks for doing this, it is a good improvement, but I don't know if
> >> this is the best way to go about it. Is there a reason why
> >> find_pc_function should not return inlined functions?
> >>
> >> I feel like most of the time we want to know the function, knowing if
> >> we're in an inlined one would be desirable, but I might be wrong. Does
> >> anyone know?
> >>
> > Hi Bruno,
> >
> > I don't know the details, but the comments in symtab.h are rather explicit
> > about it, so I assume there is a reason:
> >
> >
> > /* lookup the function symbol corresponding to the address.  The
> >     return value will not be an inlined function; the containing
> >     function will be returned instead.  */
> >
> > extern struct symbol *find_pc_function (CORE_ADDR);
> >
> > /* lookup the function symbol corresponding to the address and
> >     section.  The return value will be the closest enclosing function,
> >     which might be an inline function.  */
> >
> > extern struct symbol *find_pc_sect_containing_function
> >    (CORE_ADDR pc, struct obj_section *section);
> 
> Hi Felix,
> 
> I thought it was mostly a descriptive comment, rather than prescriptive.
> I tested changing find_pc_function locally and there were only 2
> regressions, which might just be broken assumptions, but our testsuite
> is probably not very comprehensive on inlined functions, so I don't know
> how representative this test actually is.
> 

Hi Bruno,

Could you share what changes you have done locally?
Did you basically just change find_pc_function to call
find_pc_sec_containing_function?

I saw that there is another comment in blockframe.c about
"backwards compatibility", but that has been in the code for ages and
I couldn't find anything interesting related to it.

When git blaming the comments in symtab.h, I saw they were added 
by Pedro a couple years ago:
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;f=gdb/symtab.h;h=cd2bb709940d33668fe6dbe8d4ffee0ed44c25e6
Maybe he can help shed some light on this?

Thanks,
Felix
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-03-21 14:05       ` Willgerodt, Felix
@ 2023-03-21 14:44         ` Bruno Larsen
  0 siblings, 0 replies; 16+ messages in thread
From: Bruno Larsen @ 2023-03-21 14:44 UTC (permalink / raw)
  To: Willgerodt, Felix, gdb-patches; +Cc: Pedro Alves

On 21/03/2023 15:05, Willgerodt, Felix wrote:
>> -----Original Message-----
>> From: Bruno Larsen <blarsen@redhat.com>
>> Sent: Freitag, 17. März 2023 14:34
>> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
>> patches@sourceware.org
>> Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside
>> an inline function.
>>
>> On 17/03/2023 13:56, Willgerodt, Felix wrote:
>>>> -----Original Message-----
>>>> From: Bruno Larsen <blarsen@redhat.com>
>>>> Sent: Freitag, 17. März 2023 11:14
>>>> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
>>>> patches@sourceware.org
>>>> Cc: Cristian Sandu <cristian.sandu@intel.com>
>>>> Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command
>> inside
>>>> an inline function.
>>>>
>>>> On 24/01/2023 16:19, Felix Willgerodt via Gdb-patches wrote:
>>>>> 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));
>>>> Hi Felix,
>>>>
>>>> Thanks for doing this, it is a good improvement, but I don't know if
>>>> this is the best way to go about it. Is there a reason why
>>>> find_pc_function should not return inlined functions?
>>>>
>>>> I feel like most of the time we want to know the function, knowing if
>>>> we're in an inlined one would be desirable, but I might be wrong. Does
>>>> anyone know?
>>>>
>>> Hi Bruno,
>>>
>>> I don't know the details, but the comments in symtab.h are rather explicit
>>> about it, so I assume there is a reason:
>>>
>>>
>>> /* lookup the function symbol corresponding to the address.  The
>>>      return value will not be an inlined function; the containing
>>>      function will be returned instead.  */
>>>
>>> extern struct symbol *find_pc_function (CORE_ADDR);
>>>
>>> /* lookup the function symbol corresponding to the address and
>>>      section.  The return value will be the closest enclosing function,
>>>      which might be an inline function.  */
>>>
>>> extern struct symbol *find_pc_sect_containing_function
>>>     (CORE_ADDR pc, struct obj_section *section);
>> Hi Felix,
>>
>> I thought it was mostly a descriptive comment, rather than prescriptive.
>> I tested changing find_pc_function locally and there were only 2
>> regressions, which might just be broken assumptions, but our testsuite
>> is probably not very comprehensive on inlined functions, so I don't know
>> how representative this test actually is.
>>
> Hi Bruno,
>
> Could you share what changes you have done locally?
> Did you basically just change find_pc_function to call
> find_pc_sec_containing_function?

Hi Felix

Yeah, this is exactly what I did.

>
> I saw that there is another comment in blockframe.c about
> "backwards compatibility", but that has been in the code for ages and
> I couldn't find anything interesting related to it.
Backwards compatibility is part of the reason I'm hesitant to trust my 
results. Fedora has very new everything, so I don't get to trigger bugs 
in old compilers or setups if we dont have a test manually recreating 
that. The other part is some other compiler/language/combination of 
things that could go wrong. I hope Pedro can remember the original 
reasoning!
>
> When git blaming the comments in symtab.h, I saw they were added
> by Pedro a couple years ago:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;f=gdb/symtab.h;h=cd2bb709940d33668fe6dbe8d4ffee0ed44c25e6
> Maybe he can help shed some light on this?
>
> Thanks,
> Felix
> 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


-- 
Cheers,
Bruno


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-03-17 13:33     ` Bruno Larsen
  2023-03-21 14:05       ` Willgerodt, Felix
@ 2023-04-11 13:08       ` Willgerodt, Felix
  1 sibling, 0 replies; 16+ messages in thread
From: Willgerodt, Felix @ 2023-04-11 13:08 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Bruno Larsen, gdb-patches

Hi Pedro,

Sorry for pinging you directly, but could you help us shed some light on
this question?

Thanks a lot,
Felix

> -----Original Message-----
> From: Willgerodt, Felix
> Sent: Dienstag, 21. März 2023 15:05
> To: Bruno Larsen <blarsen@redhat.com>; gdb-patches@sourceware.org
> Cc: Pedro Alves <pedro@palves.net>
> Subject: RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an
> inline function.
> 
> > -----Original Message-----
> > From: Bruno Larsen <blarsen@redhat.com>
> > Sent: Freitag, 17. März 2023 14:34
> > To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> > patches@sourceware.org
> > Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside
> > an inline function.
> >
> > On 17/03/2023 13:56, Willgerodt, Felix wrote:
> > >> -----Original Message-----
> > >> From: Bruno Larsen <blarsen@redhat.com>
> > >> Sent: Freitag, 17. März 2023 11:14
> > >> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> > >> patches@sourceware.org
> > >> Cc: Cristian Sandu <cristian.sandu@intel.com>
> > >> Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command
> > inside
> > >> an inline function.
> > >>
> > >> On 24/01/2023 16:19, Felix Willgerodt via Gdb-patches wrote:
> > >>> 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));
> > >> Hi Felix,
> > >>
> > >> Thanks for doing this, it is a good improvement, but I don't know if
> > >> this is the best way to go about it. Is there a reason why
> > >> find_pc_function should not return inlined functions?
> > >>
> > >> I feel like most of the time we want to know the function, knowing if
> > >> we're in an inlined one would be desirable, but I might be wrong. Does
> > >> anyone know?
> > >>
> > > Hi Bruno,
> > >
> > > I don't know the details, but the comments in symtab.h are rather explicit
> > > about it, so I assume there is a reason:
> > >
> > >
> > > /* lookup the function symbol corresponding to the address.  The
> > >     return value will not be an inlined function; the containing
> > >     function will be returned instead.  */
> > >
> > > extern struct symbol *find_pc_function (CORE_ADDR);
> > >
> > > /* lookup the function symbol corresponding to the address and
> > >     section.  The return value will be the closest enclosing function,
> > >     which might be an inline function.  */
> > >
> > > extern struct symbol *find_pc_sect_containing_function
> > >    (CORE_ADDR pc, struct obj_section *section);
> >
> > Hi Felix,
> >
> > I thought it was mostly a descriptive comment, rather than prescriptive.
> > I tested changing find_pc_function locally and there were only 2
> > regressions, which might just be broken assumptions, but our testsuite
> > is probably not very comprehensive on inlined functions, so I don't know
> > how representative this test actually is.
> >
> 
> Hi Bruno,
> 
> Could you share what changes you have done locally?
> Did you basically just change find_pc_function to call
> find_pc_sec_containing_function?
> 
> I saw that there is another comment in blockframe.c about
> "backwards compatibility", but that has been in the code for ages and
> I couldn't find anything interesting related to it.
> 
> When git blaming the comments in symtab.h, I saw they were added
> by Pedro a couple years ago:
> https://sourceware.org/git/?p=binutils-
> gdb.git;a=commit;f=gdb/symtab.h;h=cd2bb709940d33668fe6dbe8d4ffee0ed
> 44c25e6
> Maybe he can help shed some light on this?
> 
> Thanks,
> Felix
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PING] [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-01-24 15:19 [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function Felix Willgerodt
                   ` (3 preceding siblings ...)
  2023-03-17 10:13 ` Bruno Larsen
@ 2023-04-24 14:28 ` Willgerodt, Felix
  2023-04-25 14:09 ` Andrew Burgess
  5 siblings, 0 replies; 16+ messages in thread
From: Willgerodt, Felix @ 2023-04-24 14:28 UTC (permalink / raw)
  To: gdb-patches; +Cc: Bruno Larsen

*Ping* v4.

There was a small discussion on this series:
https://sourceware.org/pipermail/gdb-patches/2023-March/198166.html

But it didn't lead anywhere yet, so I decided to ping again.
(Sorry if that isn't the right way to proceed in such cases.)
But I still think this version is fixing something useful, and
is the right way for the current internal API.
Fixing that API could be done in the future if someone thinks
that is needed.

Thanks,
Felix

> -----Original Message-----
> From: Willgerodt, Felix
> Sent: Donnerstag, 16. März 2023 16:08
> To: gdb-patches@sourceware.org
> Subject: RE: [PING] [PATCH 1/1] gdb: Avoid warning for the jump command
> inside an inline function.
> 
> *Ping* v3
> 
> Thanks,
> Felix
> 
> > -----Original Message-----
> > From: Willgerodt, Felix
> > Sent: Montag, 6. März 2023 10:04
> > To: gdb-patches@sourceware.org
> > Subject: RE: [PING] [PATCH 1/1] gdb: Avoid warning for the jump command
> > inside an inline function.
> >
> > *Ping* v2
> >
> > Thanks,
> > Felix
> >
> > > -----Original Message-----
> > > From: Willgerodt, Felix
> > > Sent: Montag, 20. Februar 2023 13:51
> > > To: gdb-patches@sourceware.org
> > > Subject: [PING] [PATCH 1/1] gdb: Avoid warning for the jump command
> > > inside an inline function.
> > >
> > > *Ping*
> > >
> > > Thanks,
> > > Felix
> > >
> > > > -----Original Message-----
> > > > From: Willgerodt, Felix <felix.willgerodt@intel.com>
> > > > Sent: Dienstag, 24. Januar 2023 16:20
> > > > To: gdb-patches@sourceware.org
> > > > Cc: Willgerodt, Felix <felix.willgerodt@intel.com>; Cristian Sandu
> > > > <cristian.sandu@intel.com>
> > > > Subject: [PATCH 1/1] gdb: Avoid warning for the jump command inside
> an
> > > > inline function.
> > > >
> > > > 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));
> > > >    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"
> > > > +    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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-01-24 15:19 [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function Felix Willgerodt
                   ` (4 preceding siblings ...)
  2023-04-24 14:28 ` [PING] " Willgerodt, Felix
@ 2023-04-25 14:09 ` Andrew Burgess
  2023-04-25 14:40   ` Willgerodt, Felix
  2023-05-04  8:10   ` Willgerodt, Felix
  5 siblings, 2 replies; 16+ messages in thread
From: Andrew Burgess @ 2023-04-25 14:09 UTC (permalink / raw)
  To: Felix Willgerodt via Gdb-patches, gdb-patches
  Cc: Felix Willgerodt, Cristian Sandu

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-04-25 14:09 ` Andrew Burgess
@ 2023-04-25 14:40   ` Willgerodt, Felix
  2023-05-04  8:10   ` Willgerodt, Felix
  1 sibling, 0 replies; 16+ messages in thread
From: Willgerodt, Felix @ 2023-04-25 14:40 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Dienstag, 25. April 2023 16:09
> To: Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org>; gdb-
> patches@sourceware.org
> Cc: Willgerodt, Felix <felix.willgerodt@intel.com>; Cristian Sandu
> <cristian.sandu@intel.com>
> Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside
> an inline function.
> 
> 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.

Right, I will remove it.

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

Thank you for your review and thorough feedback. I agree with your
thoughts and conclusion.
I am wondering though, as you gave me a reviewed-by but not an
approved-by, should I wait a bit more or can I push this? I posted it originally
in January and pinged a few times. So I tend to think that waiting more isn't
worth it, but I am of course fine with doing that.

Thanks,
Felix
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-04-25 14:09 ` Andrew Burgess
  2023-04-25 14:40   ` Willgerodt, Felix
@ 2023-05-04  8:10   ` Willgerodt, Felix
  2023-05-05 16:21     ` Andrew Burgess
  1 sibling, 1 reply; 16+ messages in thread
From: Willgerodt, Felix @ 2023-05-04  8:10 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

> -----Original Message-----
> From: Willgerodt, Felix
> Sent: Dienstag, 25. April 2023 16:40
> To: Andrew Burgess <aburgess@redhat.com>; gdb-
> patches@sourceware.org
> Subject: RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an
> inline function.
> 
> > -----Original Message-----
> > From: Andrew Burgess <aburgess@redhat.com>
> > Sent: Dienstag, 25. April 2023 16:09
> > To: Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org>; gdb-
> > patches@sourceware.org
> > Cc: Willgerodt, Felix <felix.willgerodt@intel.com>; Cristian Sandu
> > <cristian.sandu@intel.com>
> > Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside
> > an inline function.
> >
> > 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.
> 
> Right, I will remove it.
> 
> > Reviewed-By: Andrew Burgess <aburgess@redhat.com>
> >
> > Thanks,
> > Andrew
> 
> Thank you for your review and thorough feedback. I agree with your
> thoughts and conclusion.
> I am wondering though, as you gave me a reviewed-by but not an
> approved-by, should I wait a bit more or can I push this? I posted it originally
> in January and pinged a few times. So I tend to think that waiting more isn't
> worth it, but I am of course fine with doing that.
> 
> Thanks,
> Felix

*Pinging again after 10 days of no further comments*

Is it okay to push this with the untested line removed?

Thanks,
Felix
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-05-04  8:10   ` Willgerodt, Felix
@ 2023-05-05 16:21     ` Andrew Burgess
  2023-05-08  7:22       ` Willgerodt, Felix
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Burgess @ 2023-05-05 16:21 UTC (permalink / raw)
  To: Willgerodt, Felix, gdb-patches

"Willgerodt, Felix" <felix.willgerodt@intel.com> writes:

>> -----Original Message-----
>> From: Willgerodt, Felix
>> Sent: Dienstag, 25. April 2023 16:40
>> To: Andrew Burgess <aburgess@redhat.com>; gdb-
>> patches@sourceware.org
>> Subject: RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an
>> inline function.
>> 
>> > -----Original Message-----
>> > From: Andrew Burgess <aburgess@redhat.com>
>> > Sent: Dienstag, 25. April 2023 16:09
>> > To: Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org>; gdb-
>> > patches@sourceware.org
>> > Cc: Willgerodt, Felix <felix.willgerodt@intel.com>; Cristian Sandu
>> > <cristian.sandu@intel.com>
>> > Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command inside
>> > an inline function.
>> >
>> > 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.
>> 
>> Right, I will remove it.
>> 
>> > Reviewed-By: Andrew Burgess <aburgess@redhat.com>
>> >
>> > Thanks,
>> > Andrew
>> 
>> Thank you for your review and thorough feedback. I agree with your
>> thoughts and conclusion.
>> I am wondering though, as you gave me a reviewed-by but not an
>> approved-by, should I wait a bit more or can I push this? I posted it originally
>> in January and pinged a few times. So I tend to think that waiting more isn't
>> worth it, but I am of course fine with doing that.
>> 
>> Thanks,
>> Felix
>
> *Pinging again after 10 days of no further comments*
>
> Is it okay to push this with the untested line removed?

As nobody has complained:

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

Thanks,
Andrew


>
> Thanks,
> Felix
> 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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function.
  2023-05-05 16:21     ` Andrew Burgess
@ 2023-05-08  7:22       ` Willgerodt, Felix
  0 siblings, 0 replies; 16+ messages in thread
From: Willgerodt, Felix @ 2023-05-08  7:22 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Freitag, 5. Mai 2023 18:22
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> patches@sourceware.org
> Subject: RE: [PATCH 1/1] gdb: Avoid warning for the jump command inside an
> inline function.
> 
> "Willgerodt, Felix" <felix.willgerodt@intel.com> writes:
> 
> >> -----Original Message-----
> >> From: Willgerodt, Felix
> >> Sent: Dienstag, 25. April 2023 16:40
> >> To: Andrew Burgess <aburgess@redhat.com>; gdb-
> >> patches@sourceware.org
> >> Subject: RE: [PATCH 1/1] gdb: Avoid warning for the jump command
> inside an
> >> inline function.
> >>
> >> > -----Original Message-----
> >> > From: Andrew Burgess <aburgess@redhat.com>
> >> > Sent: Dienstag, 25. April 2023 16:09
> >> > To: Felix Willgerodt via Gdb-patches <gdb-patches@sourceware.org>;
> gdb-
> >> > patches@sourceware.org
> >> > Cc: Willgerodt, Felix <felix.willgerodt@intel.com>; Cristian Sandu
> >> > <cristian.sandu@intel.com>
> >> > Subject: Re: [PATCH 1/1] gdb: Avoid warning for the jump command
> inside
> >> > an inline function.
> >> >
> >> > 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.
> >>
> >> Right, I will remove it.
> >>
> >> > Reviewed-By: Andrew Burgess <aburgess@redhat.com>
> >> >
> >> > Thanks,
> >> > Andrew
> >>
> >> Thank you for your review and thorough feedback. I agree with your
> >> thoughts and conclusion.
> >> I am wondering though, as you gave me a reviewed-by but not an
> >> approved-by, should I wait a bit more or can I push this? I posted it
> originally
> >> in January and pinged a few times. So I tend to think that waiting more
> isn't
> >> worth it, but I am of course fine with doing that.
> >>
> >> Thanks,
> >> Felix
> >
> > *Pinging again after 10 days of no further comments*
> >
> > Is it okay to push this with the untested line removed?
> 
> As nobody has complained:
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>
> 
> Thanks,
> Andrew
> 

Thanks! I pushed this now.

Felix
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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2023-05-08  7:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-24 15:19 [PATCH 1/1] gdb: Avoid warning for the jump command inside an inline function Felix Willgerodt
2023-02-20 12:50 ` [PING] " Willgerodt, Felix
2023-03-06  9:03 ` Willgerodt, Felix
2023-03-16 15:08 ` Willgerodt, Felix
2023-03-17 10:13 ` Bruno Larsen
2023-03-17 12:56   ` Willgerodt, Felix
2023-03-17 13:33     ` Bruno Larsen
2023-03-21 14:05       ` Willgerodt, Felix
2023-03-21 14:44         ` Bruno Larsen
2023-04-11 13:08       ` Willgerodt, Felix
2023-04-24 14:28 ` [PING] " Willgerodt, Felix
2023-04-25 14:09 ` Andrew Burgess
2023-04-25 14:40   ` Willgerodt, Felix
2023-05-04  8:10   ` Willgerodt, Felix
2023-05-05 16:21     ` Andrew Burgess
2023-05-08  7:22       ` Willgerodt, Felix

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).