[RFA,2/2] PR cli/19551 - change formatting of "Reading symbols" messages

Message ID 20170413041504.14435-3-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey April 13, 2017, 4:15 a.m. UTC
  From: Tom Tromey <tromey@redhat.com>

PR cli/19551 notes that the "Reading symbols" messages can be messy, for
example:

(gdb) file /bin/gdb
Reading symbols from /bin/gdb...Reading symbols from /bin/gdb...(no debugging symbols found)...done.
(no debugging symbols found)...done.

In this case the first message is being interrupted by the message for
the minidebug info; then the subsequent output is emitted strangely.

This patch changes gdb to use a progress bar when reading debug info.
It modifies the DWARF reader(s) to update the progress.  Any printing
is deferred until the first progress report, so the messages no longer
clash.

While printing the status message it looks like:

Reading symbols from ./gdb
[##############                                                  ]

The "#"s show the progress; these are only printed on a terminal.

When it is finished it looks like:

Reading symbols from .gnu_debugdata for /usr/bin/gdb
Reading symbols from /bin/gdb

I made the MI implementation do nothing.  MI has a
"status-async-output" production in the grammar:

'STATUS-ASYNC-OUTPUT ==>'
     '[ TOKEN ] "+" ASYNC-OUTPUT NL'

... which maybe could be used for this sort of thing.  Currently I think
it's only used for "load" progress (see mi_load_progress); so it wasn't
clear to me whether this would be a good idea.

gdb/ChangeLog
2017-04-12  Tom Tromey  <tom@tromey.com>

	PR cli/19551:
	* utils.h (get_chars_per_line): Declare.
	* utils.c (get_chars_per_line): New function.
	* ui-out.h (ui_out::progress_meter): New class.
	(ui_out::progress, ui_out::do_progress_start)
	(ui_out::do_progress_notify, ui_out::do_progress_end): New
	methods.
	* ui-out.c (do_progress_end)
	(make_cleanup_ui_out_progress_begin_end, ui_out_progress): New
	functions.
	* symfile.c (symbol_file_add_with_addrs): Create a
	progress_meter.
	(symbol_file_add_with_addrs): Likewise.
	* psymtab.c (require_partial_symbols): Create a progress_meter.
	* mi/mi-out.h (mi_ui_out::do_progress_start)
	(mi_ui_out::do_progress_notify, mi_ui_out::do_progress_end): New
	methods.
	* dwarf2read.c (create_cus_from_index_list): Call
	ui_out::progress.  Add total_progress_steps argument.
	(create_cus_from_index): Add total_progress_steps argument.
	(create_signatured_type_table_from_index): Update.  Call
	ui_out::progress.
	(dwarf2_read_index): Update.
	(dw2_expand_all_symtabs, dwarf2_build_psymtabs_hard): Call
	ui_out::progress.
	* cli-out.h (struct cli_ui_out) <do_progress_start,
	do_progress_notify, do_progress_end>: New methods.
	<enum meter_stat, struct cli_progress_info>: New.
	<m_meters>: New member.
	* cli-out.c (cli_ui_out::do_progress_start)
	(cli_ui_out::do_progress_notify, cli_ui_out::do_progress_end): New
	methods.

gdb/testsuite/ChangeLog
2017-04-12  Tom Tromey  <tom@tromey.com>

	PR cli/19551:
	* lib/mi-support.exp (mi_gdb_file_cmd): Update regexps.
	* lib/gdb.exp (gdb_file_cmd): Update regexps.
	* gdb.stabs/weird.exp: Update regexp.
	* gdb.server/solib-list.exp: Update regexp.
	* gdb.linespec/linespec.exp: Update regexp.
	* gdb.multi/remove-inferiors.exp (test_remove_inferiors): Update
	regexp.
	* gdb.dwarf2/dw2-stack-boundary.exp: Update regexp.
	* gdb.dwarf2/dw2-objfile-overlap.exp: Update regexp.
	* gdb.cp/cp-relocate.exp: Update regexp.
	* gdb.base/sym-file.exp: Update regexps.
	* gdb.base/sepdebug.exp: Update regexps.
	* gdb.base/relocate.exp: Update regexps.
	* gdb.base/print-symbol-loading.exp (test_load_core): Update regexp.
	* gdb.base/kill-detach-inferiors-cmd.exp: Update regexp.
	* gdb.base/dbx.exp (gdb_file_cmd): Update regexp.
	* gdb.base/code_elim.exp: Update regexp.
	* gdb.base/break-unload-file.exp (test_break): Update regexp.
	* gdb.base/break-interp.exp (test_attach_gdb): Update regexp.
	* gdb.base/break-idempotent.exp (force_breakpoint_re_set): Update
	regexp.
	* gdb.base/attach.exp (do_attach_tests): Update regexp.
	* gdb.python/py-section-script.exp: Update regexps.
---
 gdb/ChangeLog                                      | 35 +++++++++
 gdb/cli-out.c                                      | 85 ++++++++++++++++++++++
 gdb/cli-out.h                                      | 29 ++++++++
 gdb/dwarf2read.c                                   | 39 +++++++---
 gdb/mi/mi-out.h                                    | 12 +++
 gdb/psymtab.c                                      | 20 ++---
 gdb/symfile.c                                      | 82 +++++++++++----------
 gdb/testsuite/ChangeLog                            | 27 +++++++
 gdb/testsuite/gdb.base/attach.exp                  | 10 +--
 gdb/testsuite/gdb.base/break-idempotent.exp        |  2 +-
 gdb/testsuite/gdb.base/break-interp.exp            |  2 +-
 gdb/testsuite/gdb.base/break-unload-file.exp       |  2 +-
 gdb/testsuite/gdb.base/code_elim.exp               | 10 +--
 gdb/testsuite/gdb.base/dbx.exp                     |  2 +-
 .../gdb.base/kill-detach-inferiors-cmd.exp         |  2 +-
 gdb/testsuite/gdb.base/print-symbol-loading.exp    |  2 +-
 gdb/testsuite/gdb.base/relocate.exp                |  4 +-
 gdb/testsuite/gdb.base/sepdebug.exp                |  2 +-
 gdb/testsuite/gdb.base/sym-file.exp                |  4 +-
 gdb/testsuite/gdb.cp/cp-relocate.exp               |  2 +-
 gdb/testsuite/gdb.dwarf2/dw2-objfile-overlap.exp   |  2 +-
 gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp    |  2 +-
 gdb/testsuite/gdb.linespec/linespec.exp            |  2 +-
 gdb/testsuite/gdb.multi/remove-inferiors.exp       |  2 +-
 gdb/testsuite/gdb.python/py-section-script.exp     |  2 +-
 gdb/testsuite/gdb.server/solib-list.exp            |  2 +-
 gdb/testsuite/gdb.stabs/weird.exp                  |  2 +-
 gdb/testsuite/lib/gdb.exp                          |  8 +-
 gdb/testsuite/lib/mi-support.exp                   |  4 +-
 gdb/ui-out.h                                       | 36 +++++++++
 gdb/utils.c                                        |  8 ++
 gdb/utils.h                                        |  4 +
 32 files changed, 353 insertions(+), 94 deletions(-)
  

Comments

Pedro Alves April 18, 2017, 5:42 p.m. UTC | #1
On 04/13/2017 05:15 AM, Tom Tromey wrote:
> From: Tom Tromey <tromey@redhat.com>
> 
> PR cli/19551 notes that the "Reading symbols" messages can be messy, for
> example:
> 
> (gdb) file /bin/gdb
> Reading symbols from /bin/gdb...Reading symbols from /bin/gdb...(no debugging symbols found)...done.
> (no debugging symbols found)...done.
> 
> In this case the first message is being interrupted by the message for
> the minidebug info; then the subsequent output is emitted strangely.
> 
> This patch changes gdb to use a progress bar when reading debug info.

Nice!

> It modifies the DWARF reader(s) to update the progress.  

What does GDB show with the reader?  Is their output now broken, same,
etc?

> Any printing
> is deferred until the first progress report, so the messages no longer
> clash.

Can you clarify what printing you're referring to?
What happens if gdb emits e.g., complaints while a progress bar
is half full?

> 
> While printing the status message it looks like:
> 
> Reading symbols from ./gdb
> [##############                                                  ]
> 
> The "#"s show the progress; these are only printed on a terminal.
> 
> When it is finished it looks like:
> 
> Reading symbols from .gnu_debugdata for /usr/bin/gdb
> Reading symbols from /bin/gdb

I played with this a bit and it looks like a real nice improvement
to me.

I noticed that if you clear the screen such that the prompt is not at
the bottom when gdb reads symbols (e.g., start gdb on itself,
run to main, do ctrl-l to clear the screen, and do
"nosharedlibrary / sharedlibrary"), and if symbol reading is quick, then
the meter appears/disappears very quickly in what looks like an
odd flash.  It looks a tiny bit annoying to me.  Though not a deal breaker
and I'll get used to it for sure.  One way around it would be to
not print the meter unless the operation is taking longer than some
minimum time, like 1s or some such.

> 
> I made the MI implementation do nothing.  MI has a
> "status-async-output" production in the grammar:
> 
> 'STATUS-ASYNC-OUTPUT ==>'
>      '[ TOKEN ] "+" ASYNC-OUTPUT NL'
> 
> ... which maybe could be used for this sort of thing.  Currently I think
> it's only used for "load" progress (see mi_load_progress); so it wasn't
> clear to me whether this would be a good idea.

I think it'd be a good idea, but of course you don't have to do
it yourself.

Looks like the testsuite changes needs some more work.  I thought
I'd see how it looks, and ran (only) the gdb.base/attach.exp test,
and got:

 Running rc/gdb/testsuite/gdb.base/attach.exp ...
 ERROR: internal buffer is full.

I'm attaching the resulting gdb.log.


> 
> gdb/ChangeLog
> 2017-04-12  Tom Tromey  <tom@tromey.com>

(I think the correct thing to do wrt to authorship/copyright
is add both email addresses as multiple authors.  See e.g.,
commit 3e29f34a4eef.)
> index 2a59869..e99fce2 100644
> --- a/gdb/cli-out.c
> +++ b/gdb/cli-out.c
> @@ -236,6 +236,91 @@ cli_ui_out::do_redirect (ui_file *outstream)
>      m_streams.pop_back ();
>  }
>  
> +void
> +cli_ui_out::do_progress_start (const std::string &name, int should_print)

bool for "should_print" ?

> +{
> +  struct ui_file *stream = m_streams.back ();
> +  cli_progress_info meter;
> +
> +  meter.last_value = 0;

How about adding a ctor like this:

    cli_progress_info (meter_state printing_,
		       const std::string &name_)
      : printing (printing_),
        name (name_),
	last_value (0)
    {}

and then calling emplace_back further below.  You'd
need a:

  meter_state printing;

temporary ...

> +  meter.name = name;
> +  if (!ui_file_isatty (stream))
> +    {
> +      fprintf_unfiltered (stream, "%s...", meter.name.c_str ());
> +      gdb_flush (stream);

(Note these could use the isatty/printf/flush methods of ui_file.
Likewise throughout.)

> +      meter.printing = WORKING;

... for these, of course.

> +    }
> +  else
> +    {
> +      /* Don't actually emit anything until the first call notify us
> +	 of progress.  This makes it so a second progress message can
> +	 be started before the first one has been notified, without
> +	 messy output.  */
> +      meter.printing = should_print ? START : NO_PRINT;

I find the START etc names to bit too generic to be directly
in cli_ui_out scope.  I think they should be either METER_START etc.,
or meter_state should be an enum class, or meter_state stays a
regular enum but is moved to cli_progress_info, perhaps.

> +    }
> +
> +  m_meters.push_back (meter);
> +}
> +
> +void
> +cli_ui_out::do_progress_notify (double howmuch)
> +{
> +  struct ui_file *stream = m_streams.back ();

We've dropped most of the redundant "struct" in "struct ui_file"
in cli-out.c/h recently.  Let's not add it back in new code.

> +  cli_progress_info &meter (m_meters.back ());

Below you've written this as:

    cli_progress_info &meter = m_meters.back ();

I prefer the latter using =, as it's easier to reason
about.

> +
> +  if (meter.printing == NO_PRINT)
> +    return;
> +
> +  if (meter.printing == START)
> +    {
> +      fprintf_unfiltered (stream, "%s\n", meter.name.c_str ());
> +      gdb_flush (stream);
> +      meter.printing = WORKING;
> +    }
> +
> +  if (ui_file_isatty (stream))
> +    {
> +      int i, max;
> +      int width = get_chars_per_line () - 3;
> +
> +      max = width * howmuch;
> +      fprintf_unfiltered (stream, "\r[");
> +      for (i = 0; i < width; ++i)

These can now be:

 int max = ...
 for (int i = 0; ...

etc., likewise throughout.

> +	fprintf_unfiltered (stream, i < max ? "#" : " ");
> +      fprintf_unfiltered (stream, "]");
> +      gdb_flush (stream);
> +    }
> +}
> +


>    std::vector<ui_file *> m_streams;
>    bool m_suppress_output;
> +
> +  /* Represents the printing state of a progress meter.  */
> +  enum meter_state
> +  {
> +    /* Printing will start with the next output.  */
> +    START,
> +    /* Printing has already started.  */
> +    WORKING,
> +    /* Printing should not be done.  */
> +    NO_PRINT
> +  };
> +
> +  /* The state of a recent progress meter.  */
> +  struct cli_progress_info
> +  {
> +    /* The current state.  */
> +    enum meter_state printing;
> +    /* The name to print.  */
> +    std::string name;
> +    /* The last notification value.  */
> +    double last_value;
> +  };
> +
> +  /* Stack of progress meters.  */
> +  std::vector<cli_progress_info> m_meters;

I think it'd be good to have this comment expanded a bit to
explain why we need a stack, and what happens when we're
already printing a meter and a new stack level appears.

>  };
>  
>  extern cli_ui_out *cli_out_new (struct ui_file *stream);
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index b58d0fc..7d99c99 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c

Could you add some comments about what is considered a "step"
here, how the number of total progress steps is determined, etc.?

> diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
> index fea94f2..490c06a 100644
> --- a/gdb/mi/mi-out.h
> +++ b/gdb/mi/mi-out.h
> @@ -72,6 +72,18 @@ protected:
>    virtual bool do_is_mi_like_p () override
>    { return true; }
>  
> +  virtual void do_progress_start (const std::string &, int) override
> +  {
> +  }
> +
> +  virtual void do_progress_notify (double) override
> +  {
> +  }
> +
> +  virtual void do_progress_end () override
> +  {
> +  }

If implementations can do nothing, shouldn't "nothing" be the
default implementation?

> @@ -78,23 +79,22 @@ require_partial_symbols (struct objfile *objfile, int verbose)
>  
>        if (objfile->sf->sym_read_psymbols)
>  	{
> +	  gdb::optional<ui_out::progress_meter> meter;
> +
>  	  if (verbose)
>  	    {
> -	      printf_unfiltered (_("Reading symbols from %s..."),
> -				 objfile_name (objfile));
> -	      gdb_flush (gdb_stdout);
> +	      std::string text = (std::string ("Reading symbols from ")
> +				  + objfile_name (objfile));
> +
> +	      meter.emplace (current_uiout, text, verbose);

It looked to me that the "progress text" string is always a 
discardable string that could be moved all the way to meter.name
instead of duped.  I.e., here you'd have:

     meter.emplace (current_uiout, std::move (text), verbose);

> +  {
> +    std::string progress_text;
> +
> +    if (should_print)
> +      {
> +	if (deprecated_pre_add_symbol_hook)
> +	  deprecated_pre_add_symbol_hook (name);
> +
> +	progress_text = std::string ("Reading symbols from ") + name;
> +      }
> +    else
> +      progress_text = "";

This else branch is unnecessary.  strings are empty by default.

> +
> +    ui_out::progress_meter meter (current_uiout, progress_text, should_print);

   ui_out::progress_meter meter (current_uiout, 
                                 std::move (progress_text), 
                                 should_print);

Etc (re. move).  Of course, the progress_meter prototype
would be adjusted accordingly.


> +    syms_from_objfile (objfile, addrs, add_flags);
> +  }
>  
>    /* We now have at least a partial symbol table.  Check to see if the
>       user requested that all symbols be read on initial access via either
>       the gdb startup command line or on a per symbol file basis.  Expand
>       all partial symbol tables for this objfile if so.  */
>  
> -  if ((flags & OBJF_READNOW))
> -    {
> -      if (should_print)
> -	{
> -	  printf_unfiltered (_("expanding to full symbols..."));
> -	  wrap_here ("");
> -	  gdb_flush (gdb_stdout);
> -	}
> +  {
> +    if ((flags & OBJF_READNOW))
> +      {
> +	std::string progress_text;
>  
> -      if (objfile->sf)
> -	objfile->sf->qf->expand_all_symtabs (objfile);
> -    }
> +	if (should_print)
> +	  progress_text = std::string ("Expanding full symbols for ") + name;
> +	else
> +	  progress_text = "";
>  
> -  if (should_print && !objfile_has_symbols (objfile))
> -    {
> -      wrap_here ("");
> -      printf_unfiltered (_("(no debugging symbols found)..."));
> -      wrap_here ("");
> -    }
> +	ui_out::progress_meter meter (current_uiout, progress_text,
> +				      should_print);
> +
> +	if (objfile->sf)
> +	  objfile->sf->qf->expand_all_symtabs (objfile);
> +      }
> +  }
>  
>    if (should_print)
>      {
> +      if (!objfile_has_symbols (objfile))
> +	printf_unfiltered (_(" (no debugging symbols found)\n"));
> +
>        if (deprecated_post_add_symbol_hook)
>  	deprecated_post_add_symbol_hook ();
> -      else
> -	printf_unfiltered (_("done.\n"));
>      }
>  
>    /* We print some messages regardless of whether 'from_tty ||
> @@ -2483,10 +2486,6 @@ reread_symbols (void)
>  	  struct cleanup *old_cleanups;
>  	  struct section_offsets *offsets;
>  	  int num_offsets;
> -	  char *original_name;
> -
> -	  printf_unfiltered (_("`%s' has changed; re-reading symbols.\n"),
> -			     objfile_name (objfile));
>  
>  	  /* There are various functions like symbol_file_add,
>  	     symfile_bfd_open, syms_from_objfile, etc., which might
> @@ -2502,6 +2501,11 @@ reread_symbols (void)
>  	  /* We need to do this whenever any symbols go away.  */
>  	  make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
>  
> +	  std::string text
> +	    = (std::string (_("`%s' has changed; re-reading symbols.\n"))
> +	       + objfile_name (objfile));

This transformation doesn't look right.  Note the %s.

> +	  ui_out::progress_meter meter (current_uiout, text, 1);
> +
>  	  if (exec_bfd != NULL
>  	      && filename_cmp (bfd_get_filename (objfile->obfd),
>  			       bfd_get_filename (exec_bfd)) == 0)
> @@ -2548,8 +2552,7 @@ reread_symbols (void)
>  	      error (_("Can't open %s to read symbols."), obfd_filename);
>  	  }
>  


> +/* See utils.h.  */
> +
> +int
> +get_chars_per_line (void)

You can drop the "void".

> +{
> +  return chars_per_line;
> +}
> +

> +/* Return the number of characters in a line.  */
> +
> +extern int get_chars_per_line (void);
> +

Ditto.

Thanks,
Pedro Alves
  
Pedro Alves April 18, 2017, 5:44 p.m. UTC | #2
On 04/18/2017 06:42 PM, Pedro Alves wrote:
>> It modifies the DWARF reader(s) to update the progress.  
> What does GDB show with the reader?  Is their output now broken, same,
> etc?
> 

Sorry, I meant, what does GDB show with the other readers.

Thanks,
Pedro Alves
  
Tom Tromey April 20, 2017, 12:11 a.m. UTC | #3
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> It modifies the DWARF reader(s) to update the progress.  

Pedro> What does GDB show with the reader?  Is their output now broken, same,
Pedro> etc?
Pedro> Sorry, I meant, what does GDB show with the other readers.

I didn't update these, so it just prints, e.g.:

Reading symbols from outputs/gdb.stabs/weird/weirdx.o

I could take a look at the others I suppose.

>> Any printing is deferred until the first progress report, so the
>> messages no longer clash.

Pedro> Can you clarify what printing you're referring to?

This bit meant that the "Reading ..." message isn't emitted until the
first progress update.  This is what makes it so these messages don't
clash.

Pedro> What happens if gdb emits e.g., complaints while a progress bar
Pedro> is half full?

You get terrible-looking stuff like:

(gdb) file ../gdb
Reading symbols from ../gdb
[###################################                                          ]unsupported stack op: 'DW_OP_stack_value'...unsupported stack op: 'DW_OP_stack_va[####################################                                         ]u[####################################                                         ]u[######################################                                       ]u[##############################################                               ]unsupported stack op: 'DW_OP_stack_value'...unsupported stack op: 'DW_OP_stack_value'...unsupported stack op: 'DW_OP_stack_value'...unsupported stack op: 'DW_OP_[##############################################################               ]unsupported stack op: 'DW_OP_stack_value'...unsupported stack op: 'DW_OP_stack_va                                                                               


Maybe there's no easy way to improve this :( The issue is that the
complaint system just writes to the ui-file, bypassing ui-out entirely.
Complaints could be fixed, but this same problem would apply to any
possible output during symbol reading.

Though FWIW I don't think anybody other than gdb developers enables
complaints.

Pedro> I noticed that if you clear the screen such that the prompt is not at
Pedro> the bottom when gdb reads symbols (e.g., start gdb on itself,
Pedro> run to main, do ctrl-l to clear the screen, and do
Pedro> "nosharedlibrary / sharedlibrary"), and if symbol reading is quick, then
Pedro> the meter appears/disappears very quickly in what looks like an
Pedro> odd flash.  It looks a tiny bit annoying to me.  Though not a deal breaker
Pedro> and I'll get used to it for sure.  One way around it would be to
Pedro> not print the meter unless the operation is taking longer than some
Pedro> minimum time, like 1s or some such.

I'm wondering if you are seeing the case where the first notification is
at howmuch=1.0, causing printing of the progress bar followed by
immediately erasing it.

I can't see the effect myself, but that's an idea... I changed it to
avoid this case specifically.

>> +  /* Stack of progress meters.  */
>> +  std::vector<cli_progress_info> m_meters;

Pedro> I think it'd be good to have this comment expanded a bit to
Pedro> explain why we need a stack, and what happens when we're
Pedro> already printing a meter and a new stack level appears.

This case also doesn't really work.  If a meter is already printing, a
new meter will just confuse the output.  The current system relies on
the way that gdb does symbol reading, where it discovers separate
debuginfo before doing any work on the original objfile.

Pedro> If implementations can do nothing, shouldn't "nothing" be the
Pedro> default implementation?

I made the change; but I did it this way originally since the base class
looked purely abstract to me.

I made all the other changes.  I still didn't write the test for the
first patch, though, and I'm also experiencing a possible intermittent
with the buildbot (not sure, maybe it's my other patch series), so it
may be a little while before v2.

Tom
  
Simon Marchi April 27, 2017, 9:40 p.m. UTC | #4
On 2017-04-13 00:15, Tom Tromey wrote:
> From: Tom Tromey <tromey@redhat.com>
> 
> PR cli/19551 notes that the "Reading symbols" messages can be messy, 
> for
> example:
> 
> (gdb) file /bin/gdb
> Reading symbols from /bin/gdb...Reading symbols from /bin/gdb...(no
> debugging symbols found)...done.
> (no debugging symbols found)...done.
> 
> In this case the first message is being interrupted by the message for
> the minidebug info; then the subsequent output is emitted strangely.
> 
> This patch changes gdb to use a progress bar when reading debug info.
> It modifies the DWARF reader(s) to update the progress.  Any printing
> is deferred until the first progress report, so the messages no longer
> clash.
> 
> While printing the status message it looks like:
> 
> Reading symbols from ./gdb
> [##############                                                  ]
> 
> The "#"s show the progress; these are only printed on a terminal.
> 
> When it is finished it looks like:
> 
> Reading symbols from .gnu_debugdata for /usr/bin/gdb
> Reading symbols from /bin/gdb
> 
> I made the MI implementation do nothing.  MI has a
> "status-async-output" production in the grammar:
> 
> 'STATUS-ASYNC-OUTPUT ==>'
>      '[ TOKEN ] "+" ASYNC-OUTPUT NL'
> 
> ... which maybe could be used for this sort of thing.  Currently I 
> think
> it's only used for "load" progress (see mi_load_progress); so it wasn't
> clear to me whether this would be a good idea.

Hi Tom,

I think that having a progress bar would be really awesome (right after 
making GDB load debug info faster so fast that we don't need one).  
However, I think that feature would deserve a patch of its own with a 
more expressive title.  I am sure many people will be interested, but 
like this it's hidden under some obscure bug fix.

Would it be possible to make this patch a simple bug fix and put the 
progress bar in a patch of its own?

Thanks,

Simon
  
Tom Tromey April 27, 2017, 11:16 p.m. UTC | #5
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> However, I think that feature would deserve a patch of its own
Simon> with a more expressive title.  I am sure many people will be
Simon> interested, but like this it's hidden under some obscure bug fix.

I imagine most users don't read the history but rather would notice when
gdb's behavior changes.

Simon> Would it be possible to make this patch a simple bug fix and put the
Simon> progress bar in a patch of its own?

I guess it would require some delayed output machinery, like what is in
this patch.  I don't really see the benefit of doing this transform --
it seems like a good amount of work for the same result in the end.
However, I don't mind if you, or someone else, wants to take this over.

thanks,
Tom
  
Simon Marchi April 27, 2017, 11:53 p.m. UTC | #6
On 2017-04-27 19:16, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> However, I think that feature would deserve a patch of its own
> Simon> with a more expressive title.  I am sure many people will be
> Simon> interested, but like this it's hidden under some obscure bug 
> fix.
> 
> I imagine most users don't read the history but rather would notice 
> when
> gdb's behavior changes.

I was mostly thinking about people subscribed to this list.  Given the 
rather high volume, I assume that most people just read the titles and 
dig further if they are interested.  A feature such as this one would be 
of interest to many, I'm sure.
  
Tom Tromey May 29, 2017, 5:24 p.m. UTC | #7
Simon> Would it be possible to make this patch a simple bug fix and put the
Simon> progress bar in a patch of its own?

I'm finally admitting that I won't ever work up the motivation for this,
so I'm dropping this patch.

Tom
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c56de68..064ce21 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,6 +1,41 @@ 
 2017-04-12  Tom Tromey  <tom@tromey.com>
 
 	PR cli/19551:
+	* utils.h (get_chars_per_line): Declare.
+	* utils.c (get_chars_per_line): New function.
+	* ui-out.h (ui_out::progress_meter): New class.
+	(ui_out::progress, ui_out::do_progress_start)
+	(ui_out::do_progress_notify, ui_out::do_progress_end): New
+	methods.
+	* ui-out.c (do_progress_end)
+	(make_cleanup_ui_out_progress_begin_end, ui_out_progress): New
+	functions.
+	* symfile.c (symbol_file_add_with_addrs): Create a
+	progress_meter.
+	(symbol_file_add_with_addrs): Likewise.
+	* psymtab.c (require_partial_symbols): Create a progress_meter.
+	* mi/mi-out.h (mi_ui_out::do_progress_start)
+	(mi_ui_out::do_progress_notify, mi_ui_out::do_progress_end): New
+	methods.
+	* dwarf2read.c (create_cus_from_index_list): Call
+	ui_out::progress.  Add total_progress_steps argument.
+	(create_cus_from_index): Add total_progress_steps argument.
+	(create_signatured_type_table_from_index): Update.  Call
+	ui_out::progress.
+	(dwarf2_read_index): Update.
+	(dw2_expand_all_symtabs, dwarf2_build_psymtabs_hard): Call
+	ui_out::progress.
+	* cli-out.h (struct cli_ui_out) <do_progress_start,
+	do_progress_notify, do_progress_end>: New methods.
+	<enum meter_stat, struct cli_progress_info>: New.
+	<m_meters>: New member.
+	* cli-out.c (cli_ui_out::do_progress_start)
+	(cli_ui_out::do_progress_notify, cli_ui_out::do_progress_end): New
+	methods.
+
+2017-04-12  Tom Tromey  <tom@tromey.com>
+
+	PR cli/19551:
 	* symfile-add-flags.h (enum symfile_add_flags)
 	<SYMFILE_NOT_FILENAME>: New constant.
 	* symfile.c (read_symbols): Use SYMFILE_NOT_FILENAME.  Get
diff --git a/gdb/cli-out.c b/gdb/cli-out.c
index 2a59869..e99fce2 100644
--- a/gdb/cli-out.c
+++ b/gdb/cli-out.c
@@ -236,6 +236,91 @@  cli_ui_out::do_redirect (ui_file *outstream)
     m_streams.pop_back ();
 }
 
+void
+cli_ui_out::do_progress_start (const std::string &name, int should_print)
+{
+  struct ui_file *stream = m_streams.back ();
+  cli_progress_info meter;
+
+  meter.last_value = 0;
+  meter.name = name;
+  if (!ui_file_isatty (stream))
+    {
+      fprintf_unfiltered (stream, "%s...", meter.name.c_str ());
+      gdb_flush (stream);
+      meter.printing = WORKING;
+    }
+  else
+    {
+      /* Don't actually emit anything until the first call notify us
+	 of progress.  This makes it so a second progress message can
+	 be started before the first one has been notified, without
+	 messy output.  */
+      meter.printing = should_print ? START : NO_PRINT;
+    }
+
+  m_meters.push_back (meter);
+}
+
+void
+cli_ui_out::do_progress_notify (double howmuch)
+{
+  struct ui_file *stream = m_streams.back ();
+  cli_progress_info &meter (m_meters.back ());
+
+  if (meter.printing == NO_PRINT)
+    return;
+
+  if (meter.printing == START)
+    {
+      fprintf_unfiltered (stream, "%s\n", meter.name.c_str ());
+      gdb_flush (stream);
+      meter.printing = WORKING;
+    }
+
+  if (ui_file_isatty (stream))
+    {
+      int i, max;
+      int width = get_chars_per_line () - 3;
+
+      max = width * howmuch;
+      fprintf_unfiltered (stream, "\r[");
+      for (i = 0; i < width; ++i)
+	fprintf_unfiltered (stream, i < max ? "#" : " ");
+      fprintf_unfiltered (stream, "]");
+      gdb_flush (stream);
+    }
+}
+
+void
+cli_ui_out::do_progress_end ()
+{
+  struct ui_file *stream = m_streams.back ();
+  cli_progress_info &meter = m_meters.back ();
+
+  if (meter.printing != NO_PRINT)
+    {
+      if (ui_file_isatty (stream))
+	{
+	  int i;
+	  int width = get_chars_per_line () - 3;
+
+	  fprintf_unfiltered (stream, "\r");
+	  for (i = 0; i < width + 2; ++i)
+	    fprintf_unfiltered (stream, " ");
+	  fprintf_unfiltered (stream, "\r");
+	  gdb_flush (stream);
+	}
+      else
+	{
+	  fprintf_unfiltered (stream, "done.\n");
+	  gdb_flush (stream);
+	}
+    }
+
+  m_meters.pop_back ();
+}
+
 /* local functions */
 
 /* Like cli_ui_out::do_field_fmt, but takes a variable number of args
diff --git a/gdb/cli-out.h b/gdb/cli-out.h
index 1b6a1ad..51da43d 100644
--- a/gdb/cli-out.h
+++ b/gdb/cli-out.h
@@ -62,6 +62,10 @@  protected:
   virtual void do_flush () override;
   virtual void do_redirect (struct ui_file *outstream) override;
 
+  virtual void do_progress_start (const std::string &, int) override;
+  virtual void do_progress_notify (double) override;
+  virtual void do_progress_end () override;
+
   bool suppress_output ()
   { return m_suppress_output; }
 
@@ -73,6 +77,31 @@  private:
 
   std::vector<ui_file *> m_streams;
   bool m_suppress_output;
+
+  /* Represents the printing state of a progress meter.  */
+  enum meter_state
+  {
+    /* Printing will start with the next output.  */
+    START,
+    /* Printing has already started.  */
+    WORKING,
+    /* Printing should not be done.  */
+    NO_PRINT
+  };
+
+  /* The state of a recent progress meter.  */
+  struct cli_progress_info
+  {
+    /* The current state.  */
+    enum meter_state printing;
+    /* The name to print.  */
+    std::string name;
+    /* The last notification value.  */
+    double last_value;
+  };
+
+  /* Stack of progress meters.  */
+  std::vector<cli_progress_info> m_meters;
 };
 
 extern cli_ui_out *cli_out_new (struct ui_file *stream);
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index b58d0fc..7d99c99 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2940,7 +2940,8 @@  create_cus_from_index_list (struct objfile *objfile,
 			    const gdb_byte *cu_list, offset_type n_elements,
 			    struct dwarf2_section_info *section,
 			    int is_dwz,
-			    int base_offset)
+			    int base_offset,
+			    double total_progress_steps)
 {
   offset_type i;
 
@@ -2964,6 +2965,8 @@  create_cus_from_index_list (struct objfile *objfile,
 					struct dwarf2_per_cu_quick_data);
       the_cu->is_dwz = is_dwz;
       dwarf2_per_objfile->all_comp_units[base_offset + i / 2] = the_cu;
+
+      current_uiout->progress ((i + 2 * base_offset) / total_progress_steps);
     }
 }
 
@@ -2973,7 +2976,8 @@  create_cus_from_index_list (struct objfile *objfile,
 static void
 create_cus_from_index (struct objfile *objfile,
 		       const gdb_byte *cu_list, offset_type cu_list_elements,
-		       const gdb_byte *dwz_list, offset_type dwz_elements)
+		       const gdb_byte *dwz_list, offset_type dwz_elements,
+		       double total_progress_steps)
 {
   struct dwz_file *dwz;
 
@@ -2983,14 +2987,15 @@  create_cus_from_index (struct objfile *objfile,
 	       dwarf2_per_objfile->n_comp_units);
 
   create_cus_from_index_list (objfile, cu_list, cu_list_elements,
-			      &dwarf2_per_objfile->info, 0, 0);
+			      &dwarf2_per_objfile->info, 0, 0,
+			      total_progress_steps);
 
   if (dwz_elements == 0)
     return;
 
   dwz = dwarf2_get_dwz_file ();
   create_cus_from_index_list (objfile, dwz_list, dwz_elements, &dwz->info, 1,
-			      cu_list_elements / 2);
+			      cu_list_elements / 2, total_progress_steps);
 }
 
 /* Create the signatured type hash table from the index.  */
