[patch] fix unwind handling on hppa elf targets

[patch] fix unwind handling on hppa elf targets

[Randolph Chung]

This patch adds some infrastructure to make the generic hppa-tdep code
more sharable with hppa-linux. The unwind offset calculation is
applicable to all hppa-elf targets and not only on 64-bit targets. 

2004-04-10  Randolph Chung  <tausq@debian.org>

	* hppa-tdep.h (gdbarch_tdep): Add is_elf flag.
	* hppa-hpux-tdep.c (hppa_hpux_elf_init_abi): Set is_elf flag.
	* hppa-tdep.c (record_text_segment_lowaddr): Calculate text segment
	address using file offset.
	(internalize_unwinds): Predicate unwind offset calculation on is_elf
	flag rather than on width of binary. Remove "hokey" text segment
	masking (it is indeed wrong).

Index: hppa-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.h,v
retrieving revision 1.1
diff -u -p -r1.1 hppa-tdep.h
--- hppa-tdep.h	15 Aug 2003 23:02:03 -0000	1.1
+++ hppa-tdep.h	10 Apr 2004 08:50:20 -0000
@@ -21,10 +21,13 @@
 #ifndef HPPA_TDEP_H
 #define HPPA_TDEP_H
 
 /* Target-dependent structure in gdbarch.  */
 struct gdbarch_tdep
 {
   /* The number of bytes in an address.  For now, this field is designed
      to allow us to differentiate hppa32 from hppa64 targets.  */
   int bytes_per_address;
+
+  /* Is this an ELF target? This can be 64-bit HP-UX, or 32/64-bit Linux */
+  int is_elf;
 };
Index: hppa-hpux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-hpux-tdep.c,v
retrieving revision 1.14
diff -u -p -r1.14 hppa-hpux-tdep.c
--- hppa-hpux-tdep.c	8 Apr 2004 21:18:12 -0000	1.14
+++ hppa-hpux-tdep.c	10 Apr 2004 08:50:20 -0000
@@ -727,6 +727,9 @@ hppa_hpux_som_init_abi (struct gdbarch_i
 static void
 hppa_hpux_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  tdep->is_elf = 1;
   hppa_hpux_init_abi (info, gdbarch);
 }
 
