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

[Will Hawkins <hawkinsw@obs.cr>]

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
>