Re: [PATCH v4] Add support for the readnever concept

[Pedro Alves <palves@redhat.com>]

On 11/30/2017 12:25 AM, Sergio Durigan Junior wrote:

> - Split off the fix to 'symbol-file' command argument parsing.  This
>   patch depends on it
>   (<https://sourceware.org/ml/gdb-patches/2017-11/msg00799.html>).

This info above conflicts with ...

> While implementing the code for the 'symbol-file' command, I noticed a
> bug in 'symbol_file_command': GDB adds the symbol file before
> finishing parsing all the options, which means that the position of an
> option in the command impacts whether it will be considered or not.  I
> changed the code there in order to only add the symbol file after all
> options have been parsed.

... this info here.  The patch still has that bit...

> 
> It's also worth mentioning that this patch tests whether GDB correctly
> fails to initialize if both '--readnow' and '--readnever' options are
> passed.  We didn't have any infrastructure to test that, so I added a
> new version of 'gdb_spawn', called 'gdb_spawn_ignore_error', which can
> be used to start a GDB that will fail without compromising the entire
> testcase.

This is stale.

> gdb/testsuite/ChangeLog:
> 
> 2017-11-29  Joel Brobecker  <brobecker@adacore.com>
> 	    Sergio Durigan Junior  <sergiodj@redhat.com>
> 	    Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.base/readnever.c, gdb.base/readnever.exp: New files.
> 	* lib/gdb.exp (default_gdb_spawn): Add 'ignore_error'
> 	parameter.  Handle case when 'ignore_error' is set.
> 	(gdb_spawn_ignore_error): New function.

Also stale...

> @@ -699,7 +700,8 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
>  	}
>      }
>  
> -  bfd_map_over_sections (abfd, coff_locate_sections, (void *) info);
> +  if (!readnever_symbol_files && !(objfile->flags & OBJF_READNEVER))
> +    bfd_map_over_sections (abfd, coff_locate_sections, (void *) info);

I realized something: is it really ever possible to get here with

 readnever_symbol_files && !(objfile->flags & OBJF_READNEVER)

??

If readnever_symbol_files is set, then it'll have been propagated to
objfile->flags before we get here, no?

So references to the global readnever_symbol_files can (and should)
be removed from the readers, right?  This removes the need for
the new '#include "top.h"'s too, so please remove those from
the patch along the way.

>  
>    if (info->stabsects)
>      {
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 4f552225aa..a3127dd637 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -1037,6 +1037,15 @@ Read each symbol file's entire symbol table immediately, rather than
>  the default, which is to read it incrementally as it is needed.
>  This makes startup slower, but makes future operations faster.
>  
> +@item --readnever
> +@cindex @code{--readnever}, command-line option
> +Do not read each symbol file's symbolic debug information.  This makes
> +startup faster but at the expense of not being able to perform
> +symbolic debugging.  DWARF unwind information is also not read,
> +meaning backtraces may become incomplete or inaccurate.  One use of
> +this is when a user simply wants to do the following sequence: attach,
> +dump core, detach.  Loading the debugging information in this case is
> +an unnecessary cause of delay.
>  @end table
>  
>  @node Mode Options
> @@ -18576,6 +18585,14 @@ tables by using the @samp{-readnow} option with any of the commands that
>  load symbol table information, if you want to be sure @value{GDBN} has the
>  entire symbol table available.
>  
> +@cindex @code{-readnever}, option for symbol-file command
> +@cindex never read symbols
> +@cindex symbols, never read
> +@item symbol-file @r{[} -readnever @r{]} @var{filename}
> +@itemx file @r{[} -readnever @r{]} @var{filename}
> +You can instruct @value{GDBN} to never read the symbolic information
> +contained in @var{filename} by using the @samp{-readnever} option.

Maybe copy the rationale for this found in "--readnever" here
or xref the "--readnever" command line option?


>  #define READNOW_HELP \
>    "The '-readnow' option will cause GDB to read the entire symbol file\n\
>  immediately.  This makes the command slower, but may make future operations\n\
> -faster."
> +faster.\n"
> +
> +#define READNEVER_HELP \
> +  "The '-readnever' option will prevent GDB from reading the symbol file's\n\
> +symbolic debug information."
>  

Why not merge these two macros in one?  We'll always want to
use them together, in the same order?  You could rename the
macro to READNOW_READNEVER_HELP for example.

Thanks,
Pedro Alves