Message ID | 20191221153325.6961-1-ssbssa@yahoo.de |
---|---|
State | New, archived |
Headers |
Received: (qmail 58584 invoked by alias); 21 Dec 2019 15:34:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 58573 invoked by uid 89); 21 Dec 2019 15:34:05 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.1 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=1618, HContent-Transfer-Encoding:8bit X-HELO: sonic301-21.consmr.mail.ir2.yahoo.com Received: from sonic301-21.consmr.mail.ir2.yahoo.com (HELO sonic301-21.consmr.mail.ir2.yahoo.com) (77.238.176.98) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 21 Dec 2019 15:34:04 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.de; s=s2048; t=1576942442; bh=/pPP6qb2RZOlDcpvJdmOecXR1LZZTwnPFLv7UhpTc0Q=; h=From:To:Subject:Date:References:From:Subject; b=Uy/5poKeL2rHoS17/aHlq8C1wtaaUaL/1+Qjv8pghf3Cvvkm7DokWl31TKAxlhZ9y7Ou5tdBqGS1/ybqqVBSA7Wrq52urQHtYwvV9yaWtoigYjpCe0KaUpWFNvInKVixZPtloZ3F1M4oHWxOt61oD3ioQZX4SFcAULRvIrmI81VHSabRcJrFOumxSc0wpRx+VmyNQbrX/SB1W8ORn5bd+QogXJmaN5BiPg3PrXaa+9UM4ogNwnowTLj2aTb+6Kq+h25H91QvNEZMsoZZ9PxpUdZ//5PKhATkcmNtzYNyWh5gqJE4WspKMZBMJkx3oTwj4JwdL1ndRnB9ryS6Qn+rCA== Received: from sonic.gate.mail.ne1.yahoo.com by sonic301.consmr.mail.ir2.yahoo.com with HTTP; Sat, 21 Dec 2019 15:34:02 +0000 Received: by smtp425.mail.ir2.yahoo.com (Oath Hermes SMTP Server) with ESMTPA ID a7d5c3ce358ba3b89925607548da72ef; Sat, 21 Dec 2019 15:33:57 +0000 (UTC) From: "Hannes Domani via gdb-patches" <gdb-patches@sourceware.org> Reply-To: Hannes Domani <ssbssa@yahoo.de> To: gdb-patches@sourceware.org Subject: [RFC][PATCH] Call tui_before_prompt in do_scroll_vertical Date: Sat, 21 Dec 2019 16:33:25 +0100 Message-Id: <20191221153325.6961-1-ssbssa@yahoo.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit References: <20191221153325.6961-1-ssbssa.ref@yahoo.de> Content-Length: 1763 X-IsSubscribed: yes |
Commit Message
Terekhov, Mikhail via Gdb-patches
Dec. 21, 2019, 3:33 p.m. UTC
Without this call scrolling in the src window does not work at all. First I tried it with tui_update_source_windows_with_line, but this didn't reset from_source_symtab (which was set deep in print_source_lines), which resulted in some weird behavior when switching from "layout split" to "layout asm" after scrolling down in the src window (the asm window was then overwritten by the src window). gdb/ChangeLog: 2019-12-21 Hannes Domani <ssbssa@yahoo.de> * tui/tui-hooks.c (tui_before_prompt): Remove static. * tui/tui-source.c (tui_before_prompt): Add prototype. (tui_source_window::do_scroll_vertical): Add tui_before_prompt call. --- gdb/tui/tui-hooks.c | 2 +- gdb/tui/tui-source.c | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-)
Comments
* Hannes Domani via gdb-patches <gdb-patches@sourceware.org> [2019-12-21 16:33:25 +0100]: > Without this call scrolling in the src window does not work at all. Thanks for spotting this and starting work on a fix. When fixing regressions like this its always useful to track down the commit that introduced the failure and add a link to it. This is not about blaming others, but just adds a really useful historical log. In this case, it was this commit that caused the breakage in scrolling: commit b4b49dcbff6b437fa8b4e2fc0c3f27b457f11310 (refs/bisect/bad) Date: Wed Nov 13 16:47:58 2019 -0700 Don't call tui_show_source from tui_ui_out > > First I tried it with tui_update_source_windows_with_line, but this didn't > reset from_source_symtab (which was set deep in print_source_lines), > which resulted in some weird behavior when switching from "layout split" > to "layout asm" after scrolling down in the src window (the asm window > was then overwritten by the src window). I was able to reproduce the layout split -> layout asm issue you describe, but, I'm not convinced that the solution below is the right way to go. Tom might suggest a better solution quicker, but if not I'll try to review this code properly soon. However, the questions/observations I have are: - It doesn't feel like calling a before prompt hook is the right way to go, and even if this is basically the right solution, we should split the important part of the hook into a separate function with a suitable name and call that instead. - I also don't understand why calling this hook in tui_souce_window::do_scroll_vertical changes what happens when we switch to tui_disasm_window. Stepping through from the 'layout asm' seems to show that we do correctly change to the asm window, and its only when we later call display_gdb_prompt (from event-top.c) that we overwrite the asm window with the source for some reason. I'd like to understand what's going on there more as it feels like if we could fix that as almost a separate issue then we should go back to calling tui_update_source_windows_with_line as you tried first, which feels like a better solution. - On coding style within GDB, we declare functions in header files, and define them in source files. Also declarations are marked extern and include return type and function name on one line, so it would be: extern void tui_before_prompt (const char *current_gdb_prompt); But like I said, I'll need convincing that this is the right approach to fix this. Thanks, Andrew > > gdb/ChangeLog: > > 2019-12-21 Hannes Domani <ssbssa@yahoo.de> > > * tui/tui-hooks.c (tui_before_prompt): Remove static. > * tui/tui-source.c (tui_before_prompt): Add prototype. > (tui_source_window::do_scroll_vertical): Add tui_before_prompt call. > --- > gdb/tui/tui-hooks.c | 2 +- > gdb/tui/tui-source.c | 5 +++++ > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c > index 8576bb8fcc..8b9e70316f 100644 > --- a/gdb/tui/tui-hooks.c > +++ b/gdb/tui/tui-hooks.c > @@ -179,7 +179,7 @@ tui_inferior_exit (struct inferior *inf) > > /* Observer for the before_prompt notification. */ > > -static void > +void > tui_before_prompt (const char *current_gdb_prompt) > { > tui_refresh_frame_and_register_information (); > diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c > index 0728263b8c..dfe5721edf 100644 > --- a/gdb/tui/tui-source.c > +++ b/gdb/tui/tui-source.c > @@ -39,6 +39,9 @@ > #include "tui/tui-source.h" > #include "gdb_curses.h" > > +void > +tui_before_prompt (const char *current_gdb_prompt); > + > /* Function to display source in the source window. */ > bool > tui_source_window::set_contents (struct gdbarch *arch, > @@ -158,6 +161,8 @@ tui_source_window::do_scroll_vertical (int num_to_scroll) > l.u.line_no = 1; > > print_source_lines (s, l.u.line_no, l.u.line_no + 1, 0); > + > + tui_before_prompt (""); > } > } > > -- > 2.24.1 >
Am Sonntag, 22. Dezember 2019, 01:57:56 MEZ hat Andrew Burgess <andrew.burgess@embecosm.com> Folgendes geschrieben: > * Hannes Domani via gdb-patches <gdb-patches@sourceware.org> [2019-12-21 16:33:25 +0100]: > > > Without this call scrolling in the src window does not work at all. > > Thanks for spotting this and starting work on a fix. > > When fixing regressions like this its always useful to track down the > commit that introduced the failure and add a link to it. This is not > about blaming others, but just adds a really useful historical log. > > In this case, it was this commit that caused the breakage in scrolling: > > commit b4b49dcbff6b437fa8b4e2fc0c3f27b457f11310 (refs/bisect/bad) > Date: Wed Nov 13 16:47:58 2019 -0700 > > Don't call tui_show_source from tui_ui_out I have never tried bisecting before, and I don't know how it works on Windows, but I'll try to do that the next time. > > > > First I tried it with tui_update_source_windows_with_line, but this didn't > > reset from_source_symtab (which was set deep in print_source_lines), > > which resulted in some weird behavior when switching from "layout split" > > to "layout asm" after scrolling down in the src window (the asm window > > was then overwritten by the src window). > > I was able to reproduce the layout split -> layout asm issue you > describe, but, I'm not convinced that the solution below is the right > way to go. > > Tom might suggest a better solution quicker, but if not I'll try to > review this code properly soon. > > However, the questions/observations I have are: > > - It doesn't feel like calling a before prompt hook is the right way > to go, and even if this is basically the right solution, we should > split the important part of the hook into a separate function with > a suitable name and call that instead. I wasn't 100% sure that this is the proper solution either, that's why I added [RFC]. > - I also don't understand why calling this hook in > tui_souce_window::do_scroll_vertical changes what happens when we > switch to tui_disasm_window. Stepping through from the 'layout > asm' seems to show that we do correctly change to the asm window, > and its only when we later call display_gdb_prompt (from > event-top.c) that we overwrite the asm window with the source for > some reason. I'd like to understand what's going on there more as > it feels like if we could fix that as almost a separate issue then > we should go back to calling tui_update_source_windows_with_line as > you tried first, which feels like a better solution. That's what I tried to describe with this: > > First I tried it with tui_update_source_windows_with_line, but this didn't > > reset from_source_symtab (which was set deep in print_source_lines), The variable from_source_symtab was set somewhere in print_source_lines, and because it was not reset, the next time tui_before_prompt is called, the source window is automatically added again in tui_refresh_frame_and_register_information. > - On coding style within GDB, we declare functions in header files, > and define them in source files. Also declarations are marked > extern and include return type and function name on one line, so it > would be: > > extern void tui_before_prompt (const char *current_gdb_prompt); > > But like I said, I'll need convincing that this is the right > approach to fix this. Again, I agree with you that this is probably not the correct fix, that's why I added [RFC] and didn't bother with the function prototype in the correct header file. If that's not the proper use of [RFC], I apologize. Regards Hannes Domani > > Thanks, > Andrew > > > > > > gdb/ChangeLog: > > > > 2019-12-21 Hannes Domani <ssbssa@yahoo.de> > > > > * tui/tui-hooks.c (tui_before_prompt): Remove static. > > * tui/tui-source.c (tui_before_prompt): Add prototype. > > (tui_source_window::do_scroll_vertical): Add tui_before_prompt call. > > --- > > gdb/tui/tui-hooks.c | 2 +- > > gdb/tui/tui-source.c | 5 +++++ > > 2 files changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c > > index 8576bb8fcc..8b9e70316f 100644 > > --- a/gdb/tui/tui-hooks.c > > +++ b/gdb/tui/tui-hooks.c > > @@ -179,7 +179,7 @@ tui_inferior_exit (struct inferior *inf) > > > > /* Observer for the before_prompt notification. */ > > > > -static void > > +void > > tui_before_prompt (const char *current_gdb_prompt) > > { > > tui_refresh_frame_and_register_information (); > > diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c > > index 0728263b8c..dfe5721edf 100644 > > --- a/gdb/tui/tui-source.c > > +++ b/gdb/tui/tui-source.c > > @@ -39,6 +39,9 @@ > > #include "tui/tui-source.h" > > #include "gdb_curses.h" > > > > +void > > +tui_before_prompt (const char *current_gdb_prompt); > > + > > /* Function to display source in the source window. */ > > bool > > tui_source_window::set_contents (struct gdbarch *arch, > > @@ -158,6 +161,8 @@ tui_source_window::do_scroll_vertical (int num_to_scroll) > > l.u.line_no = 1; > > > > print_source_lines (s, l.u.line_no, l.u.line_no + 1, 0); > > + > > + tui_before_prompt (""); > > } > > } > > > > -- > > 2.24.1 > > >
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c index 8576bb8fcc..8b9e70316f 100644 --- a/gdb/tui/tui-hooks.c +++ b/gdb/tui/tui-hooks.c @@ -179,7 +179,7 @@ tui_inferior_exit (struct inferior *inf) /* Observer for the before_prompt notification. */ -static void +void tui_before_prompt (const char *current_gdb_prompt) { tui_refresh_frame_and_register_information (); diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c index 0728263b8c..dfe5721edf 100644 --- a/gdb/tui/tui-source.c +++ b/gdb/tui/tui-source.c @@ -39,6 +39,9 @@ #include "tui/tui-source.h" #include "gdb_curses.h" +void +tui_before_prompt (const char *current_gdb_prompt); + /* Function to display source in the source window. */ bool tui_source_window::set_contents (struct gdbarch *arch, @@ -158,6 +161,8 @@ tui_source_window::do_scroll_vertical (int num_to_scroll) l.u.line_no = 1; print_source_lines (s, l.u.line_no, l.u.line_no + 1, 0); + + tui_before_prompt (""); } }