[PATCH] Fix for 64-Bit Core Files on AIX

[PATCH] Fix for 64-Bit Core Files on AIX

[Roberts, Dennis]

[-- Attachment #1: Type: text/plain, Size: 223 bytes --]

Could someone please review this patch and apply it if it passes review?

Note: the BFD modifications have already been submitted to the binutils mailing list; I just included them here for reference.

Thanks,
Dennis

[-- Attachment #2: aix-64-bit-core-files.patch --]
[-- Type: application/octet-stream, Size: 4857 bytes --]

diff -rc src.orig/bfd/ChangeLog src/bfd/ChangeLog
*** src.orig/bfd/ChangeLog	Mon Apr 14 12:58:48 2008
--- src/bfd/ChangeLog	Tue Apr 15 13:30:07 2008
***************
*** 1,3 ****
--- 1,14 ----
+ 2008-04-15  Dennis Roberts  <dennis.roberts@sunquestinfo.com>
+ 
+ 	* aix5ppc-core.c: Define macros for the default architecture and
+ 	machine for matching core files.
+ 	(xcoff64_core_p): Set the architecture and machine to the default
+ 	values defined in the macros mentioned above.
+ 	* rs6000-core.c: Define macros to determine whether or not the
+ 	core file header uses the core_dumpxx header format.
+ 	(rs6000coff_core_p): Don't match core files that use the
+ 	core_dumpxx header format.
+ 
  2008-04-14  Aurelien Jarno  <aurelien@aurel32.net>
  
  	* configure.in: Link with the PIC version of libiberty on
diff -rc src.orig/bfd/aix5ppc-core.c src/bfd/aix5ppc-core.c
*** src.orig/bfd/aix5ppc-core.c	Tue Jul  3 07:26:39 2007
--- src/bfd/aix5ppc-core.c	Tue Apr 15 13:33:11 2008
***************
*** 40,45 ****
--- 40,49 ----
  #include <sys/ldr.h>
  #include <core.h>
  
+ /* The default architecture and machine for matching core files. */
+ #define DEFAULT_ARCHITECTURE	bfd_arch_powerpc
+ #define DEFAULT_MACHINE		bfd_mach_ppc_620
+ 
  #define	core_hdr(abfd)		((struct core_dumpxx *) abfd->tdata.any)
  
  #define CHECK_FILE_OFFSET(s, v) \
***************
*** 48,53 ****
--- 52,59 ----
  const bfd_target *
  xcoff64_core_p (bfd *abfd)
  {
+   enum bfd_architecture arch;
+   unsigned long mach;
    struct core_dumpxx core, *new_core_hdr;
    struct stat statbuf;
    asection *sec;
***************
*** 217,222 ****
--- 223,233 ----
  	  sec->filepos = vminfo.vminfo_offset;
  	}
      }
+ 
+   /* Set the architecture and machine. */
+   arch = DEFAULT_ARCHITECTURE;
+   mach = DEFAULT_MACHINE;
+   bfd_default_set_arch_mach (abfd, arch, mach);
  
    return_value = (bfd_target *) abfd->xvec;	/* This is garbage for now.  */
  
diff -rc src.orig/bfd/rs6000-core.c src/bfd/rs6000-core.c
*** src.orig/bfd/rs6000-core.c	Tue Jul  3 07:26:42 2007
--- src/bfd/rs6000-core.c	Tue Apr 15 13:33:10 2008
***************
*** 136,141 ****
--- 136,154 ----
  # define CORE_NEW(c)	0
  #endif
  
+ /* Return whether CoreHdr C usese core_dumpxx structure.
+ 
+    FIXME: the core file format version number used here definitely indicates
+    that struct core_dumpxx should be used to represent the core file header,
+    but that may not be the only such format version number.  */
+ 
+ #ifdef AIX_5_CORE
+ # define CORE_DUMPXX_VERSION  267312562
+ # define CNEW_IS_CORE_DUMPXX(c)       ((c).new.c_version == CORE_DUMPXX_VERSION)
+ #else
+ # define CNEW_IS_CORE_DUMPXX(c)       0
+ #endif
+ 
  /* Return the c_stackorg field from struct core_dumpx C.  */
  
  #ifdef AIX_CORE_DUMPX_CORE
***************
*** 326,331 ****
--- 339,351 ----
      {
        if (bfd_get_error () != bfd_error_system_call)
  	bfd_set_error (bfd_error_wrong_format);
+       return NULL;
+     }
+ 
+   /* This isn't the right handler for 64-bit core files on AIX 5.x. */
+   if (CORE_NEW (core) && CNEW_IS_CORE_DUMPXX(core))
+     {
+       bfd_set_error (bfd_error_wrong_format);
        return NULL;
      }
  
diff -rc src.orig/gdb/ChangeLog src/gdb/ChangeLog
*** src.orig/gdb/ChangeLog	Mon Apr 14 12:47:28 2008
--- src/gdb/ChangeLog	Tue Apr 15 13:32:36 2008
***************
*** 1,3 ****
--- 1,8 ----
+ 2008-04-15  Dennis Roberts  <dennis.roberts@sunquestinfo.com>
+ 
+ 	* rs6000-tdep.c (rs6000_gdbarch_init): Use the BFD architecture,
+ 	rather than a hard-coded architecture, for xcoff executables.
+ 
  2008-04-14  Daniel Jacobowitz  <dan@codesourcery.com>
  
  	* Makefile.in (GNULIB_H): New.  Trigger all-lib.
diff -rc src.orig/gdb/rs6000-tdep.c src/gdb/rs6000-tdep.c
*** src.orig/gdb/rs6000-tdep.c	Wed Feb 20 07:34:43 2008
--- src/gdb/rs6000-tdep.c	Tue Apr 15 13:33:10 2008
***************
*** 3307,3324 ****
  	wordsize = 4;
      }
  
