Re: PING: [PATCH] Fix a bug of addrmap

Tom Tromey
2008-11-25T17:06:59Z

>>>>> "Jie" == Jie Zhang <jie.zhang@analog.com> writes:

Jie> Could someone give a review on this patch:
Jie> http://sourceware.org/ml/gdb-patches/2008-10/msg00503.html

I was curious about this patch so I took a look.

Since addrmap is a relatively self-contained data structure, I figured
it was a good candidate for a unit test. So, I wrote a test case
based on your original report.

I think the test case is pretty clear. And sure enough, it fails
before your patch and it passes after your pass. So, on that basis I
would support checking in your patch. However, please note I cannot
approve it.

I think it also worth checking in the appended. If we ever find
another addrmap bug, we could extend the unit test. Please review
this, thanks.

Tom

2008-11-25 Tom Tromey <tromey@redhat.com>

* Makefile.in (test-addrmap.o): New target.
(test-addrmap): Likewise.
(clean mostlyclean): Remove test-addrmap.
* addrmap.c (xfree): New function.
(internal_error): Likewise.
(test_inclusion): Likewise.
(main): Likewise.

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 5432c88..8d947d2 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -968,6 +968,15 @@ test-cp-name-parser$(EXEEXT): test-cp-name-parser.o $(LIBIBERTY)
$(CC_LD) $(INTERNAL_LDFLAGS) -o test-cp-name-parser$(EXEEXT) \
test-cp-name-parser.o $(LIBIBERTY)

+# Addrmap has unit tests which can be run standalone.
+test-addrmap.o: addrmap.c
+ $(COMPILE) -DADDRMAP_UNIT_TEST $(srcdir)/addrmap.c
+ $(POSTCOMPILE)
+
+test-addrmap: test-addrmap.o $(LIBIBERTY)
+ $(CC_LD) $(INTERNAL_LDFLAGS) -o test-addrmap$(EXEEXT) \
+ test-addrmap.o $(LIBIBERTY)
+
# We do this by grepping through sources. If that turns out to be too slow,
# maybe we could just require every .o file to have an initialization routine
# of a given name (top.o -> _initialize_top, etc.).
@@ -1117,7 +1126,7 @@ clean mostlyclean: $(CONFIG_CLEAN)
rm -f init.c version.c
rm -f gdb$(EXEEXT) core make.log
rm -f gdb[0-9]$(EXEEXT)
- rm -f test-cp-name-parser$(EXEEXT)
+ rm -f test-cp-name-parser$(EXEEXT) test-addrmap$(EXEEXT)
rm -f xml-builtin.c stamp-xml

.PHONY: clean-tui
diff --git a/gdb/addrmap.c b/gdb/addrmap.c
index 68832e9..a73a945 100644
--- a/gdb/addrmap.c
+++ b/gdb/addrmap.c
@@ -541,3 +541,67 @@ _initialize_addrmap (void)
gdb_assert (sizeof (splay_tree_key) >= sizeof (CORE_ADDR *));
gdb_assert (sizeof (splay_tree_value) >= sizeof (void *));
}
+
+\f
+/* Unit testing. */
+
+#ifdef ADDRMAP_UNIT_TEST
+
+/* When this file is built as a standalone program, xmalloc comes from
+ libiberty --- in which case we have to provide xfree ourselves. */
+
+void
+xfree (void *ptr)
+{
+ free (ptr);
+}
+
+/* Likewise for internal_error. */
+NORETURN void
+internal_error (const char *file, int line, const char *string, ...)
+{
+ /* The user will have to debug this anyway. */
+ exit (1);
+}
+
+/* A test case reported to the list. Return 1 if ok, 0 on
+ failure. */
+static int
+test_inclusion (void)
+{
+ struct obstack alloc;
+ struct addrmap *map;
+ char *fooc = "foo.c";
+ char *mainc = "main.c";
+ int result;
+
+ obstack_init (&alloc);
+ map = addrmap_create_mutable (&alloc);
+
+ addrmap_set_empty (map, 0x400448, 0x40053e, mainc);
+ addrmap_set_empty (map, 0x400454, 0x40045f, fooc);
+
+ map = addrmap_create_fixed (map, &alloc);
+
+ result = (addrmap_find (map, 0x0) == NULL
+ && addrmap_find (map, 0x400447) == NULL
+ && addrmap_find (map, 0x400448) == mainc
+ && addrmap_find (map, 0x400453) == mainc
+ && addrmap_find (map, 0x400454) == fooc
+ && addrmap_find (map, 0x40045f) == fooc
+ && addrmap_find (map, 0x400460) == mainc
+ && addrmap_find (map, 0x40053e) == mainc
+ && addrmap_find (map, 0x40053f) == NULL);
+
+ obstack_free (&alloc, 0);
+
+ return result;
+}
+
+int
+main (int argc, char **argv)
+{
+ return !test_inclusion ();
+}
+
+#endif /* ADDRMAP_UNIT_TEST */