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

Pedro Alves
2017-11-30T11:53:11Z

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