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

[Joel Brobecker <brobecker@adacore.com>]

Hi Tom,

On Mon, Mar 29, 2021 at 11:02:13PM -0600, 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.

Great, thank you!

Speaking of the branch, given your patch below, perhaps we can have
best of both worlds if we go with the solution you suggest?
If we could converge on that by the time the weekend comes,
I think it would be reasonable to backport your patch to the branch
as well.

> 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,

Same here.

> so why bother optimizing.  But when I added the gdb-index code, I
> noticed the possibility and I guess I couldn't resist.

Lol.

> 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))
> +      /* 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);

Sounds like a good idea, especially if that avoids a complicated
plan to make it work.

The only small comment I have is that it would be useful, I think,
to explain why we say we cannot share the dwarf2_per_bfd when -readnow
is in effect (too complicated to support and not worth the effort
considering that performance is typically not as important in
the case of -readnow).

-- 
Joel