[binutils-gdb] Fix latent bug in source cache

[Tom Tromey]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=269249d9406096dd59aecd8845e960fdddb1ebfe

commit 269249d9406096dd59aecd8845e960fdddb1ebfe
Author: Tom Tromey <tromey@adacore.com>
Date:   Tue Jul 23 08:22:50 2019 -0600

    Fix latent bug in source cache
    
    The source cache was not returning the final \n of the requested range
    of lines.  This caused regressions with later patches in this series,
    so this patch pre-emptively fixes the bug.
    
    This adds a self-test of "extract_lines" to the source cache code.  To
    make it simpler to test, I changed extract_lines to be a static
    function, and changed it's API a bit.
    
    gdb/ChangeLog
    2019-08-06  Tom Tromey  <tromey@adacore.com>
    
    	* source-cache.c (extract_lines): No longer a method.
    	Changed type of parameter.  Include final newline.
    	(selftests::extract_lines_test): New function.
    	(_initialize_source_cache): Likewise.
    	* source-cache.h (class source_cache)
    	<extract_lines>: Don't declare.

Diff:
---
 gdb/ChangeLog      |  9 +++++++++
 gdb/source-cache.c | 49 ++++++++++++++++++++++++++++++++++++++-----------
 gdb/source-cache.h |  6 ------
 3 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e1bd880..9fb0abb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2019-08-06  Tom Tromey  <tromey@adacore.com>
 
+	* source-cache.c (extract_lines): No longer a method.
+	Changed type of parameter.  Include final newline.
+	(selftests::extract_lines_test): New function.
+	(_initialize_source_cache): Likewise.
+	* source-cache.h (class source_cache)
+	<extract_lines>: Don't declare.
+
+2019-08-06  Tom Tromey  <tromey@adacore.com>
+
 	* breakpoint.c (init_breakpoint_sal): Update.
 	(breakpoint): Update.
 	* breakpoint.h (struct breakpoint) <filter>: Now a
diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index f5bb641..f0cb6b8 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -22,6 +22,7 @@
 #include "source.h"
 #include "cli/cli-style.h"
 #include "symtab.h"
+#include "gdbsupport/selftest.h"
 
 #ifdef HAVE_SOURCE_HIGHLIGHT
 /* If Gnulib redirects 'open' and 'close' to its replacements
@@ -80,11 +81,12 @@ source_cache::get_plain_source_lines (struct symtab *s, int first_line,
   return true;
 }
 
-/* See source-cache.h.  */
-
-std::string
-source_cache::extract_lines (const struct source_text &text, int first_line,
-			     int last_line)
+/* A helper function for get_plain_source_lines that extracts the
+   desired source lines from TEXT, putting them into LINES_OUT.  The
+   arguments are as for get_source_lines.  The return value is the
+   desired lines.  */
+static std::string
+extract_lines (const std::string &text, int first_line, int last_line)
 {
   int lineno = 1;
   std::string::size_type pos = 0;
@@ -92,7 +94,7 @@ source_cache::extract_lines (const struct source_text &text, int first_line,
 
   while (pos != std::string::npos && lineno <= last_line)
     {
-      std::string::size_type new_pos = text.contents.find ('\n', pos);
+      std::string::size_type new_pos = text.find ('\n', pos);
 
       if (lineno == first_line)
 	first_pos = pos;
@@ -103,8 +105,10 @@ source_cache::extract_lines (const struct source_text &text, int first_line,
 	  if (first_pos == std::string::npos)
 	    return {};
 	  if (pos == std::string::npos)
-	    pos = text.contents.size ();
-	  return text.contents.substr (first_pos, pos - first_pos);
+	    pos = text.size ();
+	  else
+	    ++pos;
+	  return text.substr (first_pos, pos - first_pos);
 	}
       ++lineno;
       ++pos;
@@ -187,7 +191,7 @@ source_cache::get_source_lines (struct symtab *s, int first_line,
 	{
 	  if (item.fullname == fullname)
 	    {
-	      *lines = extract_lines (item, first_line, last_line);
+	      *lines = extract_lines (item.contents, first_line, last_line);
 	      return true;
 	    }
 	}
@@ -233,8 +237,8 @@ source_cache::get_source_lines (struct symtab *s, int first_line,
 	      if (m_source_map.size () > MAX_ENTRIES)
 		m_source_map.erase (m_source_map.begin ());
 
-	      *lines = extract_lines (m_source_map.back (), first_line,
-				      last_line);
+	      *lines = extract_lines (m_source_map.back ().contents,
+				      first_line, last_line);
 	      return true;
 	    }
 	}
@@ -243,3 +247,26 @@ source_cache::get_source_lines (struct symtab *s, int first_line,
 
   return get_plain_source_lines (s, first_line, last_line, lines);
 }
+
+#if GDB_SELF_TEST
+namespace selftests
+{
+static void extract_lines_test ()
+{
+  std::string input_text = "abc\ndef\nghi\njkl\n";
+
+  SELF_CHECK (extract_lines (input_text, 1, 1) == "abc\n");
+  SELF_CHECK (extract_lines (input_text, 2, 1) == "");
+  SELF_CHECK (extract_lines (input_text, 1, 2) == "abc\ndef\n");
+  SELF_CHECK (extract_lines ("abc", 1, 1) == "abc");
+}
+}
+#endif
+
+void
+_initialize_source_cache ()
+{
+#if GDB_SELF_TEST
+  selftests::register_test ("source-cache", selftests::extract_lines_test);
+#endif
+}
diff --git a/gdb/source-cache.h b/gdb/source-cache.h
index e2e25a1..a00efbf 100644
--- a/gdb/source-cache.h
+++ b/gdb/source-cache.h
@@ -63,12 +63,6 @@ private:
      are as for get_source_lines.  */
   bool get_plain_source_lines (struct symtab *s, int first_line,
 			       int last_line, std::string *lines_out);
-  /* A helper function for get_plain_source_lines that extracts the
-     desired source lines from TEXT, putting them into LINES_OUT.  The
-     arguments are as for get_source_lines.  The return value is the
-     desired lines.  */
-  std::string extract_lines (const struct source_text &text, int first_line,
-			     int last_line);
 
   /* The contents of the cache.  */
   std::vector<source_text> m_source_map;