!   if (!from_xcoff_exec)
!     {
!       arch = info.bfd_arch_info->arch;
!       mach = info.bfd_arch_info->mach;
!     }
!   else
!     {
!       arch = bfd_arch_powerpc;
!       bfd_default_set_arch_mach (&abfd, arch, 0);
!       info.bfd_arch_info = bfd_get_arch_info (&abfd);
!       mach = info.bfd_arch_info->mach;
!     }
  
    /* For e500 executables, the apuinfo section is of help here.  Such
       section contains the identifier and revision number of each
--- 3307,3315 ----
  	wordsize = 4;
      }
  
!   /* Get the architecture and machine from the BFD. */
!   arch = info.bfd_arch_info->arch;
!   mach = info.bfd_arch_info->mach;
  
    /* For e500 executables, the apuinfo section is of help here.  Such
       section contains the identifier and revision number of each

Re: [PATCH] Fix for 64-Bit Core Files on AIX

[Joel Brobecker]

Dennis,

> 2008-04-15  Dennis Roberts  <dennis.roberts@sunquestinfo.com>
> 
> 	* rs6000-tdep.c (rs6000_gdbarch_init): Use the BFD architecture,
> 	rather than a hard-coded architecture, for xcoff executables.

I have reviewed the change, and it looks OK (and can be applied
independently of the BFD part). But I just wanted to make sure that
you have indeed tested this change both in 32 and 64 bit modes.
Is that correct?  

The change is sufficiently small that it qualifies as a non-legally
significant change (patch repeated at the end of this email for quick
reference), but if you think you are going to make more contributions,
I would encourage you to provide a copyright assignment to the FSF.
We could then offer you write access to the repository.  I can help you
get started if you like.

In the meantime, I will commit your change as soon as you have confirmed
testing.

Thank you,
-- 
Joel

> diff -rc src.orig/gdb/rs6000-tdep.c src/gdb/rs6000-tdep.c
> *** src.orig/gdb/rs6000-tdep.c	Wed Feb 20 07:34:43 2008
> --- src/gdb/rs6000-tdep.c	Tue Apr 15 13:33:10 2008
> ***************
> *** 3307,3324 ****
>   	wordsize = 4;
>       }
>   
> !   if (!from_xcoff_exec)
> !     {
> !       arch = info.bfd_arch_info->arch;
> !       mach = info.bfd_arch_info->mach;
> !     }
> !   else
> !     {
> !       arch = bfd_arch_powerpc;
> !       bfd_default_set_arch_mach (&abfd, arch, 0);
> !       info.bfd_arch_info = bfd_get_arch_info (&abfd);
> !       mach = info.bfd_arch_info->mach;
> !     }
>   
>     /* For e500 executables, the apuinfo section is of help here.  Such
>        section contains the identifier and revision number of each
> --- 3307,3315 ----
>   	wordsize = 4;
>       }
>   
> !   /* Get the architecture and machine from the BFD. */
> !   arch = info.bfd_arch_info->arch;
> !   mach = info.bfd_arch_info->mach;
>   
>     /* For e500 executables, the apuinfo section is of help here.  Such
>        section contains the identifier and revision number of each

RE: [PATCH] Fix for 64-Bit Core Files on AIX

[Roberts, Dennis]

Hi Joel,

Yes, I did a quick manual test with both 32- and 64-bit core files and executables to make sure that the changes worked as expected.  I also ran the testsuite several times before and after the change was made to make sure that there were no adverse side-effects.  The only caveat is that the only version of AIX that I was able to use for testing was 5.3.0.0.

I would be interested in providing a copyright assignment to the FSF.  How would I go about doing that?

Thanks,
Dennis

-----Original Message-----
From: Joel Brobecker [mailto:brobecker@adacore.com]
Sent: Wednesday, April 16, 2008 2:16 PM
To: Roberts, Dennis
Cc: 'gdb-patches@sourceware.org'
Subject: Re: [PATCH] Fix for 64-Bit Core Files on AIX

Dennis,

> 2008-04-15  Dennis Roberts  <dennis.roberts@sunquestinfo.com>
>
>       * rs6000-tdep.c (rs6000_gdbarch_init): Use the BFD architecture,
>       rather than a hard-coded architecture, for xcoff executables.

I have reviewed the change, and it looks OK (and can be applied independently of the BFD part). But I just wanted to make sure that you have indeed tested this change both in 32 and 64 bit modes.
Is that correct?

The change is sufficiently small that it qualifies as a non-legally significant change (patch repeated at the end of this email for quick reference), but if you think you are going to make more contributions, I would encourage you to provide a copyright assignment to the FSF.
We could then offer you write access to the repository.  I can help you get started if you like.

In the meantime, I will commit your change as soon as you have confirmed testing.

Thank you,
--
Joel

> diff -rc src.orig/gdb/rs6000-tdep.c src/gdb/rs6000-tdep.c
> *** src.orig/gdb/rs6000-tdep.c        Wed Feb 20 07:34:43 2008
> --- src/gdb/rs6000-tdep.c     Tue Apr 15 13:33:10 2008
> ***************
> *** 3307,3324 ****
>       wordsize = 4;
>       }
>
> !   if (!from_xcoff_exec)
> !     {
> !       arch = info.bfd_arch_info->arch;
> !       mach = info.bfd_arch_info->mach;
> !     }
> !   else
> !     {
> !       arch = bfd_arch_powerpc;
> !       bfd_default_set_arch_mach (&abfd, arch, 0);
> !       info.bfd_arch_info = bfd_get_arch_info (&abfd);
> !       mach = info.bfd_arch_info->mach;
> !     }
>
>     /* For e500 executables, the apuinfo section is of help here.  Such
>        section contains the identifier and revision number of each
> --- 3307,3315 ----
>       wordsize = 4;
>       }
>
> !   /* Get the architecture and machine from the BFD. */
> !   arch = info.bfd_arch_info->arch;
> !   mach = info.bfd_arch_info->mach;
>
>     /* For e500 executables, the apuinfo section is of help here.  Such
>        section contains the identifier and revision number of each

Re: [PATCH] Fix for 64-Bit Core Files on AIX

[Joel Brobecker]

> Yes, I did a quick manual test with both 32- and 64-bit core files and
> executables to make sure that the changes worked as expected.  I also
> ran the testsuite several times before and after the change was made
> to make sure that there were no adverse side-effects.  The only caveat
> is that the only version of AIX that I was able to use for testing was
> 5.3.0.0.

Thanks for confirming the testing. I will see if I can test the change
on AIX 5.1 and then commit it.

> I would be interested in providing a copyright assignment to the FSF.
> How would I go about doing that?

[paperwork started - private email sent]. Let me know when the paperwork
is complete.

-- 
Joel

RE: [PATCH] Fix for 64-Bit Core Files on AIX

[Roberts, Dennis]

> [paperwork started - private email sent]. Let me know when
> the paperwork is complete.

Will do; thanks.

Dennis

Re: [PATCH] Fix for 64-Bit Core Files on AIX

[Joel Brobecker]

> > Yes, I did a quick manual test with both 32- and 64-bit core files and
> > executables to make sure that the changes worked as expected.  I also
> > ran the testsuite several times before and after the change was made
> > to make sure that there were no adverse side-effects.  The only caveat
> > is that the only version of AIX that I was able to use for testing was
> > 5.3.0.0.
> 
> Thanks for confirming the testing. I will see if I can test the change
> on AIX 5.1 and then commit it.

I wasn't able to test this change on 5.1 as our 5.1 machine apparently
stopped working a while ago. I did some testing on 5.2, and then
committed your patch.

Just a couple of additional remarks:

  - I fixed a missing space at the end of your comment:
      /* Your comment should have too space after the dot.  */

  - Would it be much work for you to change the MIME type used for
    your patches? Right now, it's application/octet-stream which
    makes it harder to read and reply to.

