From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15354 invoked by alias); 27 Nov 2007 20:47:56 -0000 Received: (qmail 15321 invoked by uid 22791); 27 Nov 2007 20:47:55 -0000 X-Spam-Check-By: sourceware.org Received: from bluesmobile.specifix.com (HELO bluesmobile.specifix.com) (216.129.118.140) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 27 Nov 2007 20:47:51 +0000 Received: from [127.0.0.1] (bluesmobile.specifix.com [216.129.118.140]) by bluesmobile.specifix.com (Postfix) with ESMTP id D75173BA8F; Tue, 27 Nov 2007 12:47:49 -0800 (PST) Subject: Re: [ob] unbreak MI From: Michael Snyder To: Vladimir Prus Cc: Nick Roberts , gdb-patches@sources.redhat.com In-Reply-To: <200711271000.06151.ghost@cs.msu.su> References: <200708312244.58216.ghost@cs.msu.su> <18251.47321.156923.358334@kahikatea.snap.net.nz> <200711271000.06151.ghost@cs.msu.su> Content-Type: text/plain Date: Tue, 27 Nov 2007 20:47:00 -0000 Message-Id: <1196195698.2501.80.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.10.3 (2.10.3-4.fc7) Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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: 2007-11/txt/msg00515.txt.bz2 On Tue, 2007-11-27 at 10:00 +0300, Vladimir Prus wrote: > On Tuesday 27 November 2007 09:27:37 Nick Roberts wrote: > > > > Generally, with a NULL pointer, or and address that can't be dereferenced, > > > > MI prints out the value field as value="". > > > > > > > > What is the problem in this case? Why isn't the right fix to add a > > > > check_typedef somewhere? > > > > > > check_typedef? The original problem was that check_typedef was getting > > > called on NULL pointer, so adding more check_typedef calls won't help. > > > Probably: > > > > > > if (!gdb_type) > > > ui_out_field_string (uiout, "value", ""); > > > else if (mi_print_value_p (gdb_type, print_values)) > > > ui_out_field_string (uiout, "value", varobj_get_value (var)); > > > > > > is the right logic? > > > > It's probably the right logic, but it seems to cure the symptom rather than the > > cause. What I mean't, I guess, was where/how does check_typedef is get passed > > a NULL pointer? And can't that call be conditioned (i.e. "add a *check* to > > check_typedef") , e.g., something like: > > > > if (!gdb_type) > > check_typedef (gdb_type) > > Just look at mi_print_value_p, and you'll see a call to check_typedef. Actually, > the code previously looked like: > > if (type != NULL) > type = check_typedef (type); > > It was changed in revision 1.38, with the following comment: > > 2007-08-28 Michael Snyder > > * mi/mi-cmd-var.c (mi_print_value_p): No longer necessary to > check for null before calling check_typedef. > > However, apparently check_typedef still crashes when passed NULL, > and it can be passed NULL. It doesn't crash -- it calls assert, therefore abort. The debate at the time was whether it made more sense to check for null before every call to check_typedef, or simply to have check_typedef do the check for null itself. Makeing a change in one place seemed easier than makeing a change in 100's of places. And it's not clear that check_typedef can do anything intelligent to recover if a null pointer is passed --- hence the abort. Probably calling error rather than abort would be acceptable. > > The original code, in fact, was in error too, because of this: > > return (TYPE_CODE (type) != TYPE_CODE_ARRAY > && TYPE_CODE (type) != TYPE_CODE_STRUCT > && TYPE_CODE (type) != TYPE_CODE_UNION); > > This will crash if 'type' is NULL. Testsuite fails to detect this because presently > type is NULL only for C++ pseudo-fields ('public'/'private') and the code > above is only executed for --simple-values. > > Does this clarify things? > > - Volodya