* [PATCH] Get rid of VEC(mem_range_s)
@ 2017-10-16 2:29 Simon Marchi
2017-10-16 12:58 ` Pedro Alves
0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2017-10-16 2:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
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/Makefile.in | 2 +
gdb/exec.c | 55 ++++++------------
gdb/memrange.c | 46 ++++-----------
gdb/memrange.h | 17 ++++--
gdb/mi/mi-main.c | 20 +++----
gdb/remote.c | 19 ++----
gdb/tracepoint.c | 13 ++---
gdb/tracepoint.h | 2 +-
gdb/unittests/memrange-selftests.c | 115 +++++++++++++++++++++++++++++++++++++
9 files changed, 179 insertions(+), 110 deletions(-)
create mode 100644 gdb/unittests/memrange-selftests.c
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0f1ba54..e21ea55 100644
--- a/gdb/Makefile.in
+++ b/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 \
diff --git a/gdb/exec.c b/gdb/exec.c
index 6eda9b2..2fa543b 100644
--- a/gdb/exec.c
+++ b/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
- 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;
diff --git a/gdb/memrange.c b/gdb/memrange.c
index 74da19d..34feac5 100644
--- a/gdb/memrange.c
+++ b/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;
- 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);
}
}
diff --git a/gdb/memrange.h b/gdb/memrange.h
index 029ec71..fb10cda 100644
--- a/gdb/memrange.h
+++ b/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
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index a94e329..8dc955d 100644
--- a/gdb/mi/mi-main.c
+++ b/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;
- 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);
}
}
diff --git a/gdb/remote.c b/gdb/remote.c
index e2bdd11..b38ace9 100644
--- a/gdb/remote.c
+++ b/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;
-
- 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;
}
}
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 9c07315..fdc3b38 100644
--- a/gdb/tracepoint.c
+++ b/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);
- nr->length = std::min (hi1, hi2) - nr->start;
+ result->emplace_back (start, length);
}
- normalize_mem_ranges (*result);
+ normalize_mem_ranges (result);
return 1;
}
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 88c18c3..8364d38 100644
--- a/gdb/tracepoint.h
+++ b/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);
diff --git a/gdb/unittests/memrange-selftests.c b/gdb/unittests/memrange-selftests.c
new file mode 100644
index 0000000..6487578
--- /dev/null
+++ b/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 (void)
+{
+ /* 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);
+}
--
2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Get rid of VEC(mem_range_s)
2017-10-16 2:29 [PATCH] Get rid of VEC(mem_range_s) Simon Marchi
@ 2017-10-16 12:58 ` Pedro Alves
2017-10-16 15:07 ` Simon Marchi
0 siblings, 1 reply; 3+ messages in thread
From: Pedro Alves @ 2017-10-16 12:58 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 10/16/2017 03:28 AM, Simon Marchi wrote:
> 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.
Excellent! Thanks for doing that. LGTM.
> +static void
> +normalize_mem_ranges_tests (void)
(void) -> ()
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Get rid of VEC(mem_range_s)
2017-10-16 12:58 ` Pedro Alves
@ 2017-10-16 15:07 ` Simon Marchi
0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2017-10-16 15:07 UTC (permalink / raw)
To: Pedro Alves; +Cc: Simon Marchi, gdb-patches
On 2017-10-16 08:58, Pedro Alves wrote:
> On 10/16/2017 03:28 AM, Simon Marchi wrote:
>> 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.
>
> Excellent! Thanks for doing that. LGTM.
>
>> +static void
>> +normalize_mem_ranges_tests (void)
>
> (void) -> ()
Thanks, pushed with that fixed.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-16 15:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-16 2:29 [PATCH] Get rid of VEC(mem_range_s) Simon Marchi
2017-10-16 12:58 ` Pedro Alves
2017-10-16 15:07 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).