Index: hppa-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
retrieving revision 1.142
diff -u -p -r1.142 hppa-tdep.c
--- hppa-tdep.c	6 Apr 2004 16:11:02 -0000	1.142
+++ hppa-tdep.c	10 Apr 2004 08:50:20 -0000
@@ -360,10 +360,14 @@ static CORE_ADDR low_text_segment_addres
 static void
 record_text_segment_lowaddr (bfd *abfd, asection *section, void *ignored)
 {
-  if (((section->flags & (SEC_ALLOC | SEC_LOAD | SEC_READONLY))
+  if ((section->flags & (SEC_ALLOC | SEC_LOAD | SEC_READONLY))
        == (SEC_ALLOC | SEC_LOAD | SEC_READONLY))
-      && section->vma < low_text_segment_address)
-    low_text_segment_address = section->vma;
+    {
+      bfd_vma value = section->vma - section->filepos;
+
+      if (value < low_text_segment_address)
+          low_text_segment_address = value;
+    }
 }
 
 static void
@@ -381,22 +385,18 @@ internalize_unwinds (struct objfile *obj
 
       low_text_segment_address = -1;
 
-      /* If addresses are 64 bits wide, then unwinds are supposed to
+      /* For ELF targets, then unwinds are supposed to
 	 be segment relative offsets instead of absolute addresses. 
 
 	 Note that when loading a shared library (text_offset != 0) the
 	 unwinds are already relative to the text_offset that will be
 	 passed in.  */
-      if (TARGET_PTR_BIT == 64 && text_offset == 0)
+      if (gdbarch_tdep (current_gdbarch)->is_elf && text_offset == 0)
 	{
 	  bfd_map_over_sections (objfile->obfd,
 				 record_text_segment_lowaddr, NULL);
 
-	  /* ?!? Mask off some low bits.  Should this instead subtract
-	     out the lowest section's filepos or something like that?
-	     This looks very hokey to me.  */
-	  low_text_segment_address &= ~0xfff;
-	  text_offset += low_text_segment_address;
+	  text_offset = low_text_segment_address;
 	}
 
       bfd_get_section_contents (objfile->obfd, section, buf, 0, size);
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] fix unwind handling on hppa elf targets

[Joel Brobecker]

> 2004-04-10  Randolph Chung  <tausq@debian.org>
> 
> 	* hppa-tdep.h (gdbarch_tdep): Add is_elf flag.
> 	* hppa-hpux-tdep.c (hppa_hpux_elf_init_abi): Set is_elf flag.
> 	* hppa-tdep.c (record_text_segment_lowaddr): Calculate text segment
> 	address using file offset.
> 	(internalize_unwinds): Predicate unwind offset calculation on is_elf
> 	flag rather than on width of binary. Remove "hokey" text segment
> 	masking (it is indeed wrong).

It looks like the value of is_elf is undefined in the case of hppa32-hpux.
I think you need to set it to zero in hppa_gdbarch_init().

I'd like to be given some time to test this patch on hppa32 and hppa64
before this gets checked in. Would that be ok?

> 
> Index: hppa-tdep.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/hppa-tdep.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 hppa-tdep.h
> --- hppa-tdep.h	15 Aug 2003 23:02:03 -0000	1.1
> +++ hppa-tdep.h	10 Apr 2004 08:50:20 -0000
> @@ -21,10 +21,13 @@
>  #ifndef HPPA_TDEP_H
>  #define HPPA_TDEP_H
>  
>  /* Target-dependent structure in gdbarch.  */
>  struct gdbarch_tdep
>  {
>    /* The number of bytes in an address.  For now, this field is designed
>       to allow us to differentiate hppa32 from hppa64 targets.  */
>    int bytes_per_address;
> +
> +  /* Is this an ELF target? This can be 64-bit HP-UX, or 32/64-bit Linux */
> +  int is_elf;
>  };
> Index: hppa-hpux-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/hppa-hpux-tdep.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 hppa-hpux-tdep.c
> --- hppa-hpux-tdep.c	8 Apr 2004 21:18:12 -0000	1.14
> +++ hppa-hpux-tdep.c	10 Apr 2004 08:50:20 -0000
> @@ -727,6 +727,9 @@ hppa_hpux_som_init_abi (struct gdbarch_i
>  static void
>  hppa_hpux_elf_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
> +  tdep->is_elf = 1;
>    hppa_hpux_init_abi (info, gdbarch);
>  }
>  
> Index: hppa-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/hppa-tdep.c,v
> retrieving revision 1.142
> diff -u -p -r1.142 hppa-tdep.c
> --- hppa-tdep.c	6 Apr 2004 16:11:02 -0000	1.142
> +++ hppa-tdep.c	10 Apr 2004 08:50:20 -0000
> @@ -360,10 +360,14 @@ static CORE_ADDR low_text_segment_addres
>  static void
>  record_text_segment_lowaddr (bfd *abfd, asection *section, void *ignored)
>  {
> -  if (((section->flags & (SEC_ALLOC | SEC_LOAD | SEC_READONLY))
> +  if ((section->flags & (SEC_ALLOC | SEC_LOAD | SEC_READONLY))
>         == (SEC_ALLOC | SEC_LOAD | SEC_READONLY))
> -      && section->vma < low_text_segment_address)
> -    low_text_segment_address = section->vma;
> +    {
> +      bfd_vma value = section->vma - section->filepos;
> +
> +      if (value < low_text_segment_address)
> +          low_text_segment_address = value;
> +    }
>  }
>  
>  static void
> @@ -381,22 +385,18 @@ internalize_unwinds (struct objfile *obj
>  
>        low_text_segment_address = -1;
>  
> -      /* If addresses are 64 bits wide, then unwinds are supposed to
> +      /* For ELF targets, then unwinds are supposed to
>  	 be segment relative offsets instead of absolute addresses. 
>  
>  	 Note that when loading a shared library (text_offset != 0) the
>  	 unwinds are already relative to the text_offset that will be
>  	 passed in.  */
> -      if (TARGET_PTR_BIT == 64 && text_offset == 0)
> +      if (gdbarch_tdep (current_gdbarch)->is_elf && text_offset == 0)
>  	{
>  	  bfd_map_over_sections (objfile->obfd,
>  				 record_text_segment_lowaddr, NULL);
>  
> -	  /* ?!? Mask off some low bits.  Should this instead subtract
> -	     out the lowest section's filepos or something like that?
> -	     This looks very hokey to me.  */
> -	  low_text_segment_address &= ~0xfff;
> -	  text_offset += low_text_segment_address;
> +	  text_offset = low_text_segment_address;
>  	}
>  
>        bfd_get_section_contents (objfile->obfd, section, buf, 0, size);
> -- 
> Randolph Chung
> Debian GNU/Linux Developer, hppa/ia64 ports
> http://www.tausq.org/

-- 
Joel

Re: [patch] fix unwind handling on hppa elf targets

[Randolph Chung]

> It looks like the value of is_elf is undefined in the case of hppa32-hpux.
> I think you need to set it to zero in hppa_gdbarch_init().

hrm, i thought the struct is zero-filled, but you are right, i guess it
is not.

> I'd like to be given some time to test this patch on hppa32 and hppa64
> before this gets checked in. Would that be ok?

pls do. i do try to test hpux2.0w, but i don't have access to a narrow
hpux machine to test.

thanks, :)
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] fix unwind handling on hppa elf targets

[Joel Brobecker]

On Sat, Apr 10, 2004 at 10:45:01AM -0700, Randolph Chung wrote:
> > It looks like the value of is_elf is undefined in the case of hppa32-hpux.
> > I think you need to set it to zero in hppa_gdbarch_init().
> 
> hrm, i thought the struct is zero-filled, but you are right, i guess it
> is not.

Perhaps this is what we should do? I've never been a fan of zero-filling, 
but that might be because I come from Ada.

> > I'd like to be given some time to test this patch on hppa32 and hppa64
> > before this gets checked in. Would that be ok?
> 
> pls do. i do try to test hpux2.0w, but i don't have access to a narrow
> hpux machine to test.

Have you already tested this patch on hpux-2.0w? That would probably
save me a fair bit of time if I didn't have to.

-- 
Joel

Re: [patch] fix unwind handling on hppa elf targets

[Randolph Chung]

> Have you already tested this patch on hpux-2.0w? That would probably
> save me a fair bit of time if I didn't have to.

i've done some testing. any particular scenario you want me to test?
there are hundreds of test failures on hppa (hpux and linux) so it's
difficult to use that to gauge a patch, but in this case i've mostly
been looking at "maint print unwind <func>" and matching it against the
assembly output (objdump -d). I should also point out that i only test 
against gcc/binutils. if you are interested in hpux toolchain please 
give that a spin.

thanks
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [patch] fix unwind handling on hppa elf targets

[Joel Brobecker]

> i've done some testing. any particular scenario you want me to test?
> there are hundreds of test failures on hppa (hpux and linux) so it's
> difficult to use that to gauge a patch,

I know, but I'd like to avoid that number of failures to grow, so I
really think rerunning the testsuite at each patch is useful.

> I should also point out that i only test against gcc/binutils. if you
> are interested in hpux toolchain please give that a spin.

I don't have access to the HPUX tools, so I also test using GCC/GNAT
too.  Michael Chastain, on the other hand, has been giving this
combination several spins. He sends his results from time to time.

Anyway: I verified your patch against the testsuite on HPUX 11 with
both 32bits and 64bits modes. No regression reported. I should note
that I had to modify your patch for two things:
  
  - zero initialize the tdep structure, but allocation using XZALLOC
    instead of the XMALLOC macro.

  - Tweak the patch to add a #include of hppa-tdep.h in
    hppa-hpux-tdep.c. Your patch seems to be dependent on another
    patch which I wasn't able to locate in the gdb-patches archives.
    So I just did this tweak which was good enough for my purpose.
    (oh, and the patch to hppa-tdep.h didn't apply cleanly either -
    for the same reason I would imagine)

(I should also add that I couldn't review your patch, not enough
knowledge about hppa/hpux and not enough time right now to study it.
I hope somebody else will be able to help you there).

-- 
Joel

Re: [patch] fix unwind handling on hppa elf targets

[Randolph Chung]

> I know, but I'd like to avoid that number of failures to grow, so I
> really think rerunning the testsuite at each patch is useful.

sure, i can do that too. right now i'm trying to figure out some basic
things that are broken. i'm hoping that fixing a few things will cut
down the number of failures drastically :)

it seems to me that frame unwinding is rather broken still :( i suspect
that's causing a large number of failures.

> Anyway: I verified your patch against the testsuite on HPUX 11 with
> both 32bits and 64bits modes. No regression reported. I should note
> that I had to modify your patch for two things:

thanks for the feedback. i have a few cleanup patches in the queue so i
might have missed some bits. sorry about that. i'll post an updated/
cleaned up version for review.

thanks
randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/