diff mbox

[2/4] gdb: Handle requests to print source lines backward

Message ID fffae4824d7a9d78c1ed24f548b5d88bdfdc2dbd.1546860547.git.andrew.burgess@embecosm.com
State New
Headers show

Commit Message

Andrew Burgess Jan. 7, 2019, 12:42 p.m. UTC
...by which I mean from high line number to low, not, actually
backward character by character!

Commit:

  commit 62f29fda90cf1d5a1899f57ef78452471c707fd6
  Date:   Tue Oct 9 22:21:05 2018 -0600

      Highlight source code using GNU Source Highlight

introduced a regression in the test gdb.linespec/explicit.exp, in
which a request is made to GDB to print a reverse sequence of lines,
from +10 to -10 from the current line number.  The expected behaviour
is that GDB prints nothing.  The above commit changed this so that GDB
now prints:

  Line number 32 out of range; /path/to/gdb/testsuite/gdb.linespec/explicit.c has 71 lines.

which is a little confusing.

This commit fixes the regression, and restores the behaviour that GDB
prints nothing.

While I was passing I noticed a call to `back` on a std::string that I
was concerned could be empty if the request for source lines returns
an empty string.  I don't know if it would be possible for a request
for lines to return an empty string, I guess it should be impossible,
in which case, maybe this should be an assertion, but adding a `empty`
check, seems like an easy and cheap safety net.

gdb/ChangeLog:

	* source.c (print_source_lines_base): Handle requests to print
	reverse line number sequences, and guard against empty lines
	string.
---
 gdb/ChangeLog |  6 ++++++
 gdb/source.c  | 12 +++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

Comments

Tom Tromey Jan. 8, 2019, 5:20 a.m. UTC | #1
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> This commit fixes the regression, and restores the behaviour that GDB
Andrew> prints nothing.

Andrew> While I was passing I noticed a call to `back` on a std::string that I
Andrew> was concerned could be empty if the request for source lines returns
Andrew> an empty string.  I don't know if it would be possible for a request
Andrew> for lines to return an empty string, I guess it should be impossible,
Andrew> in which case, maybe this should be an assertion, but adding a `empty`
Andrew> check, seems like an easy and cheap safety net.

Seems reasonable.

Andrew> 	* source.c (print_source_lines_base): Handle requests to print
Andrew> 	reverse line number sequences, and guard against empty lines
Andrew> 	string.

This is ok...

Andrew> +  /* If the user requested a sequence of lines that seems to go backward
Andrew> +     (from high to low line numbers) then we don't print anything.
Andrew> +     The use of '- 1' here instead of '<=' is currently critical, we rely
Andrew> +     on the undefined wrap around behaviour of 'int' for stopline.  When
Andrew> +     the use has done: 'set listsize unlimited' then stopline can overflow
Andrew> +     and appear as MIN_INT.  This is a long-standing bug that needs
Andrew> +     fixing.  */

... assuming this was the previous status quo.

Tom
diff mbox

Patch

diff --git a/gdb/source.c b/gdb/source.c
index e77789c0dba..71da396acc8 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1346,6 +1346,16 @@  print_source_lines_base (struct symtab *s, int line, int stopline,
 
   last_source_error = 0;
 
+  /* If the user requested a sequence of lines that seems to go backward
+     (from high to low line numbers) then we don't print anything.
+     The use of '- 1' here instead of '<=' is currently critical, we rely
+     on the undefined wrap around behaviour of 'int' for stopline.  When
+     the use has done: 'set listsize unlimited' then stopline can overflow
+     and appear as MIN_INT.  This is a long-standing bug that needs
+     fixing.  */
+  if (stopline - 1 < line)
+    return;
+
   std::string lines;
   if (!g_source_cache.get_source_lines (s, line, stopline - 1, &lines))
     error (_("Line number %d out of range; %s has %d lines."),
@@ -1392,7 +1402,7 @@  print_source_lines_base (struct symtab *s, int line, int stopline,
       if (c == '\0')
 	break;
     }
-  if (lines.back () != '\n')
+  if (!lines.empty() && lines.back () != '\n')
     uiout->text ("\n");
 }