Re: [PATCHv2 5/5] gdb: fix reread_symbols when an objfile has target: prefix

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tromey@adacore.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
>>> The BFD cache (in gdb_bfd.c) uses fstat, so I think there would still be
>>> a problem here.
>
> Andrew> OK, but the original mtime is captured via a call to bfd_stat.
>
> Andrew> Isn't the problem when we have two mismatched calls.  Using bfd_stat in
> Andrew> one place (a.k.a. system stat/fstat) vs a direct call to stat/fstat from
> Andrew> GDB in another place (a.k.a. gnulib stat/fstat).
>
> Andrew> In this patch I'm proposing that we _consistently_ call bfd_stat.  Sure
> Andrew> that might disagree with system stat/fstat -- but who cares?  So long as
> Andrew> the time being calculated and compared to is a BFD time_t result then we
> Andrew> should be fine .... or am I really not understanding the problem?
>
> I think the fstat in gdb_bfd.c means that we will always re-read the
> debug info, because now gdb_bfd_open will use fstat (and this is what is
> recorded in the hash table), but reread_symbols will use bfd_stat.

I really don't think this is the case.  The code in reread_symbols is
this:

      int res;
      struct stat new_statbuf;
      if (objfile->obfd->my_archive)
        res = stat (bfd_get_filename (objfile->obfd->my_archive), &new_statbuf);
      else
        res = stat (objfile_name (objfile), &new_statbuf);
      if (res != 0)
        {
          /* FIXME, should use print_sys_errmsg but it's not filtered.  */
          gdb_printf (_("`%s' has disappeared; keeping its symbols.\n"),
                      objfile_name (objfile));
          continue;
        }
      time_t new_modtime = new_statbuf.st_mtime;
      if (new_modtime != objfile->mtime)
        {
          gdb_printf (_("`%ps' has changed; re-reading symbols.\n"),
                      styled_string (file_name_style.style (),
                                     objfile_name (objfile)));

          ...
        }

So we perform a system 'stat' call and compare this to objfile::mtime.

And objfile::mtime is initialised in objfiles.c with this code:

  if (obfd != nullptr)
    {
      mtime = bfd_get_mtime (obfd.get ());

      /* Build section table.  */
      build_objfile_section_table (this);
    }

Where bfd_get_mtime is going to fetch a value by calling bfd_stat, which
will either by the system stat (linked into libbfd.a), or will be GDB's
remote fileio stat call.

The fstat call you are looking at in gdb_bfd.c is I believe only used
as part of GDB's internal caching mechanism for BFDs.  I can't see
anywhere that these mtime values can escape gdb_bfd.c, nor do I see
anywhere that these values are compared to values returned by bfd_stat.

We know that the value in reread_symbols comes from the system stat.  So
what I'm missing, is the path by which objfile::mtime gets set from
anywhere other that within libbfd.a, such that it might get contaminated
by a gnulib stat result.

Thanks,
Andrew