From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 46341 invoked by alias); 20 Aug 2019 15:57:04 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 46332 invoked by uid 89); 20 Aug 2019 15:57:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.6 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_STOCKGEN,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=hs X-HELO: mail-wr1-f67.google.com Received: from mail-wr1-f67.google.com (HELO mail-wr1-f67.google.com) (209.85.221.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 20 Aug 2019 15:57:01 +0000 Received: by mail-wr1-f67.google.com with SMTP id g17so12970492wrr.5 for ; Tue, 20 Aug 2019 08:57:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Lz2KRIaWTBcfKbQf/H/wIdEWYGeRoMFBEu+AnGwoWzk=; b=XI0ZKh+KJSYq+p0pzqt5yWig6FxJUaVpZ47lxSHIlW2stzT7ol6MZwMuhUJcnO9Dlw V68EgnRaI2ehNXuzI/b1l4ksgeYCi16weCUNZXWe73JShXsBpqltrRp386wwzXtgQHwy KerAQgfH0x0tuWEiAO9Qv+MasTUeEBGgki8ajyvOWUVqRf64+eJsP74qGKJjCywQrYAe VuKp8JNTbJsbjqXfMVmdXgtx8uABhE0EEkwgL4llBgGl3w8hS4jalaB4mGR7Plszwz/T JN32jWCj8hQ0Zh40nUPW4fkIwtZlYEYyim5pgQe72tywMI6CICujJqQSXqNEwR+TUrCm 76HQ== Return-Path: Received: from localhost (host86-128-12-79.range86-128.btcentralplus.com. [86.128.12.79]) by smtp.gmail.com with ESMTPSA id r190sm667138wmf.0.2019.08.20.08.56.50 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 20 Aug 2019 08:56:50 -0700 (PDT) Date: Tue, 20 Aug 2019 15:57:00 -0000 From: Andrew Burgess To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v2 6/8] Make current_source_* per-program-space Message-ID: <20190820155649.GD6076@embecosm.com> References: <20190801170412.5553-1-tromey@adacore.com> <20190801170412.5553-7-tromey@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190801170412.5553-7-tromey@adacore.com> X-Fortune: Journalism will kill you, but it will keep you alive while you're at it. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-08/txt/msg00430.txt.bz2 * Tom Tromey [2019-08-01 11:04:10 -0600]: > This changes current_source_symtab and current_source_line to be > per-program-space. This ensures that switching inferiors will > preserve the current "list" location for that inferior, and also > ensures that the default expression evaluation context always comes > with the current inferior. > > No test case, because the latter problem crops up with an existing > gdb.multi test case once this entire series has been applied. I can reproduce the failure with patches 1 -> 5 applied. I did try to come up with a test that would trigger this on HEAD, but I guess I'm missing something as everything seems fine. I spotted a few minor coding standard things when looking through. Thanks, Andrew > > gdb/ChangeLog > 2019-08-01 Tom Tromey > > * source.c (struct current_source_location): New. > (current_source_key): New global. > (current_source_symtab, current_source_line) > (current_source_pspace): Remove. > (get_source_location): New function. > (get_current_source_symtab_and_line) > (set_default_source_symtab_and_line) > (set_current_source_symtab_and_line) > (clear_current_source_symtab_and_line, select_source_symtab) > (info_source_command, print_source_lines_base) > (info_line_command, search_command_helper, _initialize_source): > Update. > --- > gdb/ChangeLog | 15 ++++++ > gdb/source.c | 128 ++++++++++++++++++++++++++++++-------------------- > 2 files changed, 93 insertions(+), 50 deletions(-) > > diff --git a/gdb/source.c b/gdb/source.c > index 1aef019da44..d20cdc37c91 100644 > --- a/gdb/source.c > +++ b/gdb/source.c > @@ -66,15 +66,20 @@ struct substitute_path_rule > > static struct substitute_path_rule *substitute_path_rules = NULL; > > -/* Symtab of default file for listing lines of. */ > +/* An instance of this is attached to each program space. */ > > -static struct symtab *current_source_symtab; > +struct current_source_location > +{ > + /* Symtab of default file for listing lines of. */ > + > + struct symtab *symtab = nullptr; > > -/* Default next line to list. */ > + /* Default next line to list. */ > > -static int current_source_line; > + int line = 0; > +}; > > -static struct program_space *current_source_pspace; > +static program_space_key current_source_key; > > /* Default number of lines to print with commands like "list". > This is based on guessing how many long (i.e. more than chars_per_line > @@ -162,6 +167,19 @@ get_lines_to_list (void) > return lines_to_list; > } > > +/* A helper to return the current source location object for PSPACE, > + creating it if it does not exist. */ > + > +static current_source_location * > +get_source_location (program_space *pspace) > +{ > + current_source_location *loc > + = current_source_key.get (pspace); > + if (loc == nullptr) > + loc = current_source_key.emplace (pspace); > + return loc; > +} > + > /* Return the current source file for listing and next line to list. > NOTE: The returned sal pc and end fields are not valid. */ > > @@ -169,10 +187,11 @@ struct symtab_and_line > get_current_source_symtab_and_line (void) > { > symtab_and_line cursal; > + current_source_location *loc = get_source_location (current_program_space); > > - cursal.pspace = current_source_pspace; > - cursal.symtab = current_source_symtab; > - cursal.line = current_source_line; > + cursal.pspace = current_program_space; > + cursal.symtab = loc->symtab; > + cursal.line = loc->line; > cursal.pc = 0; > cursal.end = 0; > > @@ -194,7 +213,8 @@ set_default_source_symtab_and_line (void) > error (_("No symbol table is loaded. Use the \"file\" command.")); > > /* Pull in a current source symtab if necessary. */ > - if (current_source_symtab == 0) > + current_source_location *loc = get_source_location (current_program_space); > + if (loc->symtab == 0) > select_source_symtab (0); Could this become: if (loc->symtab == nullptr) select_source_symtab (nullptr); > } > > @@ -208,15 +228,16 @@ set_current_source_symtab_and_line (const symtab_and_line &sal) > { > symtab_and_line cursal; > > - cursal.pspace = current_source_pspace; > - cursal.symtab = current_source_symtab; > - cursal.line = current_source_line; > + current_source_location *loc = get_source_location (sal.pspace); > + > + cursal.pspace = sal.pspace; > + cursal.symtab = loc->symtab; > + cursal.line = loc->line; > cursal.pc = 0; > cursal.end = 0; > > - current_source_pspace = sal.pspace; > - current_source_symtab = sal.symtab; > - current_source_line = sal.line; > + loc->symtab = sal.symtab; > + loc->line = sal.line; > > /* Force the next "list" to center around the current line. */ > clear_lines_listed_range (); > @@ -229,8 +250,9 @@ 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; > + current_source_location *loc = get_source_location (current_program_space); > + loc->symtab = 0; nullptr again? > + loc->line = 0; > } > > /* See source.h. */ > @@ -240,13 +262,15 @@ select_source_symtab (struct symtab *s) > { > if (s) > { > - current_source_symtab = s; > - current_source_line = 1; > - current_source_pspace = SYMTAB_PSPACE (s); > + current_source_location *loc > + = get_source_location (SYMTAB_PSPACE (s)); > + loc->symtab = s; > + loc->line = 1; > return; > } > > - if (current_source_symtab) > + current_source_location *loc = get_source_location (current_program_space); > + if (loc->symtab) > return; I think this is supposed to be: if (loc->symtab != nullptr) return; > > /* Make the default place to list be the function `main' > @@ -255,16 +279,15 @@ select_source_symtab (struct symtab *s) > if (bsym.symbol != nullptr && SYMBOL_CLASS (bsym.symbol) == LOC_BLOCK) > { > symtab_and_line sal = find_function_start_sal (bsym.symbol, true); > - current_source_pspace = sal.pspace; > - current_source_symtab = sal.symtab; > - current_source_line = std::max (sal.line - (lines_to_list - 1), 1); > + loc->symtab = sal.symtab; > + loc->line = std::max (sal.line - (lines_to_list - 1), 1); > return; > } > > /* Alright; find the last file in the symtab list (ignoring .h's > and namespace symtabs). */ > > - current_source_line = 1; > + loc->line = 1; > > for (objfile *ofp : current_program_space->objfiles ()) > { > @@ -277,15 +300,12 @@ 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 = symtab; > - } > + loc->symtab = symtab; > } > } > } > > - if (current_source_symtab) > + if (loc->symtab) > return; Same again. > > for (objfile *objfile : current_program_space->objfiles ()) > @@ -293,9 +313,9 @@ select_source_symtab (struct symtab *s) > if (objfile->sf) > s = objfile->sf->qf->find_last_source_symtab (objfile); > if (s) > - current_source_symtab = s; > + loc->symtab = s; > } > - if (current_source_symtab) > + if (loc->symtab) > return; And again. > > error (_("Can't find a default source file")); > @@ -624,7 +644,9 @@ add_path (const char *dirname, char **which_path, int parse_separators) > static void > info_source_command (const char *ignore, int from_tty) > { > - struct symtab *s = current_source_symtab; > + current_source_location *loc > + = get_source_location (current_program_space); > + struct symtab *s = loc->symtab; > struct compunit_symtab *cust; > > if (!s) > @@ -1222,8 +1244,11 @@ 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; > + current_source_location *loc > + = get_source_location (current_program_space); > + > + loc->symtab = s; > + loc->line = line; > first_line_listed = line; > > /* If printing of source lines is disabled, just print file and line > @@ -1313,13 +1338,13 @@ print_source_lines_base (struct symtab *s, int line, int stopline, > { > char buf[20]; > > - last_line_listed = current_source_line; > + last_line_listed = loc->line; > if (flags & PRINT_SOURCE_LINES_FILENAME) > { > uiout->text (symtab_to_filename_for_display (s)); > uiout->text (":"); > } > - xsnprintf (buf, sizeof (buf), "%d\t", current_source_line++); > + xsnprintf (buf, sizeof (buf), "%d\t", loc->line++); > uiout->text (buf); > > while (*iter != '\0') > @@ -1411,12 +1436,14 @@ info_line_command (const char *arg, int from_tty) > > if (arg == 0) > { > - curr_sal.symtab = current_source_symtab; > + current_source_location *loc > + = get_source_location (current_program_space); > + curr_sal.symtab = loc->symtab; > curr_sal.pspace = current_program_space; > if (last_line_listed != 0) > curr_sal.line = last_line_listed; > else > - curr_sal.line = current_source_line; > + curr_sal.line = loc->line; > > sals = curr_sal; > } > @@ -1518,23 +1545,25 @@ search_command_helper (const char *regex, int from_tty, bool forward) > if (msg) > error (("%s"), msg); > > - if (current_source_symtab == 0) > + current_source_location *loc > + = get_source_location (current_program_space); > + if (loc->symtab == 0) > select_source_symtab (0); Update 0 to nullptr. > > - scoped_fd desc (open_source_file_with_line_charpos (current_source_symtab)); > + scoped_fd desc (open_source_file_with_line_charpos (loc->symtab)); > if (desc.get () < 0) > - perror_with_name (symtab_to_filename_for_display (current_source_symtab)); > + perror_with_name (symtab_to_filename_for_display (loc->symtab)); > > int line = (forward > ? last_line_listed + 1 > : last_line_listed - 1); > > - if (line < 1 || line > current_source_symtab->nlines) > + if (line < 1 || line > loc->symtab->nlines) > error (_("Expression not found")); > > - if (lseek (desc.get (), current_source_symtab->line_charpos[line - 1], 0) > + if (lseek (desc.get (), loc->symtab->line_charpos[line - 1], 0) > < 0) > - perror_with_name (symtab_to_filename_for_display (current_source_symtab)); > + perror_with_name (symtab_to_filename_for_display (loc->symtab)); > > gdb_file_up stream = desc.to_file (FDOPEN_MODE); > clearerr (stream.get ()); > @@ -1569,9 +1598,9 @@ search_command_helper (const char *regex, int from_tty, bool forward) > if (re_exec (buf.data ()) > 0) > { > /* Match! */ > - print_source_lines (current_source_symtab, line, line + 1, 0); > + print_source_lines (loc->symtab, line, line + 1, 0); > set_internalvar_integer (lookup_internalvar ("_"), line); > - current_source_line = std::max (line - lines_to_list / 2, 1); > + loc->line = std::max (line - lines_to_list / 2, 1); > return; > } > > @@ -1583,10 +1612,10 @@ search_command_helper (const char *regex, int from_tty, bool forward) > if (line < 1) > break; > if (fseek (stream.get (), > - current_source_symtab->line_charpos[line - 1], 0) < 0) > + loc->symtab->line_charpos[line - 1], 0) < 0) > { > const char *filename > - = symtab_to_filename_for_display (current_source_symtab); > + = symtab_to_filename_for_display (loc->symtab); > perror_with_name (filename); > } > } > @@ -1850,7 +1879,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. > -- > 2.20.1 >