Re: GDB 10.2 Release: Proposing Mar 13-14 for official GDB 10.2 release

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-03-30 1:02 a.m., Tom Tromey wrote:
>>> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
>>> index 704ba9f36655..5d9256bece1e 100644
>>> --- a/gdb/dwarf2/read.c
>>> +++ b/gdb/dwarf2/read.c
>>> @@ -1954,7 +1954,8 @@ dwarf2_has_info (struct objfile *objfile,
>>> doesn't require relocations and if there aren't partial symbols
>>> from some other reader.  */
>>> if (!objfile_has_partial_symbols (objfile)
>>> -         && !gdb_bfd_requires_relocations (objfile->obfd))
>>> +         && !gdb_bfd_requires_relocations (objfile->obfd)
>>> +         && 0)
>>> {
>>> /* See if one has been created for this BFD yet.  */
>>> per_bfd = dwarf2_per_bfd_bfd_data_key.get (objfile->obfd);
>>>
>>> We can then take our time to look at a proper fix for master.
> 
> Joel> It's quite nice to see that this feature could be disabled with
> Joel> a one-liner like this.
> 
> Joel> Unless Tom has other ideas, this seems like the way to go for me.
> 
> Sorry I didn't respond to this earlier.
> I think it's fine for the branch.
>
> I came up with a complicated plan for the trunk, but after some
> reflection, I think a better plan is to notice OBJF_READNOW and refuse
> to do any sharing in this case.  That is, it's your patch, but replacing
> "0" with a check of OBJF_READNOW.  The appended probably has the wrong
> line numbers but it shows the idea.
> 
> This -readnow stuff is an optimization, which in retrospect is kind of
> silly, because on the whole I'd rather people not use -readnow anyway,
> so why bother optimizing.  But when I added the gdb-index code, I
> noticed the possibility and I guess I couldn't resist.
> 
> Tom
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 11cdcd02d0a..ec044ab66e5 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -1883,9 +1883,11 @@ dwarf2_has_info (struct objfile *objfile,
>      {
>        dwarf2_per_bfd *per_bfd;
>  
> -      /* We can share a "dwarf2_per_bfd" with other objfiles if the BFD
> -	 doesn't require relocations.  */
> -      if (!gdb_bfd_requires_relocations (objfile->obfd))G
> +      /* We can share a "dwarf2_per_bfd" with other objfiles if the
> +	 BFD doesn't require relocations, and if -readnow was never
> +	 requested.  */
> +      if (!gdb_bfd_requires_relocations (objfile->obfd)
> +	  && (objfile->flags & OBJF_READNOW) == 0)
>  	{
>  	  /* See if one has been created for this BFD yet.  */
>  	  per_bfd = dwarf2_per_bfd_bfd_data_key.get (objfile->obfd);

This is fine with me, and I think it's sufficient for now.  It passes
the test I wrote, so I'll adjust my patch to use this and post a v2.

In the general case, I'm not convinced that there aren't more problems
like this, where you load the same BFD with two different "quick"
methods (as mentioned in my commit message).

We currently handle the psymtabs -> index case (commit efb763a5ea35),
necessary for when the index-cache is on.  In that case, we just keep
using the already-created psymtabs the second time around, the index is
just not used.

But imagine an index -> psymtabs case.  I imagine we could eventually
have a "-no-index" switch to the file command and/or a "set use-index
on/off" parameter, to tell GDB to not use any index.  That could be
useful in case the index gives you some trouble and you would like to
read the file without it.  So you could do "file bin" first and "file
-no-index bin" second.  The dwarf2_per_bfd is created using the index
the first time, but we wouldn't want to re-use that dwarf2_per_bfd the
second time.

Only sharing the dwarf2_per_bfd between objfiles that use the same
"quick" method would make me more confident that we won't hit other such
problematic cases.

Simon