This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 6/8] Make current_source_* per-program-space
* 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
>