Re: [PATCH v2] Make source.c:search_command_helper use source cache

[Will Hawkins <hawkinsw@obs.cr>]

Hello!

I hope that everyone is having a great Monday! If there is something
that I can do to make this patch better, please let me know. I am
sincerely enjoying the opportunity to contribute to GDB and I hope
that you find these patches helpful!

Sincerely,
Will

On Fri, Mar 29, 2024 at 10:46 AM Will Hawkins <hawkinsw@obs.cr> wrote:
>
> I am sorry for the PING. I know that I am supposed to wait for 2 weeks
> before pinging (according to the Contributing documentation) but I am
> nervous that I have not followed the proper procedures for sending the
> patch.
>
> Granting the premise that the patch is something that is useful, I am
> sure that you will have feedback. I am more than willing to make any
> changes to get it into shape.
>
> As I said before, thank you for the work that you all do maintaining
> such an incredible piece of software.
>
> Will
>
>
> On Mon, Mar 25, 2024 at 10:01 AM Will Hawkins <hawkinsw@obs.cr> wrote:
> >
> > The current implementation of search_command_helper accesses the line
> > offsets of the current program spaces's source code through
> > the source cache but then accesses its contents through the disk. This
> > PR updates the implementation so that the access of the contents is also
> > through the source cache.
> >
> > Tested on x86_64-redhat-linux.
> >
> > Signed-off-by: Will Hawkins <hawkinsw@obs.cr>
> > ---
> >     v1 -> v2:
> >         Remove leftover debugging code.
> >
> >     Rationale:
> >         Supporting access through the source cache during searching will make it
> >         easier to implement the embedded source functionality of DW_LNCT_source.
> >
> >  gdb/source.c | 79 +++++++++++++++++++++++++---------------------------
> >  1 file changed, 38 insertions(+), 41 deletions(-)
> >
> > diff --git a/gdb/source.c b/gdb/source.c
> > index bbeb4154258..ef042b0e96e 100644
> > --- a/gdb/source.c
> > +++ b/gdb/source.c
> > @@ -1617,46 +1617,45 @@ search_command_helper (const char *regex, int from_tty, bool forward)
> >    if (!source_open)
> >      error (_("source code access disabled"));
> >
> > -  scoped_fd desc (open_source_file (loc->symtab ()));
> > -  if (desc.get () < 0)
> > -    perror_with_name (symtab_to_filename_for_display (loc->symtab ()),
> > -                     -desc.get ());
> > -
> > -  int line = (forward
> > -             ? last_line_listed + 1
> > -             : last_line_listed - 1);
> > -
> >    const std::vector<off_t> *offsets;
> > -  if (line < 1
> > -      || !g_source_cache.get_line_charpos (loc->symtab (), &offsets)
> > -      || line > offsets->size ())
> > +  if (!g_source_cache.get_line_charpos (loc->symtab (), &offsets))
> >      error (_("Expression not found"));
> >
> > -  if (lseek (desc.get (), (*offsets)[line - 1], 0) < 0)
> > -    perror_with_name (symtab_to_filename_for_display (loc->symtab ()));
> > +  std::string lines;
> > +  if (!g_source_cache.get_source_lines (loc->symtab (), 1, offsets->size (),
> > +                                       &lines))
> > +    perror_with_name (symtab_to_filename_for_display (loc->symtab ()), -ENOENT);
> > +
> > +  int line_no = (forward
> > +                ? last_line_listed + 1
> > +                : last_line_listed - 1);
> >
> > -  gdb_file_up stream = desc.to_file (FDOPEN_MODE);
> > -  clearerr (stream.get ());
> > +  if (line_no < 1
> > +      || line_no > offsets->size ())
> > +    error (_("Expression not found"));
> > +
> > +  size_t current_line_start = (*offsets)[line_no - 1];
> >
> > -  gdb::def_vector<char> buf;
> > -  buf.reserve (256);
> > +  std::string buf;
> >
> >    while (1)
> >      {
> > -      buf.resize (0);
> > -
> > -      int c = fgetc (stream.get ());
> > -      if (c == EOF)
> > +      /* Invariant: current_line_start is either the index
> > +        of the start of the current line in LINES *or* the
> > +        length of the source code (LINES, when there is nothing
> > +        else to do).  */
> > +      if (current_line_start == lines.length ())
> >         break;
> > -      do
> > -       {
> > -         buf.push_back (c);
> > -       }
> > -      while (c != '\n' && (c = fgetc (stream.get ())) >= 0);
> > +
> > +      size_t current_line_end = ((line_no + 1) > offsets->size ()
> > +                                ? lines.size () - 1
> > +                                : (*offsets)[line_no] - 1);
> > +
> > +      size_t sz = current_line_end - current_line_start;
> > +      buf = lines.substr (current_line_start, sz);
> >
> >        /* Remove the \r, if any, at the end of the line, otherwise
> >          regular expressions that end with $ or \n won't work.  */
> > -      size_t sz = buf.size ();
> >        if (sz >= 2 && buf[sz - 2] == '\r')
> >         {
> >           buf[sz - 2] = '\n';
> > @@ -1664,29 +1663,27 @@ search_command_helper (const char *regex, int from_tty, bool forward)
> >         }
> >
> >        /* We now have a source line in buf, null terminate and match.  */
> > -      buf.push_back ('\0');
> > +      buf += '\0';
> >        if (re_exec (buf.data ()) > 0)
> >         {
> >           /* Match!  */
> > -         print_source_lines (loc->symtab (), line, line + 1, 0);
> > -         set_internalvar_integer (lookup_internalvar ("_"), line);
> > -         loc->set (loc->symtab (), std::max (line - lines_to_list / 2, 1));
> > +         print_source_lines (loc->symtab (), line_no, line_no + 1, 0);
> > +         set_internalvar_integer (lookup_internalvar ("_"), line_no);
> > +         loc->set (loc->symtab (), std::max (line_no - lines_to_list / 2, 1));
> >           return;
> >         }
> >
> >        if (forward)
> > -       line++;
> > +       {
> > +         line_no++;
> > +         current_line_start = current_line_end + 1;
> > +       }
> >        else
> >         {
> > -         line--;
> > -         if (line < 1)
> > +         line_no--;
> > +         if (line_no < 1)
> >             break;
> > -         if (fseek (stream.get (), (*offsets)[line - 1], 0) < 0)
> > -           {
> > -             const char *filename
> > -               = symtab_to_filename_for_display (loc->symtab ());
> > -             perror_with_name (filename);
> > -           }
> > +         current_line_start = (*offsets)[line_no - 1];
> >         }
> >      }
> >
> > --
> > 2.44.0
> >