Fix display of source code in TUI window

  Hi,

this fixes a bug in the TUI display which was pointed out by Simon.
The problem is the source code window is showing the wrong or no source
at all, when the same PC has different possible source code locations as
it typically happens when stepping into nested inline functions.
I have an idea for a test case but do not know yet how to make a working
test case out of it.  Attached also a test case consisting of step.c / step.h.
When stepping into foo the TUI is unable to display foo, since it tries to open the
wrong source file.

gdb:
2019-12-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* tui/tui-hooks.c (tui_refresh_frame_and_register_information): Use
	the correct symtab to display the source window.

gdb/testsuite:
2019-12-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* gdb.tui/step.exp: New file.
	* gdb.tui/step.c: New file.
	* gdb.tui/step.h: New file.

All existing TUI tests are unaffected by this patch,
while the new test case is fixed.


Thanks
Bernd.
  

* Bernd Edlinger <bernd.edlinger@hotmail.de> [2019-12-20 21:06:10 +0000]:

> Hi,
> 
> this fixes a bug in the TUI display which was pointed out by Simon.
> The problem is the source code window is showing the wrong or no source
> at all, when the same PC has different possible source code locations as
> it typically happens when stepping into nested inline functions.
> I have an idea for a test case but do not know yet how to make a working
> test case out of it.  Attached also a test case consisting of step.c / step.h.
> When stepping into foo the TUI is unable to display foo, since it tries to open the
> wrong source file.
> 
> gdb:
> 2019-12-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* tui/tui-hooks.c (tui_refresh_frame_and_register_information): Use
> 	the correct symtab to display the source window.
> 
> gdb/testsuite:
> 2019-12-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	* gdb.tui/step.exp: New file.
> 	* gdb.tui/step.c: New file.
> 	* gdb.tui/step.h: New file.
> 
> All existing TUI tests are unaffected by this patch,
> while the new test case is fixed.
> 
> 
> Thanks
> Bernd.

> From 9c507b092cd27cf34b783cfb200bd895cae386f6 Mon Sep 17 00:00:00 2001
> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
> Date: Fri, 20 Dec 2019 11:27:59 +0100
> Subject: [PATCH 1/2] Fix display of source code in TUI window
> 
> It is not correct to extract the pc out of the fame info
> and locate source code based on the pc only, because that
> breaks with inline_skipped_frames.
> ---
>  gdb/tui/tui-hooks.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
> index bb96f4d..51533a2 100644
> --- a/gdb/tui/tui-hooks.c
> +++ b/gdb/tui/tui-hooks.c
> @@ -115,7 +115,7 @@ static void
>  tui_refresh_frame_and_register_information (int registers_too_p)
>  {
>    struct frame_info *fi;
> -  CORE_ADDR pc;
> +  symtab_and_line sal;
>    int frame_info_changed_p;
>  
>    if (!has_stack_frames ())
> @@ -128,18 +128,9 @@ tui_refresh_frame_and_register_information (int registers_too_p)
>    /* Ensure that symbols for this frame are read in.  Also, determine
>       the source language of this frame, and switch to it if
>       desired.  */

I haven't checked, but this comment smells like it might be out of
date now - possibly even before this change.  This might be a good
time to give it a refresh.

Thanks,
Andrew



> -  if (get_frame_pc_if_available (fi, &pc))
> -    {
> -      struct symtab *s;
> -
> -      s = find_pc_line_symtab (pc);
> -      /* elz: This if here fixes the problem with the pc not being
> -	 displayed in the tui asm layout, with no debug symbols.  The
> -	 value of s would be 0 here, and select_source_symtab would
> -	 abort the command by calling the 'error' function.  */
> -      if (s)
> -	select_source_symtab (s);
> -    }
> +  sal = find_frame_sal (fi);
> +  if (sal.symtab != NULL)
> +    select_source_symtab (sal.symtab);
>  
>    /* Display the frame position (even if there is no symbols or the PC
>       is not known).  */
> -- 
> 1.9.1
> 

> From b777fc642a75115c562483b33d2eeb4b10282483 Mon Sep 17 00:00:00 2001
> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
> Date: Fri, 20 Dec 2019 21:58:47 +0100
> Subject: [PATCH 2/2] Add a TUI test case for step into inline frames
> 
> ---
>  gdb/testsuite/gdb.tui/step.c   | 36 ++++++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.tui/step.exp | 39 +++++++++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.tui/step.h   | 22 ++++++++++++++++++++++
>  3 files changed, 97 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.tui/step.c
>  create mode 100644 gdb/testsuite/gdb.tui/step.exp
>  create mode 100644 gdb/testsuite/gdb.tui/step.h
> 
> diff --git a/gdb/testsuite/gdb.tui/step.c b/gdb/testsuite/gdb.tui/step.c
> new file mode 100644
> index 0000000..cb9dc1a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.tui/step.c
> @@ -0,0 +1,36 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 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/>.  */
> +
> +#include <stdlib.h>
> +
> +__attribute__((noinline, noclone)) static void
> +bar ()
> +{
> +}
> +
> +#include "step.h"
> +
> +int
> +main ()
> +{
> +  int x;
> +
> +  x = 0;
> +  foo ();
> +  abort ();
> +  return x;
> +}
> diff --git a/gdb/testsuite/gdb.tui/step.exp b/gdb/testsuite/gdb.tui/step.exp
> new file mode 100644
> index 0000000..1d9dd9e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.tui/step.exp
> @@ -0,0 +1,39 @@
> +# Copyright 2019 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +load_lib "tuiterm.exp"
> +
> +standard_testfile step.c
> +
> +if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
> +    return -1
> +}
> +
> +Term::clean_restart 24 80 $testfile
> +if {![Term::enter_tui]} {
> +    unsupported "TUI not supported"
> +}
> +
> +set text [Term::get_all_lines]
> +gdb_assert {![string match "No Source Available" $text]} \
> +    "initial source listing"
> +
> +Term::command "break main"
> +Term::command "run"
> +Term::check_contents "list main" "32 *x = 0;"
> +Term::command "step"
> +Term::check_contents "step main" "33 *foo \\(\\);"
> +Term::command "step"
> +Term::check_contents "step foo" "21 *bar \\(\\);"
> diff --git a/gdb/testsuite/gdb.tui/step.h b/gdb/testsuite/gdb.tui/step.h
> new file mode 100644
> index 0000000..6945bee
> --- /dev/null
> +++ b/gdb/testsuite/gdb.tui/step.h
> @@ -0,0 +1,22 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2019 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/>.  */
> +
> +__attribute__((__always_inline__)) static inline int
> +foo (void)
> +{
> +  bar ();
> +}
> -- 
> 1.9.1
>
  

