From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Tom Tromey <tromey@adacore.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v2 6/8] Make current_source_* per-program-space
Date: Tue, 20 Aug 2019 15:57:00 -0000 [thread overview]
Message-ID: <20190820155649.GD6076@embecosm.com> (raw)
In-Reply-To: <20190801170412.5553-7-tromey@adacore.com>
* Tom Tromey <tromey@adacore.com> [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 <tromey@adacore.com>
>
> * 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_location> 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, "<<C++-namespaces>>") == 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
>
next prev parent reply other threads:[~2019-08-20 15:57 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-01 17:04 [PATCH v2 0/8] Handle copy relocations and add $_ada_exception Tom Tromey
2019-08-01 17:04 ` [PATCH v2 3/8] Back out earlier Ada exception change Tom Tromey
2019-08-01 17:04 ` [PATCH v2 6/8] Make current_source_* per-program-space Tom Tromey
2019-08-20 15:57 ` Andrew Burgess [this message]
2019-08-21 19:05 ` Tom Tromey
2019-08-01 17:04 ` [PATCH v2 7/8] Add $_ada_exception convenience variable Tom Tromey
2019-08-01 17:56 ` Eli Zaretskii
2019-09-20 19:16 ` Tom Tromey
2019-08-27 9:47 ` Andrew Burgess
2019-09-20 19:15 ` Tom Tromey
2019-08-01 17:04 ` [PATCH v2 2/8] Handle copy relocations Tom Tromey
2019-08-21 15:35 ` Andrew Burgess
2019-08-21 19:11 ` Tom Tromey
2019-08-22 8:41 ` Andrew Burgess
2019-09-19 17:45 ` Tom Tromey
2019-09-19 18:28 ` Tom Tromey
2019-09-19 18:40 ` Andrew Burgess
2019-08-01 17:04 ` [PATCH v2 8/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol Tom Tromey
2019-08-20 13:41 ` Andrew Burgess
2019-08-21 14:35 ` Andrew Burgess
2019-08-01 17:04 ` [PATCH v2 1/8] Change SYMBOL_VALUE_ADDRESS to be an rvalue Tom Tromey
2019-08-01 17:12 ` [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal Tom Tromey
2019-08-21 10:32 ` Andrew Burgess
2019-08-27 9:54 ` Andrew Burgess
2019-08-27 18:17 ` Christian Biesinger via gdb-patches
2019-08-28 12:34 ` Andrew Burgess
2019-08-01 17:12 ` [PATCH v2 5/8] Don't call decode_line_with_current_source from select_source_symtab Tom Tromey
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190820155649.GD6076@embecosm.com \
--to=andrew.burgess@embecosm.com \
--cc=gdb-patches@sourceware.org \
--cc=tromey@adacore.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).