From patchwork Sat Sep 15 22:21:38 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 29397 Received: (qmail 33622 invoked by alias); 15 Sep 2018 22:21:44 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 33608 invoked by uid 89); 15 Sep 2018 22:21:44 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=fullname, alright, hs, sal X-HELO: gateway24.websitewelcome.com Received: from gateway24.websitewelcome.com (HELO gateway24.websitewelcome.com) (192.185.51.110) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 15 Sep 2018 22:21:41 +0000 Received: from cm14.websitewelcome.com (cm14.websitewelcome.com [100.42.49.7]) by gateway24.websitewelcome.com (Postfix) with ESMTP id 607CA8CC8 for ; Sat, 15 Sep 2018 17:21:40 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id 1IwegyUAtkBj61Iweg8xc3; Sat, 15 Sep 2018 17:21:40 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=Hsr6+q3Gi7YcOzuixfZO47z0bLD+oqEACzZzgPpmB3k=; b=vladSN8DYJNtYMP248onx8N4ti xLiD1P7YY/vCSTsO9oDAKX9VaVhCtCu4J5JMlFsCzIIamh5xf3QLA3e1PGVg96eNC7OWct3BjT2gu B1BTKKxEbfJvp28doOSBxq8QF; Received: from 97-122-190-66.hlrn.qwest.net ([97.122.190.66]:45716 helo=bapiya) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1g1Iwe-000q4R-37; Sat, 15 Sep 2018 17:21:40 -0500 From: Tom Tromey To: Tom Tromey Cc: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [PATCH] Make "list" work again in TUI References: <20180908143153.18583-1-tom@tromey.com> <87musq4jdk.fsf@tromey.com> <66752a61-08f1-5588-691d-205e89b09471@redhat.com> <87musiqxbz.fsf@tromey.com> Date: Sat, 15 Sep 2018 16:21:38 -0600 In-Reply-To: <87musiqxbz.fsf@tromey.com> (Tom Tromey's message of "Sat, 15 Sep 2018 15:12:32 -0600") Message-ID: <87in36qu4t.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1.50 (gnu/linux) MIME-Version: 1.0 Pedro> That might suggest instead to start by seeing about trying to remove Pedro> the select_source_symtab call from tui_refresh_frame_and_register_information. Pedro> Why do we need that? Can we remove it and instead see about making Pedro> tui_refresh_frame_and_register_information update the source window Pedro> from get_current_source_symtab_and_line, if set_current_source_symtab_and_line Pedro> was meanwhile called? I.e., refresh the source window if something changed Pedro> "list"'s current source&line. Tom> I'm going to give it a try. I can almost get everything working by tracking both changes to the source SAL and changes to the "user select context", and then gating what happens in tui_refresh_frame_and_register_information based on which thing changed last. However this code in tui-winsource.c:a sal.line = line_or_addr.u.line_no + (win_info->generic.content_size - 2); sal.symtab = s; sal.pspace = SYMTAB_PSPACE (s); set_current_source_symtab_and_line (sal); triggers this warning in source.c: printf_unfiltered ("Line number %d out of range; " "%s has %d lines.\n", line_no, symtab_to_filename_for_display (s), s->nlines); I don't really understand this. Simply tracking the SAL is not enough, I think, because you want the TUI to react to changes that do not have a SAL. For example, up/down when the frames do not have debug info. Aside from the warning the other bug I see now is that if I "layout src", stop in a frame without debuginfo and then "layout asm", the view won't show the assembly until I up+down. Also during this adventure I found out about the "update" command which just does this: execute_command ("frame 0", from_tty); So this is intended to work at least. I may keep plugging away at this, who knows. The TUI really needs work. I'm appending my WIP patch. Tom diff --git a/gdb/observable.c b/gdb/observable.c index 5539b9837b5..cb967993cce 100644 --- a/gdb/observable.c +++ b/gdb/observable.c @@ -74,6 +74,7 @@ DEFINE_OBSERVABLE (inferior_call_pre); DEFINE_OBSERVABLE (inferior_call_post); DEFINE_OBSERVABLE (register_changed); DEFINE_OBSERVABLE (user_selected_context_changed); +DEFINE_OBSERVABLE (current_source_symtab_and_line_changed); } /* namespace observers */ } /* namespace gdb */ diff --git a/gdb/observable.h b/gdb/observable.h index 34447b90bb6..8d96cb5dda0 100644 --- a/gdb/observable.h +++ b/gdb/observable.h @@ -228,6 +228,12 @@ extern observable register_changed; frame has changed. */ extern observable user_selected_context_changed; +/* The CLI's notion of the current source has changed. This differs + from user_selected_context_changed in that it is also set by the + "list" command. */ + +extern observable<> current_source_symtab_and_line_changed; + } /* namespace observers */ } /* namespace gdb */ diff --git a/gdb/source.c b/gdb/source.c index ec0ea3b81e3..88c793a752e 100644 --- a/gdb/source.c +++ b/gdb/source.c @@ -45,6 +45,7 @@ #include "common/scoped_fd.h" #include #include "common/pathstuff.h" +#include "observable.h" #define OPEN_MODE (O_RDONLY | O_BINARY) #define FDOPEN_MODE FOPEN_RB @@ -201,6 +202,26 @@ set_default_source_symtab_and_line (void) select_source_symtab (0); } +/* Helper function to set the current source SAL and then notify any + observers. */ + +static void +internal_set_current_sal (struct program_space *pspace, struct symtab *symtab, + int line) +{ + /* Avoid no-op notifications. */ + if (current_source_pspace != pspace + || current_source_symtab != symtab + || current_source_line != line) + { + current_source_pspace = pspace; + current_source_symtab = symtab; + current_source_line = line; + + gdb::observers::current_source_symtab_and_line_changed.notify (); + } +} + /* Return the current default file for listing and next line to list (the returned sal pc and end fields are not valid.) and set the current default to whatever is in SAL. @@ -217,9 +238,7 @@ set_current_source_symtab_and_line (const symtab_and_line &sal) cursal.pc = 0; cursal.end = 0; - current_source_pspace = sal.pspace; - current_source_symtab = sal.symtab; - current_source_line = sal.line; + internal_set_current_sal (sal.pspace, sal.symtab, sal.line); /* Force the next "list" to center around the current line. */ clear_lines_listed_range (); @@ -232,8 +251,7 @@ set_current_source_symtab_and_line (const symtab_and_line &sal) void clear_current_source_symtab_and_line (void) { - current_source_symtab = 0; - current_source_line = 0; + internal_set_current_sal (nullptr, nullptr, 0); } /* Set the source file default for the "list" command to be S. @@ -252,9 +270,7 @@ select_source_symtab (struct symtab *s) if (s) { - current_source_symtab = s; - current_source_line = 1; - current_source_pspace = SYMTAB_PSPACE (s); + internal_set_current_sal (SYMTAB_PSPACE (s), s, 1); return; } @@ -269,9 +285,8 @@ select_source_symtab (struct symtab *s) = decode_line_with_current_source (main_name (), DECODE_LINE_FUNFIRSTLINE); const symtab_and_line &sal = sals[0]; - current_source_pspace = sal.pspace; - current_source_symtab = sal.symtab; - current_source_line = std::max (sal.line - (lines_to_list - 1), 1); + internal_set_current_sal (sal.pspace, sal.symtab, + std::max (sal.line - (lines_to_list - 1), 1)); if (current_source_symtab) return; } @@ -279,7 +294,7 @@ select_source_symtab (struct symtab *s) /* Alright; find the last file in the symtab list (ignoring .h's and namespace symtabs). */ - current_source_line = 1; + struct symtab *symtab = current_source_symtab; ALL_FILETABS (ofp, cu, s) { @@ -288,12 +303,11 @@ select_source_symtab (struct symtab *s) if (!(len > 2 && (strcmp (&name[len - 2], ".h") == 0 || strcmp (name, "<>") == 0))) - { - current_source_pspace = current_program_space; - current_source_symtab = s; - } + symtab = s; } + internal_set_current_sal (current_program_space, symtab, 1); + if (current_source_symtab) return; @@ -302,7 +316,7 @@ select_source_symtab (struct symtab *s) if (ofp->sf) s = ofp->sf->qf->find_last_source_symtab (ofp); if (s) - current_source_symtab = s; + internal_set_current_sal (current_program_space, s, 1); } if (current_source_symtab) return; @@ -1255,8 +1269,7 @@ identify_source_line (struct symtab *s, int line, int mid_statement, annotate_source (s->fullname, line, s->line_charpos[line - 1], mid_statement, get_objfile_arch (SYMTAB_OBJFILE (s)), pc); - current_source_line = line; - current_source_symtab = s; + internal_set_current_sal (current_source_pspace, s, line); clear_lines_listed_range (); return 1; } @@ -1276,8 +1289,7 @@ print_source_lines_base (struct symtab *s, int line, int stopline, struct ui_out *uiout = current_uiout; /* Regardless of whether we can open the file, set current_source_symtab. */ - current_source_symtab = s; - current_source_line = line; + internal_set_current_sal (current_source_pspace, s, line); first_line_listed = line; /* If printing of source lines is disabled, just print file and line @@ -1606,7 +1618,9 @@ forward_search_command (const char *regex, int from_tty) /* Match! */ print_source_lines (current_source_symtab, line, line + 1, 0); set_internalvar_integer (lookup_internalvar ("_"), line); - current_source_line = std::max (line - lines_to_list / 2, 1); + internal_set_current_sal (current_source_pspace, + current_source_symtab, + std::max (line - lines_to_list / 2, 1)); return; } line++; @@ -1677,7 +1691,8 @@ reverse_search_command (const char *regex, int from_tty) /* Match! */ print_source_lines (current_source_symtab, line, line + 1, 0); set_internalvar_integer (lookup_internalvar ("_"), line); - current_source_line = std::max (line - lines_to_list / 2, 1); + internal_set_current_sal (current_source_pspace, current_source_symtab, + std::max (line - lines_to_list / 2, 1)); return; } line--; @@ -1911,7 +1926,6 @@ _initialize_source (void) { struct cmd_list_element *c; - current_source_symtab = 0; init_source_path (); /* The intention is to use POSIX Basic Regular Expressions. diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c index efa02e2f08a..7a55735d53a 100644 --- a/gdb/tui/tui-hooks.c +++ b/gdb/tui/tui-hooks.c @@ -48,6 +48,7 @@ #include "tui/tui-winsource.h" #include "gdb_curses.h" +#include "source.h" /* This redefines CTRL if it is not already defined, so it must come after terminal state releated include files like and @@ -107,6 +108,23 @@ tui_event_modify_breakpoint (struct breakpoint *b) tui_update_all_breakpoint_info (); } +static bool context_changed; +static bool sal_changed; + +static void +tui_sal_changed () +{ + context_changed = false; + sal_changed = true; +} + +static void +tui_context_changed (user_selected_what) +{ + context_changed = true; + sal_changed = false; +} + /* Refresh TUI's frame and register information. This is a hook intended to be used to update the screen after potential frame and register changes. @@ -126,6 +144,18 @@ tui_refresh_frame_and_register_information (int registers_too_p) target_terminal::scoped_restore_terminal_state term_state; target_terminal::ours_for_output (); + if (sal_changed) + { + sal_changed = false; + struct symtab_and_line sal = get_current_source_symtab_and_line (); + tui_update_source_windows_with_line (sal.symtab, sal.line); + return; + } + + if (!context_changed) + return; + context_changed = false; + fi = get_selected_frame (NULL); /* Ensure that symbols for this frame are read in. Also, determine the source language of this frame, and switch to it if @@ -271,4 +301,8 @@ _initialize_tui_hooks (void) { /* Install the permanent hooks. */ gdb::observers::new_objfile.attach (tui_new_objfile_hook); + gdb::observers::current_source_symtab_and_line_changed.attach + (tui_sal_changed); + gdb::observers::user_selected_context_changed.attach + (tui_context_changed); }