diff mbox

[RFC,8.3,0/3] Some style fixes

Message ID 8336ns3uv4.fsf@gnu.org
State New, archived
Headers

Commit Message

Eli Zaretskii March 12, 2019, 4:44 p.m. UTC 
  > From: Tom Tromey <tromey@adacore.com>
> Cc: tromey@adacore.com,  gdb-patches@sourceware.org
> Date: Mon, 11 Mar 2019 14:15:36 -0600
> 
> Eli> However, the artifacts I saw in the TUI display in the
> Eli> pretest are still there.  In particular, the right-hand side of the
> Eli> frame of the source window still gets overwritten when stepping
> Eli> through the inferior's code, and it looks like the newline is not
> Eli> echoed after commands typed in the command window.
> 
> I wonder if this is the "nonl" bug.
> You can test that theory by applying the appended.

It fixes the problems in the command window, so I think we should
apply this if no better ideas are brought up.

But the problems in the source window are not solved by this patch,
they have other reasons.  I debugged almost all of the problems I'm
aware of, please see the annotated patches below.  If they look
reasonable, I will push them.

After these patches, I have only one more problem: horizontal
scrolling of the source window.  First, the first press of left-arrow
doesn't have any effect; and second, scrolling draws TAB characters
incorrectly, it treats each TAB as if it took a single column on the
screen, so text after a TAB jumps by 8 columns when you scroll instead
of moving by one column.  I hope to have solution for this tomorrow.

Here are the patches for the other problems:

1. The first patch fixes the problem noticed by Pedro: pressing DOWN
arrow in the command window doesn't scroll the source window.  This is
because we don't initialize the s->nlines field, and then
tui_vertical_source_scroll thinks we are off the chart.  This fixes
that:

Comments