Would you mind letting us know when the BFD part is checked in, and
we'll add a NEWS entry for this new feature.

Thank you,
-- 
Joel

RE: [PATCH] Fix for 64-Bit Core Files on AIX

[Roberts, Dennis]

> Just a couple of additional remarks:
>
>   - I fixed a missing space at the end of your comment:
>       /* Your comment should have too space after the dot.  */

OK.

>   - Would it be much work for you to change the MIME type used for
>     your patches? Right now, it's application/octet-stream which
>     makes it harder to read and reply to.

I'm not sure how to change the MIME type for attachments in the e-mail program we use here (Outlook), but I'll try to figure it out.  If I can't, would it be easier if I just pasted the diff into the main e-mail text?

> Would you mind letting us know when the BFD part is checked
> in, and we'll add a NEWS entry for this new feature.

Sure; I can do that.

Thanks,
Dennis

RE: [PATCH] Fix for 64-Bit Core Files on AIX

[Roberts, Dennis]

Hi Joel,

I just received word that the BFD changes related to this patch have been applied to binutils.

Thanks,
Dennis

Re: [PATCH] Fix for 64-Bit Core Files on AIX

[Joel Brobecker]

> I just received word that the BFD changes related to this patch have
> been applied to binutils.

Excellent news. Thanks Dennis, I just sent a NEWS patch to document
this enhancement.

-- 
Joel