Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Tom Tromey <tromey@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: RFA: probable rs6000-aix-tdep.c bug found by clang
Date: Wed, 17 Oct 2012 21:24:00 -0000	[thread overview]
Message-ID: <20121017212417.GP3050@adacore.com> (raw)
In-Reply-To: <87mwzk279g.fsf@fleche.redhat.com>


> 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  <tromey@redhat.com>
> 
> 	* 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


  reply	other threads:[~2012-10-17 21:24 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-17 20:06 Tom Tromey
2012-10-17 21:24 ` Joel Brobecker [this message]
2012-10-18  9:10   ` Pedro Alves
2012-10-18 15:33     ` Joel Brobecker
2012-10-18 15:46       ` Pedro Alves
2012-10-18 15:58         ` Joel Brobecker
2012-10-18 16:06           ` Pedro Alves
2012-10-18 18:49       ` Joel Brobecker
2012-10-18 19:43         ` Tom Tromey
2012-10-19 20:00         ` Joel Brobecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121017212417.GP3050@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tromey@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox