From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23505 invoked by alias); 14 Aug 2007 05:20:45 -0000 Received: (qmail 23066 invoked by uid 22791); 14 Aug 2007 05:20:42 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 14 Aug 2007 05:20:38 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 091D52AA0CF; Tue, 14 Aug 2007 01:20:37 -0400 (EDT) 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 UaTde1cT5u1z; Tue, 14 Aug 2007 01:20:36 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id A75672AA0A3; Tue, 14 Aug 2007 01:20:36 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 26A63E7B54; Mon, 13 Aug 2007 22:24:19 -0700 (PDT) Date: Tue, 14 Aug 2007 05:20:00 -0000 From: Joel Brobecker To: msnyder@sonic.net Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] ada-lang, possible_user_operator_p, null pointer Message-ID: <20070814052419.GE11498@adacore.com> References: <19577.12.7.175.2.1186864998.squirrel@webmail.sonic.net> <20070814043053.GB11498@adacore.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20070814043053.GB11498@adacore.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: 2007-08/txt/msg00279.txt.bz2 > > 2007-08-11 Michael Snyder > > > > * ada-lang.c (possible_user_operator_p): Guard against NULL. > > This is another case where I'd prefer to have the check for a NULL > type moved to the top of the function. But I'm having a tough time > determining how this might actually happen. So I'd like to sit on it > for a little longer before we proceed. I'm glad I sat on it :). I overlooked the fact that part of this function is not limited to binary operators. I also handles unary operators, so it would make sense that type1 might actually be NULL. However, the expression evaluation should always make sure that we have two operands before trying to evaluate it, so this should never happen, right? Unfortunately, digging deeper, I found elsewhere something that looks suspicious to me, and it actually allowed me to build an example that trigger a dereference of a NULL type1!!! I am currently discussing all this with Paul Hilfinger, who wrote most of this code, and I've suggested some other ways of rewriting the code that I think will make it more consistent: In short, define a separate function that matches arrays or pointer to arrays, with a guard against null pointers, the same as numeric_type_p for instance, and simplify the condition by using that function. We'd end up with something like this: case BINOP_CONCAT: return !(array_type_p (type0) && array_type_p (type1)); We also need to address the bug that this check guards against! In the meantime, I think it's fine to check in your patch. We'll probably end up rewriting this part of the code, but that won't actually change the actual logic, so.... > > Index: ada-lang.c > > =================================================================== > > RCS file: /cvs/src/src/gdb/ada-lang.c,v > > retrieving revision 1.100 > > diff -p -r1.100 ada-lang.c > > *** ada-lang.c 6 Aug 2007 20:07:44 -0000 1.100 > > --- ada-lang.c 11 Aug 2007 20:39:20 -0000 > > *************** possible_user_operator_p (enum exp_opcod > > *** 3536,3542 **** > > ((TYPE_CODE (type0) != TYPE_CODE_ARRAY > > && (TYPE_CODE (type0) != TYPE_CODE_PTR > > || TYPE_CODE (TYPE_TARGET_TYPE (type0)) != TYPE_CODE_ARRAY)) > > ! || (TYPE_CODE (type1) != TYPE_CODE_ARRAY > > && (TYPE_CODE (type1) != TYPE_CODE_PTR > > || (TYPE_CODE (TYPE_TARGET_TYPE (type1)) > > != TYPE_CODE_ARRAY)))); > > --- 3536,3542 ---- > > ((TYPE_CODE (type0) != TYPE_CODE_ARRAY > > && (TYPE_CODE (type0) != TYPE_CODE_PTR > > || TYPE_CODE (TYPE_TARGET_TYPE (type0)) != TYPE_CODE_ARRAY)) > > ! || (type1 != NULL && TYPE_CODE (type1) != TYPE_CODE_ARRAY > > && (TYPE_CODE (type1) != TYPE_CODE_PTR > > || (TYPE_CODE (TYPE_TARGET_TYPE (type1)) > > != TYPE_CODE_ARRAY)))); -- Joel