Get rid of VEC(mem_range_s)
authorSimon Marchi <simon.marchi@ericsson.com>
Mon, 16 Oct 2017 14:14:53 +0000 (10:14 -0400)
committerSimon Marchi <simon.marchi@ericsson.com>
Mon, 16 Oct 2017 15:07:18 +0000 (11:07 -0400)
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
gdb/Makefile.in
gdb/exec.c
gdb/memrange.c
gdb/memrange.h
gdb/mi/mi-main.c
gdb/remote.c
gdb/tracepoint.c
gdb/tracepoint.h
gdb/unittests/memrange-selftests.c [new file with mode: 0644]

index cc81af8ee6e061b6e5d4225ac2b938008f4f48d6..169304a832e79aa31bc248e5e0d8310813bccf9e 100644 (file)
@@ -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.
index 0f1ba54f799f8fbf706bba14e64189b70ad9cd97..e21ea55197fb2735762d540fba54de8c8b71bdf0 100644 (file)
@@ -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 \
index 6eda9b2a61d499f9115738eff593086b6972361e..2fa543b7a429f03aa140c781054959005c20a616 100644 (file)
@@ -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
-   VEC.  */
+   intersected with) MEMADDR for LEN bytes.  */
 
-static VEC(mem_range_s) *
-section_table_available_memory (VEC(mem_range_s) *memory,
-                               CORE_ADDR memaddr, ULONGEST len,
+static std::vector<mem_range>
+section_table_available_memory (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);
-         r->length = std::min (hi1, hi2) - r->start;
+         memory.emplace_back (start, length);
        }
     }
 
@@ -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;
-  struct target_section_table *table;
-  struct cleanup *old_chain;
-  mem_range_s *r;
-  int i;
+  target_section_table *table = target_get_section_table (&exec_ops);
+  std::vector<mem_range> available_memory
+    = section_table_available_memory (offset, len,
+                                     table->sections, table->sections_end);
 
-  table = target_get_section_table (&exec_ops);
-  available_memory = section_table_available_memory (available_memory,
-                                                    offset, len,
-                                                    table->sections,
-                                                    table->sections_end);
+  normalize_mem_ranges (&available_memory);
 
-  old_chain = make_cleanup (VEC_cleanup(mem_range_s),
-                           &available_memory);
-
-  normalize_mem_ranges (available_memory);
-
-  for (i = 0;
-       VEC_iterate (mem_range_s, available_memory, i, r);
-       i++)
+  for (const mem_range &r : available_memory)
     {
-      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;
index 74da19d1cd7874ccb48984ea1bcba3ea5ce6fc4e..34feac53abf6906762720798e2b2470e239bc21b 100644 (file)
@@ -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;
-      int a, b;
+      std::vector<mem_range> &m = *memory;
 
-      qsort (VEC_address (mem_range_s, ranges),
-            VEC_length (mem_range_s, ranges),
-            sizeof (mem_range_s),
-            compare_mem_ranges);
+      std::sort (m.begin (), m.end ());
 
-      a = 0;
-      ra = VEC_index (mem_range_s, ranges, a);
-      for (b = 1; VEC_iterate (mem_range_s, ranges, b, rb); b++)
+      int a = 0;
+      for (int b = 1; b < m.size (); 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,
-                               (rb->start - ra->start) + rb->length);
+             m[a].length = std::max ((CORE_ADDR) m[a].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);
     }
 }
index 029ec7104c5c6c7a92d5819d25fda064113e960c..fb10cda2b7c80bd139ece3a0e6a2641ff7b976c9 100644 (file)
@@ -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
index a94e329af8fe9d912be09f906bb1b6f57d80d51c..8dc955da1ffebf258a76b73c521834cfac4dabcc 100644 (file)
@@ -2752,40 +2752,34 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 
   /* Memory.  */
   {
-    struct cleanup *cleanups;
-    VEC(mem_range_s) *available_memory = NULL;
-    struct mem_range *r;
-    int i;
+    std::vector<mem_range> available_memory;
 
     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_int ("length", r->length);
+       uiout->field_core_addr ("address", gdbarch, r.start);
+       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);
   }
 }
 
index e2bdd115f917d27f463b92ac2146ef62b613393d..b38ace991d40b8076fe9467d0e075f0be1dfe77c 100644 (file)
@@ -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;
-
-         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)
+         if (available.empty () || 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;
-
-         do_cleanups (old_chain);
+         len = available[0].length;
        }
     }
 
index 9c073157b915168d3680a68f118be52aeba8bae7..fdc3b38490b188bb650a10d13ce0861bd1ab16d4 100644 (file)
@@ -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);
-           nr->length = std::min (hi1, hi2) - nr->start;
+           result->emplace_back (start, length);
          }
 
-      normalize_mem_ranges (*result);
+      normalize_mem_ranges (result);
       return 1;
     }
 
index 88c18c33d41194da12de7097ee261d9d89f7e302..8364d38da20a0e2cfbc40748d35578baf6815e3a 100644 (file)
@@ -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);
diff --git a/gdb/unittests/memrange-selftests.c b/gdb/unittests/memrange-selftests.c
new file mode 100644 (file)
index 0000000..e4349b6
--- /dev/null
@@ -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);
+}