Re: [RFC 8.3 3/3] Avoid a crash in source_cache::extract_lines

Pedro Alves
2019-03-13T17:07:07Z

On 03/08/2019 09:04 PM, Tom Tromey wrote:
> If the first requested line is larger than the number of lines in the
> source buffer, source_cache::extract_lines could crash, because it
> would try to pass string::npos" to string::substr.
>
> This patch avoids the crash by checking for this case.

Can you clarify how can first_pos end up as npos? Is that a bug in the
caller, or is it normal? The documentation doesn't seem to allow for that:

/* Get the source text for the source file in symtab S. FIRST_LINE
and LAST_LINE are the first and last lines to return; line
numbers are 1-based. If the file cannot be read, false is
returned. Otherwise, LINES_OUT is set to the desired text. The
returned text may include ANSI terminal escapes. */

>
> gdb/ChangeLog
> 2019-03-08 Tom Tromey <tromey@adacore.com>
>
> * source-cache.c (source_cache::extract_lines): Handle case where
> first_pos==npos.
> ---
> gdb/ChangeLog | 5 +++++
> gdb/source-cache.c | 2 ++
> 2 files changed, 7 insertions(+)
>
> diff --git a/gdb/source-cache.c b/gdb/source-cache.c
> index 27a0ade959c..b5d0d6cb7fc 100644
> --- a/gdb/source-cache.c
> +++ b/gdb/source-cache.c
> @@ -98,6 +98,8 @@ source_cache::extract_lines (const struct source_text &text, int first_line,
> {
> if (pos == std::string::npos)
> pos = text.contents.size ();
> + if (first_pos == std::string::npos)
> + first_pos = text.contents.size ();
> *lines = text.contents.substr (first_pos, pos - first_pos);
> return true;
> }
>

Thanks,
Pedro Alves