From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21165 invoked by alias); 17 Oct 2012 21:24:34 -0000 Received: (qmail 21156 invoked by uid 22791); 17 Oct 2012 21:24:32 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_NO 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; Wed, 17 Oct 2012 21:24:26 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 3F3EB1C7FB1; Wed, 17 Oct 2012 17:24:26 -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 YcY1Obegnvkr; Wed, 17 Oct 2012 17:24:26 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 0D5391C7FAD; Wed, 17 Oct 2012 17:24:25 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 001BDC4B88; Wed, 17 Oct 2012 14:24:17 -0700 (PDT) Date: Wed, 17 Oct 2012 21:24:00 -0000 From: Joel Brobecker To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: RFA: probable rs6000-aix-tdep.c bug found by clang Message-ID: <20121017212417.GP3050@adacore.com> References: <87mwzk279g.fsf@fleche.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87mwzk279g.fsf@fleche.redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2012-10/txt/msg00306.txt.bz2 > Based on indentation and logic I think that the fix is to remove the ";". > However, I have no decent way to test this and would appreciate someone > else looking at it. [...] > 2012-10-17 Tom Tromey > > * rs6000-aix-tdep.c (rs6000_aix_osabi_sniffer): Remove extraneous > semicolon. Thanks Tom, for shaming my Evil twin :-). He's pretending to be spelunking at the moment - aka hiding in a cave, while his Good twin is writing a reply... The patch looks completely correct. In fact, AdaCore's version of GDB defines the function as follow: if (bfd_get_flavour (abfd) != bfd_target_xcoff_flavour) return GDB_OSABI_UNKNOWN; /* The only noticeable difference between Lynx178 XCOFF files and AIX XCOFF files comes from the fact that there are no shared libraries on Lynx178. On AIX, we are betting that an executable linked with no shared library will never exist. */ if (xcoff_get_n_import_files (abfd) <= 0) return GDB_OSABI_UNKNOWN; return GDB_OSABI_AIX; We've had that change for a few months, now, so that should be enough evidence that the change is not going to create problems. I researched a little bit how this error managed to not break any other platform. But I think we just got lucky, because no other powerpc target outside of AIX uses the xcoff flavor. In fact, going one step further, do we need the check at all? This is how the sniffer is registered: gdbarch_register_osabi_sniffer (bfd_arch_rs6000, bfd_target_xcoff_flavour, rs6000_aix_osabi_sniffer); gdbarch_register_osabi_sniffer (bfd_arch_powerpc, bfd_target_xcoff_flavour, rs6000_aix_osabi_sniffer); So, isn't rs6000_aix_osabi_sniffer going to be called if, and only if, the bfd has a bfd_target_xcoff_flavour, thus making the check superfluous? > > diff --git a/gdb/rs6000-aix-tdep.c b/gdb/rs6000-aix-tdep.c > index 59cfa73..749c109 100644 > --- a/gdb/rs6000-aix-tdep.c > +++ b/gdb/rs6000-aix-tdep.c > @@ -723,7 +723,7 @@ static enum gdb_osabi > rs6000_aix_osabi_sniffer (bfd *abfd) > { > > - if (bfd_get_flavour (abfd) == bfd_target_xcoff_flavour); > + if (bfd_get_flavour (abfd) == bfd_target_xcoff_flavour) > return GDB_OSABI_AIX; > > return GDB_OSABI_UNKNOWN; -- Joel