Pedro Alves March 15, 2019, 12:34 p.m. UTC | #1 
On 03/12/2019 04:44 PM, Eli Zaretskii wrote:
> 1. The first patch fixes the problem noticed by Pedro: pressing DOWN
> arrow in the command window doesn't scroll the source window.  This is
> because we don't initialize the s->nlines field, and then
> tui_vertical_source_scroll thinks we are off the chart.  This fixes
> that:
> 
> --- gdb/source-cache.c~4	2019-03-10 08:34:47.422752400 +0200
> +++ gdb/source-cache.c	2019-03-12 11:50:15.094147600 +0200
> @@ -194,6 +194,12 @@ source_cache::get_source_lines (struct s
>  	  std::ifstream input (fullname);
>  	  if (input.is_open ())
>  	    {
> +	      if (s->line_charpos == 0)
> +		{
> +		  scoped_fd desc = open_source_file (s);
> +		  if (desc.get () >= 0)
> +		    find_source_lines (s, desc.get ());

I think this should return false if open_source_file fails?  I.e., like this:

	      if (s->line_charpos == 0)
		{
		  scoped_fd desc = open_source_file (s);
		  if (desc.get () < 0)
                    return false;
		  find_source_lines (s, desc.get ());

Otherwise this LGTM.  I see get_plain_source_lines has similar
code, so my immediate thought was to move that to a helper function,
but there's a difference that makes that unviable -- get_plain_source_lines
always wants to open the source file first, while here we can avoid
it unless line_charpos is 0.

Thanks,
Pedro Alves
Pedro Alves March 15, 2019, 12:43 p.m. UTC | #2 
On 03/12/2019 04:44 PM, Eli Zaretskii wrote:
> 2. The fix for avoiding to overwrite the source window frame is
> simple: revert 4a3045920.  I don't really understand why that change
> was made: AFAIU, wclrtoeol clears to the end of the window line, and
> cannot be told to clear only part of the line.
> 
> --- gdb/tui/tui-winsource.c~4	2019-02-27 06:51:50.000000000 +0200
> +++ gdb/tui/tui-winsource.c	2019-03-12 10:57:02.052875200 +0200
> @@ -285,7 +285,12 @@ tui_show_source_line (struct tui_win_inf
>      wattroff (win_info->generic.handle, A_STANDOUT);
>  
>    /* Clear to end of line but stop before the border.  */
> -  wclrtoeol (win_info->generic.handle);
> +  int x = getcurx (win_info->generic.handle);
> +  while (x + 1 < win_info->generic.width)
> +    {
> +      waddch (win_info->generic.handle, ' ');
> +      x = getcurx (win_info->generic.handle);
> +    }
>  }
>  

Makes sense to me.  I confirm this fixes it for me too.

>  void

Thanks,
Pedro Alves
Eli Zaretskii March 15, 2019, 1:36 p.m. UTC | #3 
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 15 Mar 2019 12:34:34 +0000
> 
> On 03/12/2019 04:44 PM, Eli Zaretskii wrote:
> > 1. The first patch fixes the problem noticed by Pedro: pressing DOWN
> > arrow in the command window doesn't scroll the source window.  This is
> > because we don't initialize the s->nlines field, and then
> > tui_vertical_source_scroll thinks we are off the chart.  This fixes
> > that:
> > 
> > --- gdb/source-cache.c~4	2019-03-10 08:34:47.422752400 +0200
> > +++ gdb/source-cache.c	2019-03-12 11:50:15.094147600 +0200
> > @@ -194,6 +194,12 @@ source_cache::get_source_lines (struct s
> >  	  std::ifstream input (fullname);
> >  	  if (input.is_open ())
> >  	    {
> > +	      if (s->line_charpos == 0)
> > +		{
> > +		  scoped_fd desc = open_source_file (s);
> > +		  if (desc.get () >= 0)
> > +		    find_source_lines (s, desc.get ());
> 
> I think this should return false if open_source_file fails?

How could it fail if input.is_open returns non-zero?  Are you thinking
about some race, whereby something deletes the file after the
constructor for 'input' returns?

> Otherwise this LGTM.  I see get_plain_source_lines has similar
> code, so my immediate thought was to move that to a helper function,
> but there's a difference that makes that unviable -- get_plain_source_lines
> always wants to open the source file first, while here we can avoid
> it unless line_charpos is 0.

Right.  Also, I originally hoped std::ifstream will have a way to get
at the underlying file descriptor, but no such luck, AFAICT.
Pedro Alves March 15, 2019, 1:56 p.m. UTC | #4 
On 03/15/2019 01:36 PM, Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 15 Mar 2019 12:34:34 +0000
>>
>> On 03/12/2019 04:44 PM, Eli Zaretskii wrote:
>>> 1. The first patch fixes the problem noticed by Pedro: pressing DOWN
>>> arrow in the command window doesn't scroll the source window.  This is
>>> because we don't initialize the s->nlines field, and then
>>> tui_vertical_source_scroll thinks we are off the chart.  This fixes
>>> that:
>>>
>>> --- gdb/source-cache.c~4	2019-03-10 08:34:47.422752400 +0200
>>> +++ gdb/source-cache.c	2019-03-12 11:50:15.094147600 +0200
>>> @@ -194,6 +194,12 @@ source_cache::get_source_lines (struct s
>>>  	  std::ifstream input (fullname);
>>>  	  if (input.is_open ())
>>>  	    {
>>> +	      if (s->line_charpos == 0)
>>> +		{
>>> +		  scoped_fd desc = open_source_file (s);
>>> +		  if (desc.get () >= 0)
>>> +		    find_source_lines (s, desc.get ());
>>
>> I think this should return false if open_source_file fails?
> 
> How could it fail if input.is_open returns non-zero?  Are you thinking
> about some race, whereby something deletes the file after the
> constructor for 'input' returns?

Yes, something like that.  The possibility is small, but non-zero
that it could fail.  And if it does, we end up with s->nlines
uninitialized again.

> 
>> Otherwise this LGTM.  I see get_plain_source_lines has similar
>> code, so my immediate thought was to move that to a helper function,
>> but there's a difference that makes that unviable -- get_plain_source_lines
>> always wants to open the source file first, while here we can avoid
>> it unless line_charpos is 0.
> 
> Right.  Also, I originally hoped std::ifstream will have a way to get
> at the underlying file descriptor, but no such luck, AFAICT.

Yeah, I believe GNU has a non-standard way for that, but it's not portable.
(And not worth the bother to #ifdef here for that, IMHO.

Thanks,
Pedro Alves
Tom Tromey March 15, 2019, 3:33 p.m. UTC | #5 
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Otherwise this LGTM.  I see get_plain_source_lines has similar
Pedro> code, so my immediate thought was to move that to a helper function,
Pedro> but there's a difference that makes that unviable -- get_plain_source_lines
Pedro> always wants to open the source file first, while here we can avoid
Pedro> it unless line_charpos is 0.

I don't really like having to open the file like this on every step, and
especially not just do call some other function for its side-effects.
However, this is the status quo ante, so the patch seems fine; and I
made a note to fix this up eventually.

Tom
Eli Zaretskii March 16, 2019, 12:16 p.m. UTC | #6 
> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 15 Mar 2019 12:43:21 +0000
> 
> On 03/12/2019 04:44 PM, Eli Zaretskii wrote:
> > 2. The fix for avoiding to overwrite the source window frame is
> > simple: revert 4a3045920.  I don't really understand why that change
> > was made: AFAIU, wclrtoeol clears to the end of the window line, and
> > cannot be told to clear only part of the line.
> > 
> > --- gdb/tui/tui-winsource.c~4	2019-02-27 06:51:50.000000000 +0200
> > +++ gdb/tui/tui-winsource.c	2019-03-12 10:57:02.052875200 +0200
> > @@ -285,7 +285,12 @@ tui_show_source_line (struct tui_win_inf
> >      wattroff (win_info->generic.handle, A_STANDOUT);
> >  
> >    /* Clear to end of line but stop before the border.  */
> > -  wclrtoeol (win_info->generic.handle);
> > +  int x = getcurx (win_info->generic.handle);
> > +  while (x + 1 < win_info->generic.width)
> > +    {
> > +      waddch (win_info->generic.handle, ' ');
> > +      x = getcurx (win_info->generic.handle);
> > +    }
> >  }
> >  
> 
> Makes sense to me.  I confirm this fixes it for me too.

Thanks, pushed to both branches.
Eli Zaretskii March 16, 2019, 5:58 p.m. UTC | #7 
> Cc: tromey@adacore.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Fri, 15 Mar 2019 13:56:39 +0000
> 
> >> I think this should return false if open_source_file fails?
> > 
> > How could it fail if input.is_open returns non-zero?  Are you thinking
> > about some race, whereby something deletes the file after the
> > constructor for 'input' returns?
> 
> Yes, something like that.  The possibility is small, but non-zero
> that it could fail.  And if it does, we end up with s->nlines
> uninitialized again.

OK, pushed with that change to both branches.

Thanks.
Simon Marchi March 24, 2019, 3:35 p.m. UTC | #8 
On 2019-03-16 1:58 p.m., Eli Zaretskii wrote:
>> Cc: tromey@adacore.com, gdb-patches@sourceware.org
>> From: Pedro Alves <palves@redhat.com>
>> Date: Fri, 15 Mar 2019 13:56:39 +0000
>>
>>>> I think this should return false if open_source_file fails?
>>>
>>> How could it fail if input.is_open returns non-zero?  Are you thinking
>>> about some race, whereby something deletes the file after the
>>> constructor for 'input' returns?
>>
>> Yes, something like that.  The possibility is small, but non-zero
>> that it could fail.  And if it does, we end up with s->nlines
>> uninitialized again.
> 
> OK, pushed with that change to both branches.
> 
> Thanks.

Hi all,

I get an AddressSanitizer failure, and bisecting points to this commit.

I simply "start" an executable, and there is a use-after-free happening when
trying to print the stop location.  See the dump below.

Simon

$ ./gdb -nx --data-directory=data-directory ./a.out
(gdb) start
Temporary breakpoint 1 at 0x1130: file test.cpp, line 12.
Starting program: /home/simark/build/binutils-gdb/gdb/a.out

Temporary breakpoint 1, main () at test.cpp:12
=================================================================
==26068==ERROR: AddressSanitizer: heap-use-after-free on address 0x6210003d4100 at pc 0x7fed89a34681 bp 0x7ffd8d185d80 sp 0x7ffd8d185528
READ of size 2 at 0x6210003d4100 thread T0
    #0 0x7fed89a34680 in __interceptor_strlen /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:301
    #1 0x55b6edf6c2f7 in std::char_traits<char>::length(char const*) /usr/include/c++/8.2.1/bits/char_traits.h:320
    #2 0x55b6edf6c9b2 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) /usr/include/c++/8.2.1/bits/basic_string.h:516
    #3 0x55b6ef09121b in source_cache::get_source_lines(symtab*, int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) /home/simark/src/binutils-gdb/gdb/source-cache.c:214
    #4 0x55b6ef0a15cb in print_source_lines_base /home/simark/src/binutils-gdb/gdb/source.c:1340
    #5 0x55b6ef0a2045 in print_source_lines(symtab*, int, int, enum_flags<print_source_lines_flag>) /home/simark/src/binutils-gdb/gdb/source.c:1415
    #6 0x55b6ef112c87 in print_frame_info(frame_info*, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:914
    #7 0x55b6ef10e90d in print_stack_frame(frame_info*, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:180
    #8 0x55b6ee9592f8 in print_stop_location /home/simark/src/binutils-gdb/gdb/infrun.c:7853
    #9 0x55b6ee95948f in print_stop_event(ui_out*) /home/simark/src/binutils-gdb/gdb/infrun.c:7870
    #10 0x55b6ef34b962 in tui_on_normal_stop /home/simark/src/binutils-gdb/gdb/tui/tui-interp.c:98
    #11 0x55b6ee01a14d in std::_Function_handler<void (bpstats*, int), void (*)(bpstats*, int)>::_M_invoke(std::_Any_data const&, bpstats*&&, int&&) /usr/include/c++/8.2.1/bits/std_function.h:297
    #12 0x55b6ee965415 in std::function<void (bpstats*, int)>::operator()(bpstats*, int) const /usr/include/c++/8.2.1/bits/std_function.h:687
    #13 0x55b6ee962f1b in gdb::observers::observable<bpstats*, int>::notify(bpstats*, int) const /home/simark/src/binutils-gdb/gdb/common/observable.h:106
    #14 0x55b6ee95a6e7 in normal_stop() /home/simark/src/binutils-gdb/gdb/infrun.c:8142
    #15 0x55b6ee93f236 in fetch_inferior_event(void*) /home/simark/src/binutils-gdb/gdb/infrun.c:3782
    #16 0x55b6ee8f2641 in inferior_event_handler(inferior_event_type, void*) /home/simark/src/binutils-gdb/gdb/inf-loop.c:43
    #17 0x55b6eea2a1f0 in handle_target_event /home/simark/src/binutils-gdb/gdb/linux-nat.c:4358
    #18 0x55b6ee7045f1 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #19 0x55b6ee704e89 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #20 0x55b6ee7027b5 in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:322
    #21 0x55b6ee702907 in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #22 0x55b6eeadfc16 in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:331
    #23 0x55b6eeae2ef9 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1174
    #24 0x55b6eeae30c2 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1190
    #25 0x55b6edf4fa89 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
    #26 0x7fed88ad8222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
    #27 0x55b6edf4f86d in _start (/home/simark/build/binutils-gdb/gdb/gdb+0x197186d)

0x6210003d4100 is located 0 bytes inside of 4096-byte region [0x6210003d4100,0x6210003d5100)
freed by thread T0 here:
    #0 0x7fed89a8ac19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:66
    #1 0x55b6edfe12df in xfree<char> /home/simark/src/binutils-gdb/gdb/common/common-utils.h:60
    #2 0x55b6edfea675 in gdb::xfree_deleter<char>::operator()(char*) const /home/simark/src/binutils-gdb/gdb/common/gdb_unique_ptr.h:34
    #3 0x55b6edfe532c in std::unique_ptr<char, gdb::xfree_deleter<char> >::reset(char*) /usr/include/c++/8.2.1/bits/unique_ptr.h:382
    #4 0x55b6edfe7329 in std::unique_ptr<char, gdb::xfree_deleter<char> >::operator=(std::unique_ptr<char, gdb::xfree_deleter<char> >&&) /usr/include/c++/8.2.1/bits/unique_ptr.h:289
    #5 0x55b6ef09ec2b in find_and_open_source(char const*, char const*, std::unique_ptr<char, gdb::xfree_deleter<char> >*) /home/simark/src/binutils-gdb/gdb/source.c:990
    #6 0x55b6ef09f56a in open_source_file(symtab*) /home/simark/src/binutils-gdb/gdb/source.c:1069
    #7 0x55b6ef090f78 in source_cache::get_source_lines(symtab*, int, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*) /home/simark/src/binutils-gdb/gdb/source-cache.c:205
    #8 0x55b6ef0a15cb in print_source_lines_base /home/simark/src/binutils-gdb/gdb/source.c:1340
    #9 0x55b6ef0a2045 in print_source_lines(symtab*, int, int, enum_flags<print_source_lines_flag>) /home/simark/src/binutils-gdb/gdb/source.c:1415
    #10 0x55b6ef112c87 in print_frame_info(frame_info*, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:914
    #11 0x55b6ef10e90d in print_stack_frame(frame_info*, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:180
    #12 0x55b6ee9592f8 in print_stop_location /home/simark/src/binutils-gdb/gdb/infrun.c:7853
    #13 0x55b6ee95948f in print_stop_event(ui_out*) /home/simark/src/binutils-gdb/gdb/infrun.c:7870
    #14 0x55b6ef34b962 in tui_on_normal_stop /home/simark/src/binutils-gdb/gdb/tui/tui-interp.c:98
    #15 0x55b6ee01a14d in std::_Function_handler<void (bpstats*, int), void (*)(bpstats*, int)>::_M_invoke(std::_Any_data const&, bpstats*&&, int&&) /usr/include/c++/8.2.1/bits/std_function.h:297
    #16 0x55b6ee965415 in std::function<void (bpstats*, int)>::operator()(bpstats*, int) const /usr/include/c++/8.2.1/bits/std_function.h:687
    #17 0x55b6ee962f1b in gdb::observers::observable<bpstats*, int>::notify(bpstats*, int) const /home/simark/src/binutils-gdb/gdb/common/observable.h:106
    #18 0x55b6ee95a6e7 in normal_stop() /home/simark/src/binutils-gdb/gdb/infrun.c:8142
    #19 0x55b6ee93f236 in fetch_inferior_event(void*) /home/simark/src/binutils-gdb/gdb/infrun.c:3782
    #20 0x55b6ee8f2641 in inferior_event_handler(inferior_event_type, void*) /home/simark/src/binutils-gdb/gdb/inf-loop.c:43
    #21 0x55b6eea2a1f0 in handle_target_event /home/simark/src/binutils-gdb/gdb/linux-nat.c:4358
    #22 0x55b6ee7045f1 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #23 0x55b6ee704e89 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #24 0x55b6ee7027b5 in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:322
    #25 0x55b6ee702907 in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #26 0x55b6eeadfc16 in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:331
    #27 0x55b6eeae2ef9 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1174
    #28 0x55b6eeae30c2 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1190
    #29 0x55b6edf4fa89 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32

previously allocated by thread T0 here:
    #0 0x7fed89a8b019 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:86
    #1 0x7fed88af983f in realpath@@GLIBC_2.3 (/usr/lib/libc.so.6+0x4583f)
    #2 0x7fed899dbbbc in __interceptor_canonicalize_file_name /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:3297
    #3 0x55b6ee376a03 in gdb_realpath(char const*) /home/simark/src/binutils-gdb/gdb/common/pathstuff.c:72
    #4 0x55b6ef09ec12 in find_and_open_source(char const*, char const*, std::unique_ptr<char, gdb::xfree_deleter<char> >*) /home/simark/src/binutils-gdb/gdb/source.c:990
    #5 0x55b6ef09f56a in open_source_file(symtab*) /home/simark/src/binutils-gdb/gdb/source.c:1069
    #6 0x55b6ef0a0f12 in print_source_lines_base /home/simark/src/binutils-gdb/gdb/source.c:1270
    #7 0x55b6ef0a2045 in print_source_lines(symtab*, int, int, enum_flags<print_source_lines_flag>) /home/simark/src/binutils-gdb/gdb/source.c:1415
    #8 0x55b6ef112c87 in print_frame_info(frame_info*, int, print_what, int, int) /home/simark/src/binutils-gdb/gdb/stack.c:914
    #9 0x55b6ef10e90d in print_stack_frame(frame_info*, int, print_what, int) /home/simark/src/binutils-gdb/gdb/stack.c:180
    #10 0x55b6ee9592f8 in print_stop_location /home/simark/src/binutils-gdb/gdb/infrun.c:7853
    #11 0x55b6ee95948f in print_stop_event(ui_out*) /home/simark/src/binutils-gdb/gdb/infrun.c:7870
    #12 0x55b6ef34b962 in tui_on_normal_stop /home/simark/src/binutils-gdb/gdb/tui/tui-interp.c:98
    #13 0x55b6ee01a14d in std::_Function_handler<void (bpstats*, int), void (*)(bpstats*, int)>::_M_invoke(std::_Any_data const&, bpstats*&&, int&&) /usr/include/c++/8.2.1/bits/std_function.h:297
    #14 0x55b6ee965415 in std::function<void (bpstats*, int)>::operator()(bpstats*, int) const /usr/include/c++/8.2.1/bits/std_function.h:687
    #15 0x55b6ee962f1b in gdb::observers::observable<bpstats*, int>::notify(bpstats*, int) const /home/simark/src/binutils-gdb/gdb/common/observable.h:106
    #16 0x55b6ee95a6e7 in normal_stop() /home/simark/src/binutils-gdb/gdb/infrun.c:8142
    #17 0x55b6ee93f236 in fetch_inferior_event(void*) /home/simark/src/binutils-gdb/gdb/infrun.c:3782
    #18 0x55b6ee8f2641 in inferior_event_handler(inferior_event_type, void*) /home/simark/src/binutils-gdb/gdb/inf-loop.c:43
    #19 0x55b6eea2a1f0 in handle_target_event /home/simark/src/binutils-gdb/gdb/linux-nat.c:4358
    #20 0x55b6ee7045f1 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733
    #21 0x55b6ee704e89 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859
    #22 0x55b6ee7027b5 in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:322
    #23 0x55b6ee702907 in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371
    #24 0x55b6eeadfc16 in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:331
    #25 0x55b6eeae2ef9 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1174
    #26 0x55b6eeae30c2 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1190
    #27 0x55b6edf4fa89 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32
    #28 0x7fed88ad8222 in __libc_start_main (/usr/lib/libc.so.6+0x24222)
diff mbox

Patch

--- gdb/source-cache.c~4	2019-03-10 08:34:47.422752400 +0200
+++ gdb/source-cache.c	2019-03-12 11:50:15.094147600 +0200
@@ -194,6 +194,12 @@  source_cache::get_source_lines (struct s
 	  std::ifstream input (fullname);
 	  if (input.is_open ())
 	    {
+	      if (s->line_charpos == 0)
+		{
+		  scoped_fd desc = open_source_file (s);
+		  if (desc.get () >= 0)
+		    find_source_lines (s, desc.get ());
+		}
 	      srchilite::SourceHighlight highlighter ("esc.outlang");
 	      highlighter.setStyleFile("esc.style");
 
2. The fix for avoiding to overwrite the source window frame is
simple: revert 4a3045920.  I don't really understand why that change
was made: AFAIU, wclrtoeol clears to the end of the window line, and
cannot be told to clear only part of the line.

--- gdb/tui/tui-winsource.c~4	2019-02-27 06:51:50.000000000 +0200
+++ gdb/tui/tui-winsource.c	2019-03-12 10:57:02.052875200 +0200
@@ -285,7 +285,12 @@  tui_show_source_line (struct tui_win_inf
     wattroff (win_info->generic.handle, A_STANDOUT);
 
   /* Clear to end of line but stop before the border.  */
-  wclrtoeol (win_info->generic.handle);
+  int x = getcurx (win_info->generic.handle);
+  while (x + 1 < win_info->generic.width)
+    {
+      waddch (win_info->generic.handle, ' ');
+      x = getcurx (win_info->generic.handle);
+    }
 }
 
 void

3. This patch fixes another problem noticed by Pedro: the first time
you type UP or DOWN arrow in the command window, GDB should scroll the
source window, but instead it displays the line number and the file
name in the command window(?).  What happens there is that the first
time we call tui_ui_out::do_field_int, it doesn't initialize m_line,
because m_start_of_line is -1, as set by the constructor; and then the
following call to tui_ui_out::do_field_string falls back to
cli_ui_out::do_field_string because m_line is zero.  The fix below is
perhaps somewhat ad-hoc, mainly because I couldn't understand the
semantics of the values of m_start_of_line, especially its
non-positive values; please consider documenting them in the header.
Maybe if someone explains the semantics, I could come up with a more
sound patch (or conclude that the below is TRT).  Also, it looks like
the second test for m_line > 0 in tui_ui_out::do_field_string is
redundant?

--- gdb/tui/tui-out.c~4	2019-02-27 06:51:50.000000000 +0200
+++ gdb/tui/tui-out.c	2019-03-12 12:30:23.924230000 +0200
@@ -34,6 +34,9 @@  tui_ui_out::do_field_int (int fldno, int
   if (suppress_output ())
     return;
 
+  if (m_start_of_line < 0 && m_line == 0)
+    m_start_of_line = 0;
+
   /* Don't print line number, keep it for later.  */
   if (m_start_of_line == 0 && strcmp (fldname, "line") == 0)
     {