Get rid of VEC(mem_range_s)

This patch replaces the last usages of VEC(mem_range_s) with
std::vector<mem_range>.  This allows getting rid of a few cleanups and
of the DEF_VEC_O(mem_range_s).

I added a test for normalize_mem_ranges to make sure I didn't break
anything there.

Regtested on the buildbot.

gdb/ChangeLog:

	* memrange.h (struct mem_range): Define operator< and operator==.
	(mem_range_s): Remove.
	(DEF_VEC_O (mem_range_s)): Remove.
	(normalize_mem_ranges): Change parameter type to std::vector.
	* memrange.c (compare_mem_ranges): Remove.
	(normalize_mem_ranges): Change parameter type to std::vector,
	adjust to vector change.
	* exec.c (section_table_available_memory): Return vector, remove
	parameter.
	(section_table_read_available_memory): Adjust to std::vector
	change.
	* remote.c (remote_read_bytes): Adjust to std::vector
	change.
	* tracepoint.h (traceframe_available_memory): Change parameter
	type to std::vector.
	* tracepoint.c (traceframe_available_memory): Change parameter
	type to std::vector, adjust.
	* gdb/mi/mi-main.c (mi_cmd_trace_frame_collected): Adjust to
	std::vector change.
	* gdb/Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
	unittests/memrange-selftests.c.
	(SUBDIR_UNITTESTS_OBS): Add memrange-selftests.o.
	* gdb/unittests/memrange-selftests.c: New file.

gdb/ChangeLog

@@ -1,3 +1,29 @@
+2017-10-16  Simon Marchi  <simon.marchi@ericsson.com>
+
+	* memrange.h (struct mem_range): Define operator< and operator==.
+	(mem_range_s): Remove.
+	(DEF_VEC_O (mem_range_s)): Remove.
+	(normalize_mem_ranges): Change parameter type to std::vector.
+	* memrange.c (compare_mem_ranges): Remove.
+	(normalize_mem_ranges): Change parameter type to std::vector,
+	adjust to vector change.
+	* exec.c (section_table_available_memory): Return vector, remove
+	parameter.
+	(section_table_read_available_memory): Adjust to std::vector
+	change.
+	* remote.c (remote_read_bytes): Adjust to std::vector
+	change.
+	* tracepoint.h (traceframe_available_memory): Change parameter
+	type to std::vector.
+	* tracepoint.c (traceframe_available_memory): Change parameter
+	type to std::vector, adjust.
+	* gdb/mi/mi-main.c (mi_cmd_trace_frame_collected): Adjust to
+	std::vector change.
+	* gdb/Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
+	unittests/memrange-selftests.c.
+	(SUBDIR_UNITTESTS_OBS): Add memrange-selftests.o.
+	* gdb/unittests/memrange-selftests.c: New file.
+
 2017-10-16  Pedro Alves  <palves@redhat.com>
 
 	* elfread.c (probe_key_free): Rename range-for variable.

gdb/Makefile.in

@@ -530,6 +530,7 @@ SUBDIR_UNITTESTS_SRCS = \
 	unittests/common-utils-selftests.c \
 	unittests/environ-selftests.c \
 	unittests/function-view-selftests.c \
+	unittests/memrange-selftests.c \
 	unittests/offset-type-selftests.c \
 	unittests/optional-selftests.c \
 	unittests/ptid-selftests.c \
@@ -541,6 +542,7 @@ SUBDIR_UNITTESTS_OBS = \
 	common-utils-selftests.o \
 	environ-selftests.o \
 	function-view-selftests.o \
+	memrange-selftests.o \
 	offset-type-selftests.o \
 	optional-selftests.o \
 	ptid-selftests.o \

gdb/exec.c

@@ -698,20 +698,18 @@ exec_read_partial_read_only (gdb_byte *readbuf, ULONGEST offset,
   return TARGET_XFER_E_IO;
 }
 
-/* Appends all read-only memory ranges found in the target section
+/* Return all read-only memory ranges found in the target section
    table defined by SECTIONS and SECTIONS_END, starting at (and
-   intersected with) MEMADDR for LEN bytes.  Returns the augmented
+   intersected with) MEMADDR for LEN bytes.  */
-   VEC.  */
 