@@ -2999,7 +3004,9 @@  static void
 create_signatured_type_table_from_index (struct objfile *objfile,
 					 struct dwarf2_section_info *section,
 					 const gdb_byte *bytes,
-					 offset_type elements)
+					 offset_type elements,
+					 offset_type base_progress_step,
+					 double total_progress_steps)
 {
   offset_type i;
   htab_t sig_types_hash;
@@ -3044,6 +3051,8 @@  create_signatured_type_table_from_index (struct objfile *objfile,
       *slot = sig_type;
 
       dwarf2_per_objfile->all_type_units[i / 3] = sig_type;
+
+      current_uiout->progress ((base_progress_step + i) / total_progress_steps);
     }
 
   dwarf2_per_objfile->signatured_types = sig_types_hash;
@@ -3334,6 +3343,7 @@  dwarf2_read_index (struct objfile *objfile)
   const gdb_byte *cu_list, *types_list, *dwz_list = NULL;
   offset_type cu_list_elements, types_list_elements, dwz_list_elements = 0;
   struct dwz_file *dwz;
+  double total_progress_steps;
 
   if (!read_index_from_section (objfile, objfile_name (objfile),
 				use_deprecated_index_sections,
@@ -3368,8 +3378,11 @@  dwarf2_read_index (struct objfile *objfile)
 	}
     }
 
+  total_progress_steps = ((double) cu_list_elements + dwz_list_elements
+			  + types_list_elements);
+
   create_cus_from_index (objfile, cu_list, cu_list_elements, dwz_list,
-			 dwz_list_elements);
+			 dwz_list_elements, total_progress_steps);
 
   if (types_list_elements)
     {
@@ -3384,7 +3397,10 @@  dwarf2_read_index (struct objfile *objfile)
 			   dwarf2_per_objfile->types, 0);
 
       create_signatured_type_table_from_index (objfile, section, types_list,
-					       types_list_elements);
+					       types_list_elements,
+					       (cu_list_elements
+						+ dwz_list_elements),
+					       total_progress_steps);
     }
 
   create_addrmap_from_index (objfile, &local_map);
