From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21181 invoked by alias); 6 Mar 2011 18:46:29 -0000 Received: (qmail 21173 invoked by uid 22791); 6 Mar 2011 18:46:28 -0000 X-SWARE-Spam-Status: No, hits=-5.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-outbound-1.vmware.com (HELO smtp-outbound-1.vmware.com) (65.115.85.69) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 06 Mar 2011 18:46:24 +0000 Received: from mailhost2.vmware.com (mailhost2.vmware.com [10.16.67.167]) by smtp-outbound-1.vmware.com (Postfix) with ESMTP id A4D134A031; Sun, 6 Mar 2011 10:46:23 -0800 (PST) Received: from msnyder-server.eng.vmware.com (promd-2s-dhcp138.eng.vmware.com [10.20.124.138]) by mailhost2.vmware.com (Postfix) with ESMTP id 9E61E8EC16; Sun, 6 Mar 2011 10:46:23 -0800 (PST) Message-ID: <4D73D67F.1030704@vmware.com> Date: Sun, 06 Mar 2011 18:54:00 -0000 From: Michael Snyder User-Agent: Thunderbird 2.0.0.24 (X11/20101201) MIME-Version: 1.0 To: Jan Kratochvil CC: "gdb-patches@sourceware.org" Subject: Re: [RFA] mdebugread.c (psymtab_to_symtab_1): assert that 'fh' is not null References: <4D71648B.1090101@vmware.com> <20110306142337.GB1895@host1.jankratochvil.net> In-Reply-To: <20110306142337.GB1895@host1.jankratochvil.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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: 2011-03/txt/msg00403.txt.bz2 Jan Kratochvil wrote: > On Fri, 04 Mar 2011 23:15:39 +0100, Michael Snyder wrote: >> This occurs along a path where fh *could* be null, and then we >> dereference it. > > I agree there is a bug. > > >> + gdb_assert (fh); > [...] >> parse_external (ext_ptr, fh->fBigendian, >> pst->section_offsets, pst->objfile); > > But I find as even a worse bug to introduce such an assertion without any > comment there. > > I at least myself perceive any assertions that the programmer thinks such > invariant is valid there. That invariant says that if its negation happens > then the programmer was wrong. > > If there is a known bug you have found and you just do not intend to fix it > now I find introducing such an assertion as misleading. The reader then > assumes such invariant and can do additional wrong conclusions from it. > > In such case there should be primarily a "FIXME" comment, best even to > reference a filed a Bug. There can be also an assertion there but the "FIXME" > I find essential there. All these recent submissions of mine are coming from running Coverity on gdb. In this case, Coverity reports that we checked the variable fh for null (suggesting that we think null is possible), but then dereferenced it on an unchecked path. I figured that triggering an assert was more gracefull than just crashing on a null pointer dereference, and gives the user the chance to recover.