-static VEC(mem_range_s) *
+static std::vector<mem_range>
-section_table_available_memory (VEC(mem_range_s) *memory,
+section_table_available_memory (CORE_ADDR memaddr, ULONGEST len,
-				CORE_ADDR memaddr, ULONGEST len,
 				struct target_section *sections,
 				struct target_section *sections_end)
 {
-  struct target_section *p;
+  std::vector<mem_range> memory;
 
-  for (p = sections; p < sections_end; p++)
+  for (target_section *p = sections; p < sections_end; p++)
     {
       if ((bfd_get_section_flags (p->the_bfd_section->owner,
 				  p->the_bfd_section)
@@ -722,7 +720,6 @@ section_table_available_memory (VEC(mem_range_s) *memory,
       if (mem_ranges_overlap (p->addr, p->endaddr - p->addr, memaddr, len))
 	{
 	  ULONGEST lo1, hi1, lo2, hi2;
-	  struct mem_range *r;
 
 	  lo1 = memaddr;
 	  hi1 = memaddr + len;
@@ -730,10 +727,10 @@ section_table_available_memory (VEC(mem_range_s) *memory,
 	  lo2 = p->addr;
 	  hi2 = p->endaddr;
 
-	  r = VEC_safe_push (mem_range_s, memory, NULL);
+	  CORE_ADDR start = std::max (lo1, lo2);
+	  int length = std::min (hi1, hi2) - start;
 
-	  r->start = std::max (lo1, lo2);
+	  memory.emplace_back (start, length);
-	  r->length = std::min (hi1, hi2) - r->start;
 	}
     }
 
@@ -744,51 +741,37 @@ enum target_xfer_status
 section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset,
 				     ULONGEST len, ULONGEST *xfered_len)
 {
-  VEC(mem_range_s) *available_memory = NULL;
+  target_section_table *table = target_get_section_table (&exec_ops);
-  struct target_section_table *table;
+  std::vector<mem_range> available_memory
-  struct cleanup *old_chain;
+    = section_table_available_memory (offset, len,
-  mem_range_s *r;
+				      table->sections, table->sections_end);
-  int i;
 
-  table = target_get_section_table (&exec_ops);
+  normalize_mem_ranges (&available_memory);
-  available_memory = section_table_available_memory (available_memory,
-						     offset, len,
-						     table->sections,
-						     table->sections_end);
 
-  old_chain = make_cleanup (VEC_cleanup(mem_range_s),
+  for (const mem_range &r : available_memory)
-			    &available_memory);
-
-  normalize_mem_ranges (available_memory);
-
-  for (i = 0;
-       VEC_iterate (mem_range_s, available_memory, i, r);
-       i++)
     {
-      if (mem_ranges_overlap (r->start, r->length, offset, len))
+      if (mem_ranges_overlap (r.start, r.length, offset, len))
 	{
 	  CORE_ADDR end;
 	  enum target_xfer_status status;
 
 	  /* Get the intersection window.  */
-	  end = std::min<CORE_ADDR> (offset + len, r->start + r->length);
+	  end = std::min<CORE_ADDR> (offset + len, r.start + r.length);
 
 	  gdb_assert (end - offset <= len);
 
-	  if (offset >= r->start)
+	  if (offset >= r.start)
 	    status = exec_read_partial_read_only (readbuf, offset,
 						  end - offset,
 						  xfered_len);
 	  else
 	    {
-	      *xfered_len = r->start - offset;
+	      *xfered_len = r.start - offset;
 	      status = TARGET_XFER_UNAVAILABLE;
 	    }
-	  do_cleanups (old_chain);
 	  return status;
 	}
     }
-  do_cleanups (old_chain);
 
   *xfered_len = len;
   return TARGET_XFER_UNAVAILABLE;

gdb/memrange.c

@@ -41,58 +41,36 @@ address_in_mem_range (CORE_ADDR address, const struct mem_range *r)
 	  && (address - r->start) < r->length);
 }
 
-/* qsort comparison function, that compares mem_ranges.  Ranges are
-   sorted in ascending START order.  */
-
-static int
-compare_mem_ranges (const void *ap, const void *bp)
-{
-  const struct mem_range *r1 = (const struct mem_range *) ap;
-  const struct mem_range *r2 = (const struct mem_range *) bp;
-
-  if (r1->start > r2->start)
-    return 1;
-  else if (r1->start < r2->start)
-    return -1;
-  else
-    return 0;
-}
-
 void
-normalize_mem_ranges (VEC(mem_range_s) *ranges)
+normalize_mem_ranges (std::vector<mem_range> *memory)
 {
   /* This function must not use any VEC operation on RANGES that
      reallocates the memory block as that invalidates the RANGES
      pointer, which callers expect to remain valid.  */
 
-  if (!VEC_empty (mem_range_s, ranges))
+  if (!memory->empty ())
     {
-      struct mem_range *ra, *rb;
+      std::vector<mem_range> &m = *memory;
-      int a, b;
 
-      qsort (VEC_address (mem_range_s, ranges),
+      std::sort (m.begin (), m.end ());
-	     VEC_length (mem_range_s, ranges),
-	     sizeof (mem_range_s),
-	     compare_mem_ranges);
 
-      a = 0;
+      int a = 0;
-      ra = VEC_index (mem_range_s, ranges, a);
+      for (int b = 1; b < m.size (); b++)
-      for (b = 1; VEC_iterate (mem_range_s, ranges, b, rb); b++)
 	{
 	  /* If mem_range B overlaps or is adjacent to mem_range A,
 	     merge them.  */
-	  if (rb->start <= ra->start + ra->length)
+	  if (m[b].start <= m[a].start + m[a].length)
 	    {
-	      ra->length = std::max ((CORE_ADDR) ra->length,
+	      m[a].length = std::max ((CORE_ADDR) m[a].length,
-				(rb->start - ra->start) + rb->length);
+				      (m[b].start - m[a].start) + m[b].length);
 	      continue;		/* next b, same a */
 	    }
 	  a++;			/* next a */
-	  ra = VEC_index (mem_range_s, ranges, a);
 
 	  if (a != b)
-	    *ra = *rb;
+	    m[a] = m[b];
 	}
-      VEC_truncate (mem_range_s, ranges, a + 1);
+
+      m.resize (a + 1);
     }
 }

gdb/memrange.h

@@ -32,6 +32,17 @@ struct mem_range
   : start (start_), length (length_)
   {}
 
+  bool operator< (const mem_range &other) const
+  {
+    return this->start < other.start;
+  }
+
+  bool operator== (const mem_range &other) const
+  {
+    return (this->start == other.start
+	    && this->length == other.length);
+  }
+
   /* Lowest address in the range.  */
   CORE_ADDR start;
 
@@ -39,10 +50,6 @@ struct mem_range
   int length;
 };
 
-typedef struct mem_range mem_range_s;
-
-DEF_VEC_O(mem_range_s);
-
 /* Returns true if the ranges defined by [start1, start1+len1) and
    [start2, start2+len2) overlap.  */
 
@@ -57,6 +64,6 @@ extern int address_in_mem_range (CORE_ADDR addr,
 /* Sort ranges by start address, then coalesce contiguous or
    overlapping ranges.  */
 
-extern void normalize_mem_ranges (VEC(mem_range_s) *memory);
+extern void normalize_mem_ranges (std::vector<mem_range> *memory);
 
 #endif

gdb/mi/mi-main.c

@@ -2752,40 +2752,34 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 
   /* Memory.  */
   {
-    struct cleanup *cleanups;
+    std::vector<mem_range> available_memory;
-    VEC(mem_range_s) *available_memory = NULL;
-    struct mem_range *r;
-    int i;
 
     traceframe_available_memory (&available_memory, 0, ULONGEST_MAX);
-    cleanups = make_cleanup (VEC_cleanup(mem_range_s), &available_memory);
 
     ui_out_emit_list list_emitter (uiout, "memory");
 
-    for (i = 0; VEC_iterate (mem_range_s, available_memory, i, r); i++)
+    for (const mem_range &r : available_memory)
       {
 	struct gdbarch *gdbarch = target_gdbarch ();
 
 	ui_out_emit_tuple tuple_emitter (uiout, NULL);
 
-	uiout->field_core_addr ("address", gdbarch, r->start);
+	uiout->field_core_addr ("address", gdbarch, r.start);
-	uiout->field_int ("length", r->length);
+	uiout->field_int ("length", r.length);
 
-	gdb::byte_vector data (r->length);
+	gdb::byte_vector data (r.length);
 
 	if (memory_contents)
 	  {
-	    if (target_read_memory (r->start, data.data (), r->length) == 0)
+	    if (target_read_memory (r.start, data.data (), r.length) == 0)
 	      {
-		std::string data_str = bin2hex (data.data (), r->length);
+		std::string data_str = bin2hex (data.data (), r.length);
 		uiout->field_string ("contents", data_str.c_str ());
 	      }
 	    else
 	      uiout->field_skip ("contents");
 	  }
       }
-
-    do_cleanups (cleanups);
   }
 }
 

gdb/remote.c

@@ -8465,7 +8465,7 @@ remote_read_bytes (struct target_ops *ops, CORE_ADDR memaddr,
 
   if (get_traceframe_number () != -1)
     {
-      VEC(mem_range_s) *available;
+      std::vector<mem_range> available;
 
       /* If we fail to get the set of available memory, then the
 	 target does not support querying traceframe info, and so we
@@ -8473,27 +8473,20 @@ remote_read_bytes (struct target_ops *ops, CORE_ADDR memaddr,
 	 target implements the old QTro packet then).  */
       if (traceframe_available_memory (&available, memaddr, len))
 	{
-	  struct cleanup *old_chain;
+	  if (available.empty () || available[0].start != memaddr)
-
-	  old_chain = make_cleanup (VEC_cleanup(mem_range_s), &available);
-
-	  if (VEC_empty (mem_range_s, available)
-	      || VEC_index (mem_range_s, available, 0)->start != memaddr)
 	    {
 	      enum target_xfer_status res;
 
 	      /* Don't read into the traceframe's available
 		 memory.  */
-	      if (!VEC_empty (mem_range_s, available))
+	      if (!available.empty ())
 		{
 		  LONGEST oldlen = len;
 
-		  len = VEC_index (mem_range_s, available, 0)->start - memaddr;
+		  len = available[0].start - memaddr;
 		  gdb_assert (len <= oldlen);
 		}
 
-	      do_cleanups (old_chain);
-
 	      /* This goes through the topmost target again.  */
 	      res = remote_xfer_live_readonly_partial (ops, myaddr, memaddr,
 						       len, unit_size, xfered_len);
@@ -8512,9 +8505,7 @@ remote_read_bytes (struct target_ops *ops, CORE_ADDR memaddr,
 	     case the target implements the deprecated QTro packet to
 	     cater for older GDBs (the target's knowledge of read-only
 	     sections may be outdated by now).  */
-	  len = VEC_index (mem_range_s, available, 0)->length;
+	  len = available[0].length;
-
-	  do_cleanups (old_chain);
 	}
     }
 

gdb/tracepoint.c

@@ -4077,20 +4077,19 @@ get_traceframe_info (void)
    undefined.  */
 
 int
-traceframe_available_memory (VEC(mem_range_s) **result,
+traceframe_available_memory (std::vector<mem_range> *result,
 			     CORE_ADDR memaddr, ULONGEST len)
 {
   struct traceframe_info *info = get_traceframe_info ();
 
   if (info != NULL)
     {
-      *result = NULL;
+      result->clear ();
 
       for (mem_range &r : info->memory)
 	if (mem_ranges_overlap (r.start, r.length, memaddr, len))
 	  {
 	    ULONGEST lo1, hi1, lo2, hi2;
-	    struct mem_range *nr;
 
 	    lo1 = memaddr;
 	    hi1 = memaddr + len;
@@ -4098,13 +4097,13 @@ traceframe_available_memory (VEC(mem_range_s) **result,
 	    lo2 = r.start;
 	    hi2 = r.start + r.length;
 
-	    nr = VEC_safe_push (mem_range_s, *result, NULL);
+	    CORE_ADDR start = std::max (lo1, lo2);
+	    int length = std::min (hi1, hi2) - start;
 
-	    nr->start = std::max (lo1, lo2);
+	    result->emplace_back (start, length);
-	    nr->length = std::min (hi1, hi2) - nr->start;
 	  }
 
-      normalize_mem_ranges (*result);
+      normalize_mem_ranges (result);
       return 1;
     }
 

gdb/tracepoint.h

@@ -400,7 +400,7 @@ extern void trace_save_ctf (const char *dirname,
 
 extern traceframe_info_up parse_traceframe_info (const char *tframe_info);
 
-extern int traceframe_available_memory (VEC(mem_range_s) **result,
+extern int traceframe_available_memory (std::vector<mem_range> *result,
 					CORE_ADDR memaddr, ULONGEST len);
 
 extern struct traceframe_info *get_traceframe_info (void);

gdb/unittests/memrange-selftests.c

@@ -0,0 +1,115 @@
+/* Self tests for mem ranges for GDB, the GNU debugger.
+
+   Copyright (C) 2017 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "selftest.h"
+#include "memrange.h"
+
+namespace selftests {
+namespace memrange_tests {
+
+static void
+normalize_mem_ranges_tests ()
+{
+  /* Empty vector.  */
+  {
+    std::vector<mem_range> ranges;
+
+    normalize_mem_ranges (&ranges);
+
+    SELF_CHECK (ranges.size () == 0);
+  }
+
+  /* With one range.  */
+  {
+    std::vector<mem_range> ranges;
+
+    ranges.emplace_back (10, 20);
+
+    normalize_mem_ranges (&ranges);
+
+    SELF_CHECK (ranges.size () == 1);
+    SELF_CHECK (ranges[0] == mem_range (10, 20));
+  }
+
+  /* Completely disjoint ranges.  */
+  {
+    std::vector<mem_range> ranges;
+
+    ranges.emplace_back (20, 1);
+    ranges.emplace_back (10, 1);
+
+    normalize_mem_ranges (&ranges);
+
+    SELF_CHECK (ranges.size () == 2);
+    SELF_CHECK (ranges[0] == mem_range (10, 1));
+    SELF_CHECK (ranges[1] == mem_range (20, 1));
+  }
+
+  /* Overlapping and contiguous ranges.  */
+  {
+    std::vector<mem_range> ranges;
+
+    ranges.emplace_back (5, 10);
+    ranges.emplace_back (10, 10);
+    ranges.emplace_back (15, 10);
+
+    normalize_mem_ranges (&ranges);
+
+    SELF_CHECK (ranges.size () == 1);
+    SELF_CHECK (ranges[0] == mem_range (5, 20));
+  }
+
+  /* Duplicate ranges.  */
+  {
+    std::vector<mem_range> ranges;
+
+    ranges.emplace_back (10, 10);
+    ranges.emplace_back (10, 10);
+
+    normalize_mem_ranges (&ranges);
+
+    SELF_CHECK (ranges.size () == 1);
+    SELF_CHECK (ranges[0] == mem_range (10, 10));
+  }
+
+  /* Range completely inside another.  */
+  {
+    std::vector<mem_range> ranges;
+
+    ranges.emplace_back (14, 2);
+    ranges.emplace_back (10, 10);
+
+    normalize_mem_ranges (&ranges);
+
+    SELF_CHECK (ranges.size () == 1);
+    SELF_CHECK (ranges[0] == mem_range (10, 10));
+  }
+}
+
+} /* namespace memrange_tests */
+} /* namespace selftests */
+
+void
+_initialize_memrange_selftests ()
+{
+  selftests::register_test
+    ("normalize_mem_ranges",
+     selftests::memrange_tests::normalize_mem_ranges_tests);
+}