From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24300 invoked by alias); 6 Feb 2009 21:38:26 -0000 Received: (qmail 24290 invoked by uid 22791); 6 Feb 2009 21:38:26 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS 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; Fri, 06 Feb 2009 21:38:22 +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 n16LaHbB006265; Fri, 6 Feb 2009 16:36:17 -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 n16LaH11003536; Fri, 6 Feb 2009 16:36:17 -0500 Received: from opsy.redhat.com (vpn-13-153.rdu.redhat.com [10.11.13.153]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n16LaGGP019109; Fri, 6 Feb 2009 16:36:17 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id F12E08880B9; Fri, 6 Feb 2009 14:36:14 -0700 (MST) To: ppluzhnikov@google.com (Paul Pluzhnikov) Cc: gdb-patches@sourceware.org Subject: Re: [patch] Fix a crash when displaying variables from shared library. References: <20090205030257.8A6073A6B7A@localhost> From: Tom Tromey Reply-To: tromey@redhat.com Date: Fri, 06 Feb 2009 21:38:00 -0000 In-Reply-To: <20090205030257.8A6073A6B7A@localhost> (Paul Pluzhnikov's message of "Wed\, 4 Feb 2009 19\:02\:57 -0800 \(PST\)") 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: 2009-02/txt/msg00160.txt.bz2 >>>>> "Paul" == Paul Pluzhnikov writes: Paul> @@ -1526,6 +1527,9 @@ do_one_display (struct display *d) [...] Paul> + if (d->exp == NULL) Paul> + d->exp = parse_expression (d->exp_string); Suppose this fails. I think what would happen is that none of the following displays would print anything. However, it seems like what should happen is that gdb deletes this display and moves on to try the next. What do you think? Also, are blocks valid after an solib is deleted? (I don't know.) If not then that means that the 'block' field must also be invalidated. Paul> + Paul> +static void Paul> +clear_dangling_display_expressions (struct so_list *solist) I think any new function needs an explanatory comment. Paul> --- solib.c 15 Jan 2009 16:35:22 -0000 1.109 Paul> +++ solib.c 5 Feb 2009 02:43:31 -0000 Paul> @@ -908,6 +908,7 @@ clear_solib (void) Paul> { Paul> struct so_list *so = so_list_head; Paul> so_list_head = so->next; Paul> + observer_notify_solib_unloaded (so); Paul> if (so->abfd) This hunk is already under discussion: http://sourceware.org/ml/gdb-patches/2009-01/msg00542.html In particular, I think Daniel's question should be answered before this goes in: http://sourceware.org/ml/gdb-patches/2009-02/msg00002.html Paul> Index: testsuite/gdb.base/solib-display.exp [...] Paul> +gdb_test "display a_global" Paul> +gdb_test "continue" Paul> +gdb_test "run" "1: a_global = 0" For regression tests, I like to add a comment explaining what is being tested. Could you do this? It doesn't have to be long. My reason for doing this is that later on it isn't always obvious what a test is trying to test. Otherwise this patch looks good to me. Tom