Re: [PATCH v2] Add support for the --readnever command-line option (DWARF only)

[Pedro Alves <palves@redhat.com>]

On 11/24/2017 04:54 AM, Sergio Durigan Junior wrote:
> On Thursday, November 23 2017, Pedro Alves wrote:
> 
>> On 11/23/2017 05:21 PM, Sergio Durigan Junior wrote:
>>
>>>>> +This option is currently limited to debug information in DWARF format.
>>>>> +For all other format, this option has no effect.
>>>>
>>>> How hard would it be to just make it work?  There's only stabs and mdebug
>>>> left, I think?  There should be a single a function somewhere that we can
>>>> add an early return.  And then we don't need to document this limitation...
>>>>
>>>> For example, in elf_symfile_read, we could just skip the elf_locate_sections
>>>> call.  In coffread.c we could skip reading stabs right after 
>>>>   bfd_map_over_sections (abfd, coff_locate_sections....);
>>>>
>>>> Looking for:
>>>>
>>>>  $ grep -h "^[a-z]*_build_psymtabs" gdb/
>>>>  coffstab_build_psymtabs (struct objfile *objfile,
>>>>  elfstab_build_psymtabs (struct objfile *objfile, asection *stabsect,
>>>>  stabsect_build_psymtabs (struct objfile *objfile, char *stab_name,
>>>>  mdebug_build_psymtabs (minimal_symbol_reader &reader,
>>>>  elfmdebug_build_psymtabs (struct objfile *objfile,
>>>>
>>>> finds all the relevant places.
>>>>
>>>> Maybe it wouldn't be that hard to make this be an objfile flag
>>>> afterall (like OBJF_READNOW is).  That'd make it possible
>>>> to add the location "-readnever" counterpart switch to add-symbol-file
>>>> too, BTW:
>>
>> I meant "logical" instead of "location".  I was staring at
>> gdb/location.c at that time.  :-P
>>
>>>>
>>>>  symfile.c:        if (strcmp (arg, "-readnow") == 0)
>>>>  symfile.c:        else if (strcmp (arg, "-readnow") == 0)
>>>
>>> Hm, I'll look into this.  Just to make it clear: the idea is to have
>>> both a --readnever global option and also a OBJF_READNEVER specific to
>>> each objfile?
>>
>> Sure, the idea is to do something similar to what's done for --readnow.
> 
> Sorry, but I guess I need a few more details on this.
> 
> The way I understand the code at elf_symfile_read, the very first thing
> to do would be to check if OBJF_READNEVER is set and return early if it
> is.  But it seems that you're proposing something a bit different when
> you say that we should "... just skip the elf_locate_sections call."  It
> doesn't seem to me that is worth continuing on that function if
> OBJF_READNEVER is present.

No, you can't return early the very first thing, because
 --readnever is supposed to skip _debug_ info, not ELF/minimal symbols...
So the "return early" would have to be _after_ the
elf_read_minimal_symbols call:

 static void
 elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
 {
   bfd *abfd = objfile->obfd;
   struct elfinfo ei;

   memset ((char *) &ei, 0, sizeof (ei));
   bfd_map_over_sections (abfd, elf_locate_sections, (void *) & ei);
 
   elf_read_minimal_symbols (objfile, symfile_flags, &ei);

I don't know whether we can reorder that.  Maybe we can.

When I looked at this quickly yesterday, I saw that elf_location_sections
is what finds the stabs and mdebug sections:

static void
elf_locate_sections (bfd *ignore_abfd, asection *sectp, void *eip)
{
  struct elfinfo *ei;

  ei = (struct elfinfo *) eip;
  if (strcmp (sectp->name, ".stab") == 0)
    {
      ei->stabsect = sectp;
    }
  else if (strcmp (sectp->name, ".mdebug") == 0)
    {
      ei->mdebugsect = sectp;
    }
}

and it seemed to be that skipping the section location would make
the parts of  elf_symfile_read that actually read the symbols
be no-ops, because the stabsect/mdebusect pointers would be NULL.

But if returning early or something else works, that's fine.

> 
> As for the *_build_psymtabs functions, I am doing exactly that: if
> objfile->flags contains OBJF_READNEVER, then just return and do nothing.

Sure, that should work too.  It's just the difference between
skipping checking whether debug info is available (skipping before
calling into those), vs letting gdb do the work to figure out whether
debug info is available, but then ignore it.
The grep for "*_build_psymtabs" was intended as a pointer to find
what the relevant code is, including to look at the code that
is calling those functions, see if there's something to be done there.

> 
> The patch is almost ready for resubmission (well, I still need to figure
> out how to test the --readnow && --readnever scenario), but I want to
> make sure I got this part right.
> 

Thanks,
Pedro Alves