From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10694 invoked by alias); 23 Feb 2009 01:08:12 -0000 Received: (qmail 10686 invoked by uid 22791); 23 Feb 2009 01:08:11 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 23 Feb 2009 01:08:03 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 5C2D02BAB66; Sun, 22 Feb 2009 20:08:04 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id ss8FcjyKqB3y; Sun, 22 Feb 2009 20:08:04 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id EB0052BAB60; Sun, 22 Feb 2009 20:08:03 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 8E45DE7ACD; Sun, 22 Feb 2009 17:07:59 -0800 (PST) Date: Mon, 23 Feb 2009 01:47:00 -0000 From: Joel Brobecker To: Paul Pluzhnikov Cc: Tom Tromey , Pedro Alves , gdb-patches@sourceware.org Subject: Re: [patch] Fix a crash when displaying variables from shared library. Message-ID: <20090223010759.GA30997@adacore.com> References: <20090205030257.8A6073A6B7A@localhost> <8ac60eac0902061837p5885b812j8a26669e799702e1@mail.gmail.com> <8ac60eac0902181458g39dfbce9k63c3329528b0aad5@mail.gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8ac60eac0902181458g39dfbce9k63c3329528b0aad5@mail.gmail.com> User-Agent: Mutt/1.4.2.2i 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/msg00425.txt.bz2 Hello Paul, > (gdb) r > > Breakpoint 1, main () at noshlib.c:11 > 11 return foo(); > warning: Unable to display "a_static": No symbol "a_static" in current context. > warning: Unable to display "a_local": No symbol "a_local" in current context. > 1: a_global = 0 Good catch! > I've updated the patch to only clear displays which actually are > about to become dangling, and extended the test to check that displays > that refer to the main executable are not affected. You forgot to include a ChangeLog... > + /* Can't re-parse the expression. Disable this display item. */ Minor style issue: You need to have two spaces after each period (one in the middle, and one at the end, before the "*/"). > +/* Answer 1 if "d" uses "solib" (and will become dangling when "solib" > + is unloaded), otherwise answer 0. */ If you don't mind, I think using "Return 1" instead of "Answer 1" would be more consistent with the other descriptions. Another minor style correction: In GDB, we refer to the function parameters by using their names in ALL_CAPS, and without the quotes. So, in your case, you would write: /* Return 1 if D uses SOLIB (and will become dangling [...] */ > + if (d->block != NULL > + && addr_low <= d->block->startaddr && d->block->endaddr <= addr_high) > + return 1; I suggest you use solib_address instead of doing the check yourself. As mentioned by Daniel in another thread, shared libraries on SVR4 systems occupy a contiguous address block, but this is not the case of DLLs where the data and text sections might be separate. I verified that solib_address should handle the DLL case. > + for (i = 0; i < d->exp->nelts; i++) > + { > + union exp_element *elts = d->exp->elts; > + if (elts[i].opcode == OP_VAR_VALUE) I'm afraid this isn't going to work for more complex structures... The problem is that you might be reading an undefined field of union exp_element. Imagine for instance that you have an expression that looks like this: "foo->bar". At one point, you'll encounter the following elements: [i ] -> STRUCTOP_PTR [i+1] -> A string [i+2] -> STRUCTOP_PTR Iterating over the expression, you'll ignore the element at index i, and then check the opcode of the element at i+1, which is the wrong field of the enum to access in this case... I can't think of a way of doing what you're trying to do off the top of my head. I'll have to think about it a little more. Perhaps others will have suggestions... Or perhaps we'll have to attack the problem with a different angle, I'm not very familiar with how "display" expressions are handled... > +gdb_test "run" "3: c_global = 43\\r\\nwarning: .*b_global.*\\r\\n1: a_global = 41" "after rebuild" Can this be changed to use either one of the runto routines, or maybe gdb_run_cmd if one of the above doesn't work in this case? > +gdb_test "run" "6: a_static = 46\\r\\n4: main_global = 44\\r\\n.*" Same here. -- Joel