From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18566 invoked by alias); 25 Nov 2008 17:06:59 -0000 Received: (qmail 18401 invoked by uid 22791); 25 Nov 2008 17:06:57 -0000 X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 25 Nov 2008 17:06:02 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id mAPH5fOv010155; Tue, 25 Nov 2008 12:05:41 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id mAPH5ePX018105; Tue, 25 Nov 2008 12:05:40 -0500 Received: from opsy.redhat.com (vpn-13-14.rdu.redhat.com [10.11.13.14]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id mAPH5d7G029077; Tue, 25 Nov 2008 12:05:39 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id 418A08880B2; Tue, 25 Nov 2008 10:05:38 -0700 (MST) To: Jie Zhang Cc: gdb-patches@sourceware.org Subject: Re: PING: [PATCH] Fix a bug of addrmap References: <48FD5DE1.9090105@analog.com> <490E9540.7000207@analog.com> From: Tom Tromey Reply-To: tromey@redhat.com Date: Tue, 25 Nov 2008 23:09:00 -0000 In-Reply-To: <490E9540.7000207@analog.com> (Jie Zhang's message of "Mon\, 03 Nov 2008 14\:08\:00 +0800") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-11/txt/msg00692.txt.bz2 >>>>> "Jie" == Jie Zhang 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 * 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 *)); } + + +/* 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 */