On 12/22/19 12:19 AM, Andrew Burgess wrote:
> * Bernd Edlinger <bernd.edlinger@hotmail.de> [2019-12-20 21:06:10 +0000]:
> 
>> Hi,
>>
>> this fixes a bug in the TUI display which was pointed out by Simon.
>> The problem is the source code window is showing the wrong or no source
>> at all, when the same PC has different possible source code locations as
>> it typically happens when stepping into nested inline functions.
>> I have an idea for a test case but do not know yet how to make a working
>> test case out of it.  Attached also a test case consisting of step.c / step.h.
>> When stepping into foo the TUI is unable to display foo, since it tries to open the
>> wrong source file.
>>
>> gdb:
>> 2019-12-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* tui/tui-hooks.c (tui_refresh_frame_and_register_information): Use
>> 	the correct symtab to display the source window.
>>
>> gdb/testsuite:
>> 2019-12-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* gdb.tui/step.exp: New file.
>> 	* gdb.tui/step.c: New file.
>> 	* gdb.tui/step.h: New file.
>>
>> All existing TUI tests are unaffected by this patch,
>> while the new test case is fixed.
>>
>>
>> Thanks
>> Bernd.
> 
>> From 9c507b092cd27cf34b783cfb200bd895cae386f6 Mon Sep 17 00:00:00 2001
>> From: Bernd Edlinger <bernd.edlinger@hotmail.de>
>> Date: Fri, 20 Dec 2019 11:27:59 +0100
>> Subject: [PATCH 1/2] Fix display of source code in TUI window
>>
>> It is not correct to extract the pc out of the fame info
>> and locate source code based on the pc only, because that
>> breaks with inline_skipped_frames.
>> ---
>>  gdb/tui/tui-hooks.c | 17 ++++-------------
>>  1 file changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
>> index bb96f4d..51533a2 100644
>> --- a/gdb/tui/tui-hooks.c
>> +++ b/gdb/tui/tui-hooks.c
>> @@ -115,7 +115,7 @@ static void
>>  tui_refresh_frame_and_register_information (int registers_too_p)
>>  {
>>    struct frame_info *fi;
>> -  CORE_ADDR pc;
>> +  symtab_and_line sal;
>>    int frame_info_changed_p;
>>  
>>    if (!has_stack_frames ())
>> @@ -128,18 +128,9 @@ tui_refresh_frame_and_register_information (int registers_too_p)
>>    /* Ensure that symbols for this frame are read in.  Also, determine
>>       the source language of this frame, and switch to it if
>>       desired.  */
> 
> I haven't checked, but this comment smells like it might be out of
> date now - possibly even before this change.  This might be a good
> time to give it a refresh.
> 

Thanks for the review, Andrew.
I've noticed that a patch from Tom Tromey fixed that
issue yesterday.  So this patch will no longer
be needed.


Thanks
Bernd.