[PATCH 2/2] Fix mem region parsing regression and add test

Simon Marchi
2017-11-09T23:18:44Z

In my patch

Get rid of VEC (mem_region)
a664f67e50eff30198097d51cec0ec4690abb2a1

I introduced a regression, where the length of the memory region is
assigned to the "hi" field. It should obviously be computed as "start +
length". To my defense, no test had caught this :). As a penance, I
wrote one.

gdb/ChangeLog:

* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
memory-map-selftests.c.
(SUBDIR_UNITTESTS_OBS): Add memory-map-selftests.o.
* memory-map.c (memory_map_start_memory): Fix computation of hi
address.
* unittests/memory-map-selftests.c: New file.
---
gdb/Makefile.in | 2 +
gdb/memory-map.c | 2 +-
gdb/unittests/memory-map-selftests.c | 81 ++++++++++++++++++++++++++++++++++++
3 files changed, 84 insertions(+), 1 deletion(-)
create mode 100644 gdb/unittests/memory-map-selftests.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 5e01816..a3bfbf9 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -531,6 +531,7 @@ SUBDIR_UNITTESTS_SRCS = \
unittests/environ-selftests.c \
unittests/function-view-selftests.c \
unittests/lookup_name_info-selftests.c \
+ unittests/memory-map-selftests.c \
unittests/memrange-selftests.c \
unittests/offset-type-selftests.c \
unittests/optional-selftests.c \
@@ -544,6 +545,7 @@ SUBDIR_UNITTESTS_OBS = \
environ-selftests.o \
function-view-selftests.o \
lookup_name_info-selftests.o \
+ memory-map-selftests.o \
memrange-selftests.o \
offset-type-selftests.o \
optional-selftests.o \
diff --git a/gdb/memory-map.c b/gdb/memory-map.c
index 9582ceb..e098209 100644
--- a/gdb/memory-map.c
+++ b/gdb/memory-map.c
@@ -71,7 +71,7 @@ memory_map_start_memory (struct gdb_xml_parser *parser,
type_p
= (ULONGEST *) xml_find_attribute (attributes, "type")->value;

- data->memory_map->emplace_back (*start_p, *length_p,
+ data->memory_map->emplace_back (*start_p, *start_p + *length_p,
(enum mem_access_mode) *type_p);
}

diff --git a/gdb/unittests/memory-map-selftests.c b/gdb/unittests/memory-map-selftests.c
new file mode 100644
index 0000000..3b282b5
--- /dev/null
+++ b/gdb/unittests/memory-map-selftests.c
@@ -0,0 +1,81 @@
+/* Self tests for memory-map 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 "memory-map.h"
+
+namespace selftests {
+namespace memory_map_tests {
+
+/* A simple valid test input for parse_memory_map. */
+
+static const char *valid_mem_map = R"(<?xml version="1.0"?>
+<!DOCTYPE memory-map
+ PUBLIC "+//IDN gnu.org//DTD GDB Memory Map V1.0//EN"
+ "http://sourceware.org/gdb/gdb-memory-map.dtd">
+<memory-map>
+ <memory type="ram" start="0" length="4096" />
+ <memory type="rom" start="65536" length="256" />
+ <memory type="flash" start="131072" length="65536">
+ <property name="blocksize">1024</property>
+ </memory>
+</memory-map>
+)";
+
+/* Validate memory region R against some expected values (the other
+ parameters). */
+
+static void
+check_mem_region (const mem_region &r, CORE_ADDR lo, CORE_ADDR hi,
+ mem_access_mode mode, int blocksize)
+{
+ SELF_CHECK (r.lo == lo);
+ SELF_CHECK (r.hi == hi);
+ SELF_CHECK (r.enabled_p);
+
+ SELF_CHECK (r.attrib.mode == mode);
+ SELF_CHECK (r.attrib.blocksize == blocksize);
+
+}
+
+/* Test the parse_memory_map function. */
+
+static void
+parse_memory_map_tests ()
+{
+ std::vector<mem_region> regions = parse_memory_map (valid_mem_map);
+
+ SELF_CHECK (regions.size () == 3);
+
+ check_mem_region (regions[0], 0, 0 + 4096, MEM_RW, -1);
+ check_mem_region (regions[1], 65536, 65536 + 256, MEM_RO, -1);
+ check_mem_region (regions[2], 131072, 131072 + 65536, MEM_FLASH, 1024);
+}
+
+} /* namespace memory_map_tests */
+} /* namespace selftests */
+
+void
+_initialize_memory_map_selftests ()
+{
+ selftests::register_test
+ ("parse_memory_map",
+ selftests::memory_map_tests::parse_memory_map_tests);
+}
--
2.7.4