Message ID | 20190801170412.5553-7-tromey@adacore.com |
---|---|
State | New |
Headers | show |
* 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 >
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
Andrew> I spotted a few minor coding standard things when looking through.
Thanks. I've fixed these.
Tom
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); } @@ -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; + 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; /* 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; 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; 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); - 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.