From: Tom Tromey <tromey@adacore.com>
Date: Tue, 23 Jul 2019 14:22:50 +0000 (-0600)
Subject: Fix latent bug in source cache
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=269249d9406096dd59aecd8845e960fdddb1ebfe;p=binutils-gdb.git

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 --git a/gdb/ChangeLog b/gdb/ChangeLog
index e1bd880d80c..9fb0abb2175 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+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.
diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index f5bb641a22b..f0cb6b80059 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 e2e25a170fd..a00efbf3fba 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;