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

[Tom Tromey <tromey@redhat.com>]

>>>>> "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 */