@@ -3939,16 +3955,17 @@  dw2_expand_symtabs_for_function (struct objfile *objfile,
 static void
 dw2_expand_all_symtabs (struct objfile *objfile)
 {
-  int i;
+  int i, count;
 
   dw2_setup (objfile);
 
-  for (i = 0; i < (dwarf2_per_objfile->n_comp_units
-		   + dwarf2_per_objfile->n_type_units); ++i)
+  count = dwarf2_per_objfile->n_comp_units + dwarf2_per_objfile->n_type_units;
+  for (i = 0; i < count; ++i)
     {
       struct dwarf2_per_cu_data *per_cu = dw2_get_cutu (i);
 
       dw2_instantiate_symtab (per_cu);
+      current_uiout->progress (i / (double) count);
     }
 }
 
@@ -6626,6 +6643,7 @@  dwarf2_build_psymtabs_hard (struct objfile *objfile)
   struct obstack temp_obstack;
   int i;
 
+  current_uiout->progress (0);
   if (dwarf_read_debug)
     {
       fprintf_unfiltered (gdb_stdlog, "Building psymtabs of objfile %s ...\n",
@@ -6656,6 +6674,7 @@  dwarf2_build_psymtabs_hard (struct objfile *objfile)
       struct dwarf2_per_cu_data *per_cu = dw2_get_cutu (i);
 
       process_psymtab_comp_unit (per_cu, 0, language_minimal);
+      current_uiout->progress (i / (double) dwarf2_per_objfile->n_comp_units);
     }
 
   /* This has to wait until we read the CUs, we need the list of DWOs.  */
diff --git a/gdb/mi/mi-out.h b/gdb/mi/mi-out.h
index fea94f2..490c06a 100644
--- a/gdb/mi/mi-out.h
+++ b/gdb/mi/mi-out.h
@@ -72,6 +72,18 @@  protected:
   virtual bool do_is_mi_like_p () override
   { return true; }
 
+  virtual void do_progress_start (const std::string &, int) override
+  {
+  }
+
+  virtual void do_progress_notify (double) override
+  {
+  }
+
+  virtual void do_progress_end () override
+  {
+  }
+
 private:
 
   void field_separator ();
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index bb482ee..9ffb796 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -35,6 +35,7 @@ 
 #include "language.h"
 #include "cp-support.h"
 #include "gdbcmd.h"
+#include "common/gdb_optional.h"
 
 struct psymbol_bcache
 {
@@ -78,23 +79,22 @@  require_partial_symbols (struct objfile *objfile, int verbose)
 
       if (objfile->sf->sym_read_psymbols)
 	{
+	  gdb::optional<ui_out::progress_meter> meter;
+
 	  if (verbose)
 	    {
-	      printf_unfiltered (_("Reading symbols from %s..."),
-				 objfile_name (objfile));
-	      gdb_flush (gdb_stdout);
+	      std::string text = (std::string ("Reading symbols from ")
+				  + objfile_name (objfile));
+
+	      meter.emplace (current_uiout, text, verbose);
 	    }
+
 	  (*objfile->sf->sym_read_psymbols) (objfile);
+
 	  if (verbose)
 	    {
 	      if (!objfile_has_symbols (objfile))
-		{
-		  wrap_here ("");
-		  printf_unfiltered (_("(no debugging symbols found)..."));
-		  wrap_here ("");
-		}
-
-	      printf_unfiltered (_("done.\n"));
+		printf_unfiltered (_(" (no debugging symbols found)\n"));
 	    }
 	}
     }
diff --git a/gdb/symfile.c b/gdb/symfile.c
index ff1e726..285663e 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1165,50 +1165,53 @@  symbol_file_add_with_addrs (bfd *abfd, const char *name,
   /* We either created a new mapped symbol table, mapped an existing
      symbol table file which has not had initial symbol reading
      performed, or need to read an unmapped symbol table.  */
-  if (should_print)
-    {
-      if (deprecated_pre_add_symbol_hook)
-	deprecated_pre_add_symbol_hook (name);
-      else
-	{
-	  printf_unfiltered (_("Reading symbols from %s..."), name);
-	  wrap_here ("");
-	  gdb_flush (gdb_stdout);
-	}
-    }
-  syms_from_objfile (objfile, addrs, add_flags);
+  {
+    std::string progress_text;
+
+    if (should_print)
+      {
+	if (deprecated_pre_add_symbol_hook)
+	  deprecated_pre_add_symbol_hook (name);
+
+	progress_text = std::string ("Reading symbols from ") + name;
+      }
+    else
+      progress_text = "";
+
+    ui_out::progress_meter meter (current_uiout, progress_text, should_print);
+    syms_from_objfile (objfile, addrs, add_flags);
+  }
 
   /* We now have at least a partial symbol table.  Check to see if the
      user requested that all symbols be read on initial access via either
      the gdb startup command line or on a per symbol file basis.  Expand
      all partial symbol tables for this objfile if so.  */
 
-  if ((flags & OBJF_READNOW))
-    {
-      if (should_print)
-	{
-	  printf_unfiltered (_("expanding to full symbols..."));
-	  wrap_here ("");
-	  gdb_flush (gdb_stdout);
-	}
+  {
+    if ((flags & OBJF_READNOW))
+      {
+	std::string progress_text;
 
-      if (objfile->sf)
-	objfile->sf->qf->expand_all_symtabs (objfile);
-    }
+	if (should_print)
+	  progress_text = std::string ("Expanding full symbols for ") + name;
+	else
+	  progress_text = "";
 
-  if (should_print && !objfile_has_symbols (objfile))
-    {
-      wrap_here ("");
-      printf_unfiltered (_("(no debugging symbols found)..."));
-      wrap_here ("");
-    }
+	ui_out::progress_meter meter (current_uiout, progress_text,
+				      should_print);
+
+	if (objfile->sf)
+	  objfile->sf->qf->expand_all_symtabs (objfile);
+      }
+  }
 
   if (should_print)
     {
+      if (!objfile_has_symbols (objfile))
+	printf_unfiltered (_(" (no debugging symbols found)\n"));
+
       if (deprecated_post_add_symbol_hook)
 	deprecated_post_add_symbol_hook ();
-      else
-	printf_unfiltered (_("done.\n"));
     }
 
   /* We print some messages regardless of whether 'from_tty ||
@@ -2483,10 +2486,6 @@  reread_symbols (void)
 	  struct cleanup *old_cleanups;
 	  struct section_offsets *offsets;
 	  int num_offsets;
-	  char *original_name;
-
-	  printf_unfiltered (_("`%s' has changed; re-reading symbols.\n"),
-			     objfile_name (objfile));
 
 	  /* There are various functions like symbol_file_add,
 	     symfile_bfd_open, syms_from_objfile, etc., which might
@@ -2502,6 +2501,11 @@  reread_symbols (void)
 	  /* We need to do this whenever any symbols go away.  */
 	  make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
 
+	  std::string text
+	    = (std::string (_("`%s' has changed; re-reading symbols.\n"))
+	       + objfile_name (objfile));
+	  ui_out::progress_meter meter (current_uiout, text, 1);
+
 	  if (exec_bfd != NULL
 	      && filename_cmp (bfd_get_filename (objfile->obfd),
 			       bfd_get_filename (exec_bfd)) == 0)
@@ -2548,8 +2552,7 @@  reread_symbols (void)
 	      error (_("Can't open %s to read symbols."), obfd_filename);
 	  }
 
-	  original_name = xstrdup (objfile->original_name);
-	  make_cleanup (xfree, original_name);
+	  std::string original_name (objfile->original_name);
 
 	  /* bfd_openr sets cacheable to true, which is what we want.  */
 	  if (!bfd_check_format (objfile->obfd, bfd_object))
@@ -2598,8 +2601,9 @@  reread_symbols (void)
 	  set_objfile_per_bfd (objfile);
 
 	  objfile->original_name
-	    = (char *) obstack_copy0 (&objfile->objfile_obstack, original_name,
-				      strlen (original_name));
+	    = (char *) obstack_copy0 (&objfile->objfile_obstack,
+				      original_name.c_str (),
+				      original_name.length ());
 
 	  /* Reset the sym_fns pointer.  The ELF reader can change it
 	     based on whether .gdb_index is present, and we need it to
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 7f858e6..76915e1 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,30 @@ 
+2017-04-12  Tom Tromey  <tom@tromey.com>
+
+	PR cli/19551:
+	* lib/mi-support.exp (mi_gdb_file_cmd): Update regexps.
+	* lib/gdb.exp (gdb_file_cmd): Update regexps.
+	* gdb.stabs/weird.exp: Update regexp.
+	* gdb.server/solib-list.exp: Update regexp.
+	* gdb.linespec/linespec.exp: Update regexp.
+	* gdb.multi/remove-inferiors.exp (test_remove_inferiors): Update
+	regexp.
+	* gdb.dwarf2/dw2-stack-boundary.exp: Update regexp.
+	* gdb.dwarf2/dw2-objfile-overlap.exp: Update regexp.
+	* gdb.cp/cp-relocate.exp: Update regexp.
+	* gdb.base/sym-file.exp: Update regexps.
+	* gdb.base/sepdebug.exp: Update regexps.
+	* gdb.base/relocate.exp: Update regexps.
+	* gdb.base/print-symbol-loading.exp (test_load_core): Update regexp.
+	* gdb.base/kill-detach-inferiors-cmd.exp: Update regexp.
+	* gdb.base/dbx.exp (gdb_file_cmd): Update regexp.
+	* gdb.base/code_elim.exp: Update regexp.
+	* gdb.base/break-unload-file.exp (test_break): Update regexp.
+	* gdb.base/break-interp.exp (test_attach_gdb): Update regexp.
+	* gdb.base/break-idempotent.exp (force_breakpoint_re_set): Update
+	regexp.
+	* gdb.base/attach.exp (do_attach_tests): Update regexp.
+	* gdb.python/py-section-script.exp: Update regexps.
+
 2017-04-12  Pedro Alves  <palves@redhat.com>
 
 	PR gdb/21323
diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index 98a50f5..301f2ef 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -167,10 +167,10 @@  proc do_attach_tests {} {
     set test "set file, before attach1"
     gdb_test_multiple "file $binfile" "$test" {
 	-re "Load new symbol table from.*y or n. $" {
-	    gdb_test "y" "Reading symbols from $escapedbinfile\.\.\.*done." \
+	    gdb_test "y" "Reading symbols from $escapedbinfile.*" \
 		"$test (re-read)"
 	}
-	-re "Reading symbols from $escapedbinfile\.\.\.*done.*$gdb_prompt $" {
+	-re "Reading symbols from $escapedbinfile.*$gdb_prompt $" {
 	    pass "$test"
 	}
     }
@@ -226,7 +226,7 @@  proc do_attach_tests {} {
 	    # executable's symbol table.  This in turn always results in
 	    # asking the user for actually loading the symbol table of the
 	    # executable.
-	    gdb_test "y" "Reading symbols from $sysroot$escapedbinfile\.\.\.*done." \
+	    gdb_test "y" "Reading symbols from $sysroot$escapedbinfile" \
 		"$test (reset file)"
 
 	    set found_exec_file 1
@@ -241,10 +241,10 @@  proc do_attach_tests {} {
 	set test "load file manually, after attach2"
 	gdb_test_multiple "file $binfile" "$test" {
 	    -re "A program is being debugged already..*Are you sure you want to change the file.*y or n. $" {
-		gdb_test "y" "Reading symbols from $escapedbinfile\.\.\.*done." \
+		gdb_test "y" "Reading symbols from $escapedbinfile" \
 		    "$test (re-read)"
 	    }
-	    -re "Reading symbols from $escapedbinfile\.\.\.*done.*$gdb_prompt $" {
+	    -re "Reading symbols from $escapedbinfile.*$gdb_prompt $" {
 		pass "$test"
 	    }
 	}
diff --git a/gdb/testsuite/gdb.base/break-idempotent.exp b/gdb/testsuite/gdb.base/break-idempotent.exp
index 2944edd..faf9d0a 100644
--- a/gdb/testsuite/gdb.base/break-idempotent.exp
+++ b/gdb/testsuite/gdb.base/break-idempotent.exp
@@ -69,7 +69,7 @@  proc force_breakpoint_re_set {} {
 	    send_gdb "y\n"
 	    exp_continue
 	}
-	-re "Reading symbols from.*done.*$gdb_prompt $" {
+	-re "Reading symbols from.*$gdb_prompt $" {
 	    pass $test
 	}
     }
diff --git a/gdb/testsuite/gdb.base/break-interp.exp b/gdb/testsuite/gdb.base/break-interp.exp
index 3a6d9a9..4398d96 100644
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -258,7 +258,7 @@  proc test_attach_gdb {file pid displacement prefix} {
 	# Print the "PIE (Position Independent Executable) displacement" message.
 	gdb_test_no_output "set verbose on"
 
-	gdb_test "file $file" "Reading symbols from .*done\\." "file"
+	gdb_test "file $file" "Reading symbols from .*" "file"
 
 	set test "attach"
 	gdb_test_multiple "attach $pid" $test {
diff --git a/gdb/testsuite/gdb.base/break-unload-file.exp b/gdb/testsuite/gdb.base/break-unload-file.exp
index e33ee9f..df464c8 100644
--- a/gdb/testsuite/gdb.base/break-unload-file.exp
+++ b/gdb/testsuite/gdb.base/break-unload-file.exp
@@ -132,7 +132,7 @@  proc test_break { initial_load always_inserted break_command } {
 		send_gdb "y\n"
 		exp_continue
 	    }
-	    -re "Reading symbols from.*done.*$gdb_prompt $" {
+	    -re "Reading symbols from.*$gdb_prompt $" {
 		pass $test
 	    }
 	}
diff --git a/gdb/testsuite/gdb.base/code_elim.exp b/gdb/testsuite/gdb.base/code_elim.exp
index 29d9f37..24b4530 100644
--- a/gdb/testsuite/gdb.base/code_elim.exp
+++ b/gdb/testsuite/gdb.base/code_elim.exp
@@ -75,7 +75,7 @@  gdb_exit
 gdb_start
 
 gdb_test "symbol-file ${binfile1}" \
-	"Reading symbols from .*${testfile1}\\.\\.\\.done\\.(|\r\nUsing host libthread_db library .*libthread_db.so.*\\.)" \
+	"Reading symbols from .*${testfile1}(|\r\nUsing host libthread_db library .*libthread_db.so.*\\.).*" \
 	"symbol-file ${testfile1}"
 
 with_test_prefix "single psymtabs" {
@@ -109,13 +109,13 @@  gdb_start
 
 with_test_prefix "order1" {
     gdb_test "add-symbol-file ${binfile1} 0x100000 -s .bss 0x120000" \
-	    "Reading symbols from .*${testfile1}\\.\\.\\.done\\." \
+	    "Reading symbols from .*${testfile1}.*" \
 	    "add-symbol-file ${testfile1} 0x100000" \
 	    "add symbol table from file \".*${testfile1}\" at.*\\(y or n\\) " \
 	    "y"
 
     gdb_test "add-symbol-file ${binfile2} 0x200000 -s .data 0x210000 -s .bss 0x220000" \
-	    "Reading symbols from .*${testfile2}\\.\\.\\.done\\." \
+	    "Reading symbols from .*${testfile2}.*" \
 	    "add-symbol-file ${testfile2} 0x200000" \
 	    "add symbol table from file \".*${testfile2}\" at.*\\(y or n\\) " \
 	    "y"
@@ -133,13 +133,13 @@  gdb_start
 
 with_test_prefix "order2" {
     gdb_test "add-symbol-file ${binfile2} 0x200000 -s .data 0x210000 -s .bss 0x220000" \
-	    "Reading symbols from .*${testfile2}\\.\\.\\.done\\." \
+	    "Reading symbols from .*${testfile2}.*" \
 	    "add-symbol-file ${testfile2} 0x200000" \
 	    "add symbol table from file \".*${testfile2}\" at.*\\(y or n\\) " \
 	    "y"
 
     gdb_test "add-symbol-file ${binfile1} 0x100000 -s .bss 0x120000" \
-	    "Reading symbols from .*${testfile1}\\.\\.\\.done\\." \
+	    "Reading symbols from .*${testfile1}.*" \
 	    "add-symbol-file ${testfile1} 0x100000" \
 	    "add symbol table from file \".*${testfile1}\" at.*\\(y or n\\) " \
 	    "y"
diff --git a/gdb/testsuite/gdb.base/dbx.exp b/gdb/testsuite/gdb.base/dbx.exp
index 19bc709..023093c 100644
--- a/gdb/testsuite/gdb.base/dbx.exp
+++ b/gdb/testsuite/gdb.base/dbx.exp
@@ -189,7 +189,7 @@  proc gdb_file_cmd {arg} {
             }
             return 0
         }
-        -re "Reading symbols from.*done.*$gdb_prompt $" {
+        -re "Reading symbols from.*$gdb_prompt $" {
             verbose "\t\tLoaded $arg into the $GDB"
             send_gdb "exec-file $arg\n" 
             gdb_expect {
diff --git a/gdb/testsuite/gdb.base/kill-detach-inferiors-cmd.exp b/gdb/testsuite/gdb.base/kill-detach-inferiors-cmd.exp
index 23c262f..26dd6fe 100644
--- a/gdb/testsuite/gdb.base/kill-detach-inferiors-cmd.exp
+++ b/gdb/testsuite/gdb.base/kill-detach-inferiors-cmd.exp
@@ -34,7 +34,7 @@  runto_main
 # Add another forked inferior process.
 gdb_test "add-inferior" "Added inferior 2" "add inferior 2"
 gdb_test "inferior 2" "Switching to inferior 2.*"
-gdb_test "file $binfile" "Reading symbols from .*done.*" "load binary"
+gdb_test "file $binfile" "Reading symbols from .*" "load binary"
 gdb_test "start" "Temporary breakpoint.*Starting program.*"
 
 # Add an attached inferior process.
diff --git a/gdb/testsuite/gdb.base/print-symbol-loading.exp b/gdb/testsuite/gdb.base/print-symbol-loading.exp
index d7b3540..331a7a8 100644
--- a/gdb/testsuite/gdb.base/print-symbol-loading.exp
+++ b/gdb/testsuite/gdb.base/print-symbol-loading.exp
@@ -56,7 +56,7 @@  proc test_load_core { print_symbol_loading } {
 	gdb_reinitialize_dir $srcdir/$subdir
 	gdb_test_no_output "set print symbol-loading $print_symbol_loading"
 	if { ${print_symbol_loading} != "off" } {
-	    gdb_test "file $binfile" "Reading symbols from.*done\\." "file"
+	    gdb_test "file $binfile" "Reading symbols from .*" "file"
 	} else {
 	    gdb_test_no_output "file $binfile" "file"
 	}
diff --git a/gdb/testsuite/gdb.base/relocate.exp b/gdb/testsuite/gdb.base/relocate.exp
index 5639cc8..9455fb1 100644
--- a/gdb/testsuite/gdb.base/relocate.exp
+++ b/gdb/testsuite/gdb.base/relocate.exp
@@ -38,7 +38,7 @@  foreach x {"-raednow" "readnow" "foo" "-readnow s"} {
 
 # Load the object file.
 gdb_test "add-symbol-file ${binfile} 0" \
-	"Reading symbols from .*${testfile}\\.o\\.\\.\\.done\\.(|\r\nUsing host libthread_db library .*libthread_db.so.*\\.)" \
+	"Reading symbols from .*${testfile}\\.o(|\r\nUsing host libthread_db library .*libthread_db.so.*\\.).*" \
 	"add-symbol-file ${testfile}.o 0" \
 	"add symbol table from file \".*${testfile}\\.o\" at\[ \t\r\n\]+\.text_addr = 0x0\[\r\n\]+\\(y or n\\) " \
 	"y"
@@ -87,7 +87,7 @@  gdb_test_no_output "set \$offset = 0x10000"
 
 # Load the object file.
 gdb_test "add-symbol-file ${binfile} \$offset" \
-	"Reading symbols from .*${testfile}\\.o\\.\\.\\.done\\.(|\r\nUsing host libthread_db library .*libthread_db.so.*\\.)" \
+	"Reading symbols from .*${testfile}\\.o(|\r\nUsing host libthread_db library .*libthread_db.so.*\\.).*" \
 	"add-symbol-file ${testfile}.o \$offset" \
 	"add symbol table from file \".*${testfile}\\.o\" at\[ \t\r\n\]+\.text_addr = 0x10000\[\r\n\]+\\(y or n\\) " \
 	"y"
diff --git a/gdb/testsuite/gdb.base/sepdebug.exp b/gdb/testsuite/gdb.base/sepdebug.exp
index ce55aa3..cd93e6b 100644
--- a/gdb/testsuite/gdb.base/sepdebug.exp
+++ b/gdb/testsuite/gdb.base/sepdebug.exp
@@ -660,7 +660,7 @@  if {[build_executable sepdebug.exp sepdebug2 sepdebug2.c debug] != -1
 
     set escapedobjdirsubdir [string_to_regexp [standard_output_file {}]]
 
-    gdb_test "file [standard_output_file sepdebug2]" "warning: the debug information found in \"${escapedobjdirsubdir}/sepdebug2\\.debug\" does not match \"${escapedobjdirsubdir}/sepdebug2\" \\(CRC mismatch\\)\\..*\\(no debugging symbols found\\).*" "CRC mismatch is reported"
+    gdb_test "file [standard_output_file sepdebug2]" "warning: the debug information found in \"${escapedobjdirsubdir}/sepdebug2\\.debug\" does not match \"${escapedobjdirsubdir}/sepdebug2\" \\(CRC mismatch\\)\\..*" "CRC mismatch is reported"
 }
 
 
diff --git a/gdb/testsuite/gdb.base/sym-file.exp b/gdb/testsuite/gdb.base/sym-file.exp
index f058021..680cea5 100644
--- a/gdb/testsuite/gdb.base/sym-file.exp
+++ b/gdb/testsuite/gdb.base/sym-file.exp
@@ -93,7 +93,7 @@  if {!$result} then {
 
 # 3) Add the library's symbols using 'add-symbol-file'.
 set result [gdb_test "add-symbol-file ${lib_syms} addr" \
-		     "Reading symbols from .*${lib_syms}\\.\\.\\.done\\." \
+		     "Reading symbols from .*${lib_syms}.*" \
 		     "add-symbol-file ${lib_basename}.so addr" \
 		     "add symbol table from file \".*${lib_basename}\\.so\"\
  at.*\\(y or n\\) " \
@@ -177,7 +177,7 @@  with_test_prefix "stale bkpts" {
 
     # Load the library's symbols.
     gdb_test "add-symbol-file ${lib_syms} addr" \
-	"Reading symbols from .*${lib_syms}\\.\\.\\.done\\." \
+	"Reading symbols from .*${lib_syms}.*" \
 	"add-symbol-file ${lib_basename}.so addr" \
 	"add symbol table from file \".*${lib_syms}\"\
 at.*\\(y or n\\) " \
diff --git a/gdb/testsuite/gdb.cp/cp-relocate.exp b/gdb/testsuite/gdb.cp/cp-relocate.exp
index 50ddebd..7827269 100644
--- a/gdb/testsuite/gdb.cp/cp-relocate.exp
+++ b/gdb/testsuite/gdb.cp/cp-relocate.exp
@@ -123,7 +123,7 @@  gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 
 gdb_test "add-symbol-file ${binfile} 0 -s ${func1_sec} 0x10000 -s ${func2_sec} 0x20000" \
-	"Reading symbols from .*${testfile}\\.o\\.\\.\\.done\\.(|\r\nUsing host libthread_db library .*libthread_db.so.*\\.)" \
+	"Reading symbols from .*${testfile}\\.o(|\r\nUsing host libthread_db library .*libthread_db.so.*\\.).*" \
 	"add-symbol-file ${testfile}.o" \
 	"add symbol table from file \".*${testfile}\\.o\" at.*\\(y or n\\) " \
 	"y"
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-objfile-overlap.exp b/gdb/testsuite/gdb.dwarf2/dw2-objfile-overlap.exp
index 90a78ef..50628b4 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-objfile-overlap.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-objfile-overlap.exp
@@ -37,7 +37,7 @@  if { [gdb_compile "${srcdir}/${subdir}/${srcfile_outer}" "${binfile_outer}" \
 clean_restart $executable_outer
 
 gdb_test "add-symbol-file $binfile_inner outer_inner" \
-         {Reading symbols from .*\.\.\.done\.} "add-symbol-file" \
+         {Reading symbols from .*} "add-symbol-file" \
 	 "\r\n\t\\.text_addr = 0x\[0-9a-f\]+\r\n\\(y or n\\) \$" "y"
 
 # Expand symtab for ${binfile_outer}.
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp b/gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp
index eb9f422..2f455fa 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-stack-boundary.exp
@@ -38,7 +38,7 @@  if [is_remote host] {
     }
 }
 gdb_test_no_output "set complaints 100"
-gdb_test "file $binfile" {Reading symbols from .*\.\.\.location description stack underflow\.\.\.location description stack overflow\.\.\..*done\.} "check partial symtab errors"
+gdb_test "file $binfile" {Reading symbols from .*location description stack underflow.*location description stack overflow.*} "check partial symtab errors"
 
 gdb_test "p underflow" {Asked for position 0 of stack, stack only has 0 elements on it\.}
 gdb_test "p overflow" " = 2"
diff --git a/gdb/testsuite/gdb.linespec/linespec.exp b/gdb/testsuite/gdb.linespec/linespec.exp
index ccb73c8..aba4db3 100644
--- a/gdb/testsuite/gdb.linespec/linespec.exp
+++ b/gdb/testsuite/gdb.linespec/linespec.exp
@@ -123,7 +123,7 @@  gdb_test "inferior 2" "Switching to inferior 2 .*" \
 # Note that in particular this should not cause errors when re-setting
 # breakpoints.
 gdb_test "file $binfile" \
-    "Reading symbols from .*done." \
+    "Reading symbols from .*." \
     "set the new inferior file for linespec tests"
 
 gdb_test "break main" \
diff --git a/gdb/testsuite/gdb.multi/remove-inferiors.exp b/gdb/testsuite/gdb.multi/remove-inferiors.exp
index b89e0be..ea99e78 100644
--- a/gdb/testsuite/gdb.multi/remove-inferiors.exp
+++ b/gdb/testsuite/gdb.multi/remove-inferiors.exp
@@ -49,7 +49,7 @@  proc test_remove_inferiors { } {
     # Load binfile and start the inferior.
     set binfile_re [string_to_regexp ${binfile}]
     gdb_test "file ${binfile}" \
-	     "Reading symbols from ${binfile_re}...done." \
+	     "Reading symbols from ${binfile_re}.*" \
 	     "load binary"
 
     if {![runto_main]} {
diff --git a/gdb/testsuite/gdb.python/py-section-script.exp b/gdb/testsuite/gdb.python/py-section-script.exp
index 6707156..212389a 100644
--- a/gdb/testsuite/gdb.python/py-section-script.exp
+++ b/gdb/testsuite/gdb.python/py-section-script.exp
@@ -140,7 +140,7 @@  with_test_prefix "sepdebug" {
 	-re "\r\nwarning: Invalid entry in \\.debug_gdb_scripts section.*\r\n$gdb_prompt $" {
 	    fail $test
 	}
-	-re "done\\.\r\n$gdb_prompt $" {
+	-re "Reading symbols from ${binfile}.*$gdb_prompt $" {
 	    pass $test
 	}
     }
diff --git a/gdb/testsuite/gdb.server/solib-list.exp b/gdb/testsuite/gdb.server/solib-list.exp
index 64fc70b..f3e41a2 100644
--- a/gdb/testsuite/gdb.server/solib-list.exp
+++ b/gdb/testsuite/gdb.server/solib-list.exp
@@ -88,7 +88,7 @@  foreach nonstop { 0 1 } { with_test_prefix "non-stop $nonstop" {
     # but before "target remote" below so that qSymbol data get already
     # initialized from BINFILE (and not from ld.so first needing a change to
     # BINFILE later).
-    gdb_test "file ${binfile}" {Reading symbols from .*\.\.\.done\..*} "file binfile" \
+    gdb_test "file ${binfile}" {Reading symbols from .*} "file binfile" \
 	     {(Are you sure you want to change the file|Load new symbol table from ".*")\? \(y or n\) } "y"
 
     set test "target $gdbserver_protocol"
diff --git a/gdb/testsuite/gdb.stabs/weird.exp b/gdb/testsuite/gdb.stabs/weird.exp
index 68a3ed6..e398a01 100644
--- a/gdb/testsuite/gdb.stabs/weird.exp
+++ b/gdb/testsuite/gdb.stabs/weird.exp
@@ -301,7 +301,7 @@  gdb_expect 60 {
 	send_gdb "y\n"
 	exp_continue
     }
-    -re "^Reading symbols from .*$binfile_re\\.\\.\\.done\.(|\r\nUsing host libthread_db library .*libthread_db.so.*\\.)\r\n$gdb_prompt $" {
+    -re "^Reading symbols from .*${binfile}.*(|\r\nUsing host libthread_db library .*libthread_db.so.*\\.)$gdb_prompt $" {
 	pass "weirdx.o read without error"
     }
     -re ".*$gdb_prompt $" {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index ea77361..43510ba 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -1465,17 +1465,17 @@  proc gdb_file_cmd { arg } {
 
     send_gdb "file $arg\n"
     gdb_expect 120 {
-	-re "Reading symbols from.*LZMA support was disabled.*done.*$gdb_prompt $" {
+	-re "Reading symbols from.*LZMA support was disabled.*$gdb_prompt $" {
 	    verbose "\t\tLoaded $arg into $GDB; .gnu_debugdata found but no LZMA available"
 	    set gdb_file_cmd_debug_info "lzma"
 	    return 0
 	}
-	-re "Reading symbols from.*no debugging symbols found.*done.*$gdb_prompt $" {
+	-re "Reading symbols from.*no debugging symbols found.*$gdb_prompt $" {
 	    verbose "\t\tLoaded $arg into $GDB with no debugging symbols"
 	    set gdb_file_cmd_debug_info "nodebug"
 	    return 0
 	}
-        -re "Reading symbols from.*done.*$gdb_prompt $" {
+        -re "Reading symbols from.*$gdb_prompt $" {
             verbose "\t\tLoaded $arg into $GDB"
 	    set gdb_file_cmd_debug_info "debug"
 	    return 0
@@ -1483,7 +1483,7 @@  proc gdb_file_cmd { arg } {
         -re "Load new symbol table from \".*\".*y or n. $" {
             send_gdb "y\n"
             gdb_expect 120 {
-                -re "Reading symbols from.*done.*$gdb_prompt $" {
+                -re "Reading symbols from.*$gdb_prompt $" {
                     verbose "\t\tLoaded $arg with new symbol table into $GDB"
 		    set gdb_file_cmd_debug_info "debug"
 		    return 0
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index 5682b7e..b8b2029 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -504,7 +504,7 @@  proc mi_gdb_file_cmd { arg } {
 # output.  Queries are an error for mi.
     send_gdb "105-file-exec-and-symbols $arg\n"
     gdb_expect 120 {
-	-re "Reading symbols from.*done.*$mi_gdb_prompt$" {
+	-re "Reading symbols from.*$mi_gdb_prompt$" {
 	    verbose "\t\tLoaded $arg into the $GDB"
 	    return 0
 	}
@@ -515,7 +515,7 @@  proc mi_gdb_file_cmd { arg } {
 	-re "Load new symbol table from \".*\".*y or n. $" {
 	    send_gdb "y\n"
 	    gdb_expect 120 {
-		-re "Reading symbols from.*done.*$mi_gdb_prompt$" {
+		-re "Reading symbols from.*$mi_gdb_prompt$" {
 		    verbose "\t\tLoaded $arg with new symbol table into $GDB"
 		    # All OK
 		}
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 9278cab..7857fba 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -135,6 +135,38 @@  class ui_out
   bool query_table_field (int colno, int *width, int *alignment,
 			  const char **col_name);
 
+  /* An object that starts and finishes a progress meter.  */
+  class progress_meter
+  {
+  public:
+    progress_meter (struct ui_out *uiout, const std::string &name,
+		    int should_print)
+      : m_uiout (uiout)
+    {
+      m_uiout->do_progress_start (name, should_print);
+    }
+
+    ~progress_meter ()
+    {
+      m_uiout->do_progress_notify (1.0);
+      m_uiout->do_progress_end ();
+    }
+
+    progress_meter (const progress_meter &) = delete;
+    progress_meter &operator= (const progress_meter &) = delete;
+
+  private:
+
+    struct ui_out *m_uiout;
+  };
+
+  /* Emit some progress corresponding to the most recently created
+     progress meter.  HOWMUCH may range from 0.0 to 1.0.  */
+  void progress (double howmuch)
+  {
+    do_progress_notify (howmuch);
+  }
+
  protected:
 
   virtual void do_table_begin (int nbrofcols, int nr_rows, const char *tblid)
@@ -165,6 +197,10 @@  class ui_out
   virtual void do_flush () = 0;
   virtual void do_redirect (struct ui_file *outstream) = 0;
 
+  virtual void do_progress_start (const std::string &, int) = 0;
+  virtual void do_progress_notify (double) = 0;
+  virtual void do_progress_end () = 0;
+
   /* Set as not MI-like by default.  It is overridden in subclasses if
      necessary.  */
 
diff --git a/gdb/utils.c b/gdb/utils.c
index b4332f8..302920b 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1847,6 +1847,14 @@  reinitialize_more_filter (void)
   chars_printed = 0;
 }
 
+/* See utils.h.  */
+
+int
+get_chars_per_line (void)
+{
+  return chars_per_line;
+}
+
 /* Indicate that if the next sequence of characters overflows the line,
    a newline should be inserted here rather than when it hits the end.
    If INDENT is non-null, it is a string to be printed to indent the
diff --git a/gdb/utils.h b/gdb/utils.h
index f3e8007..c371bad 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -155,6 +155,10 @@  extern void wrap_here (const char *);
 
 extern void reinitialize_more_filter (void);
 
+/* Return the number of characters in a line.  */
+
+extern int get_chars_per_line (void);
+
 extern int pagination_enabled;
 
 extern struct ui_file **current_ui_gdb_stdout_ptr (void);