[RFC]: patch #2 for Sun C compiled target programs

[RFC]: patch #2 for Sun C compiled target programs

[Michael Mueller]

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

For Sun C compiled 64 bit target programs "print localvar" does not work 
(PR gdb/1669).

I verified this against these compiler versions:

   Sun C 5.5 2003/03/12
   Forte Developer 7 C 5.4 2002/03/09
   Sun WorkShop 6 update 2 C 5.3 2001/05/15
   Sun WorkShop 6 2000/04/07 C 5.1

This is what happens:

   GNU gdb 2004-06-17-cvs
   Copyright 2004 Free Software Foundation, Inc.
   GDB is free software, covered by the GNU General Public License, and 
you are
   welcome to change it and/or distribute copies of it under certain 
conditions.
   Type "show copying" to see the conditions.
   There is absolutely no warranty for GDB.  Type "show warranty" for 
details.
   This GDB was configured as "sparc-sun-solaris2.8"...
   (gdb) b main
   Breakpoint 1 at 0x100000894: file p2.c, line 6.
   (gdb) run
   Starting program: /export/home/michaelm/gdb/gdb-6.1.patch/tst/p2

   Breakpoint 1, main () at p2.c:6
   6               int n = 7;
   (gdb) p n
   Cannot access memory at address 0x7ffff009


There are 2 different problems that need to be fixed. One might involve 
binutils.

*** Problem 1 *************************************************

In dbxread.c function read_ofile_symtab macro INTERNALIZE_SYMBOL (nlist, 
bufp, abfd) is called to set nlist.n_value (type bfd_vma = unsigned 
long, size 64 bit) to the negative offset of a local variable inside the 
stack frame. This offset is taken from bufp->e_value which is 4 bytes 
(bfd_byte e_value[4]). This is the macro definition:

#define INTERNALIZE_SYMBOL(intern, extern, abfd)                 \
   {                                                              \
     (intern).n_type = bfd_h_get_8 (abfd, (extern)->e_type);      \
     (intern).n_strx = bfd_h_get_32 (abfd, (extern)->e_strx);     \
     (intern).n_desc = bfd_h_get_16 (abfd, (extern)->e_desc);     \
     if (bfd_get_sign_extend_vma (abfd))                          \
       (intern).n_value = bfd_h_get_signed_32 (abfd, (extern)->e_value);\
     else                                                         \
       (intern).n_value = bfd_h_get_32 (abfd, (extern)->e_value); \
   }

The problem is that bfd_get_sign_extend_vma returns 0 and the negative 
value is not sign extended and turns into a large positive number.

There was a similar problem in the past for mips 64 bit:

     The problem with stabs and sign extension
     http://sources.redhat.com/ml/gdb/2001-08/msg00078.html

     db/ChangeLog-2001:
     2001-08-14  H.J. Lu  (hjl@gnu.org)

It was fixed by introducing the call to bfd_get_sign_extend_vma into the 
macro. Following this example one could fix this by changing binutils to 
make bfd_get_sign_extend_vma return 1 for sparc solaris 64 bit.

Function bfd_get_sign_extend_vma is also called in dwarf2read.c and I 
don't know what the effects of such a change would be there. I'm also 
not sure if this it the intended use of bfd_get_sign_extend_vma.

As a temporary workaround I changed INTERNALIZE_SYMBOL to always call 
bfd_h_get_signed_32 (see the appended dbxread.workaround).


*** Problem 2 *************************************************

Function sparc64_frame_base_address in sparc64-tdep.c needs to be fixed:

   /* ??? Should we take BIAS into account here?  */
   return cache->base;


The answer to the question in comment is yes, see the appended patch.



2004-05-18  Michael Mueller  <m.mueller99@kay-mueller.de>
     * sparc64-tdep.c: fix PR gdb/1669, printing of 64 bit
       local variables


[-- Attachment #2: sparc64-tdep.patch --]
[-- Type: text/plain, Size: 796 bytes --]

Index: sparc64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64-tdep.c,v
retrieving revision 1.12
diff -c -p -r1.12 sparc64-tdep.c
*** sparc64-tdep.c	7 Jun 2004 02:02:55 -0000	1.12
--- sparc64-tdep.c	18 Jun 2004 12:41:26 -0000
*************** sparc64_frame_base_address (struct frame
*** 568,575 ****
    struct sparc_frame_cache *cache =
      sparc64_frame_cache (next_frame, this_cache);
  
!   /* ??? Should we take BIAS into account here?  */
!   return cache->base;
  }
  
  static const struct frame_base sparc64_frame_base =
--- 568,574 ----
    struct sparc_frame_cache *cache =
      sparc64_frame_cache (next_frame, this_cache);
  
!   return cache->base + BIAS;
  }
  
  static const struct frame_base sparc64_frame_base =

[-- Attachment #3: dbxread.workaround --]
[-- Type: text/plain, Size: 837 bytes --]




###### This is a WORKAROUND, not a proper patch: #######


*** dbxread.c.orig	Fri Jun 18 14:56:56 2004
--- dbxread.c	Fri Jun 18 14:56:55 2004
*************** stabs_seek (int sym_offset)
*** 852,861 ****
--- 852,865 ----
      (intern).n_type = bfd_h_get_8 (abfd, (extern)->e_type);		\
      (intern).n_strx = bfd_h_get_32 (abfd, (extern)->e_strx);		\
      (intern).n_desc = bfd_h_get_16 (abfd, (extern)->e_desc);  		\
+ /*** workaround for Solaris sparc 64					\
      if (bfd_get_sign_extend_vma (abfd))					\
+ ***/									\
        (intern).n_value = bfd_h_get_signed_32 (abfd, (extern)->e_value);	\
+ /*** workaround for Solaris sparc 64					\
      else								\
        (intern).n_value = bfd_h_get_32 (abfd, (extern)->e_value);	\
+ ***/									\
    }
  
  /* Invariant: The symbol pointed to by symbuf_idx is the first one

Re: [RFC]: patch #2 for Sun C compiled target programs

[Mark Kettenis]

   Date: Fri, 18 Jun 2004 19:21:13 +0200
   From: Michael Mueller <m.mueller99@kay-mueller.de>

   For Sun C compiled 64 bit target programs "print localvar" does not work 
   (PR gdb/1669).

   I verified this against these compiler versions:

      Sun C 5.5 2003/03/12
      Forte Developer 7 C 5.4 2002/03/09
      Sun WorkShop 6 update 2 C 5.3 2001/05/15
      Sun WorkShop 6 2000/04/07 C 5.1

Would it be difficult for you to test with GCC too?

   This is what happens:

   There are 2 different problems that need to be fixed. One might involve 
   binutils.

   *** Problem 1 *************************************************

   In dbxread.c function read_ofile_symtab macro INTERNALIZE_SYMBOL (nlist, 
   bufp, abfd) is called to set nlist.n_value (type bfd_vma = unsigned 
   long, size 64 bit) to the negative offset of a local variable inside the 
   stack frame. This offset is taken from bufp->e_value which is 4 bytes 
   (bfd_byte e_value[4]). This is the macro definition:

   #define INTERNALIZE_SYMBOL(intern, extern, abfd)                 \
      {                                                              \
	(intern).n_type = bfd_h_get_8 (abfd, (extern)->e_type);      \
	(intern).n_strx = bfd_h_get_32 (abfd, (extern)->e_strx);     \
	(intern).n_desc = bfd_h_get_16 (abfd, (extern)->e_desc);     \
	if (bfd_get_sign_extend_vma (abfd))                          \
	  (intern).n_value = bfd_h_get_signed_32 (abfd, (extern)->e_value);\
	else                                                         \
	  (intern).n_value = bfd_h_get_32 (abfd, (extern)->e_value); \
      }

   The problem is that bfd_get_sign_extend_vma returns 0 and the negative 
   value is not sign extended and turns into a large positive number.

   There was a similar problem in the past for mips 64 bit:

	The problem with stabs and sign extension
	http://sources.redhat.com/ml/gdb/2001-08/msg00078.html

	db/ChangeLog-2001:
	2001-08-14  H.J. Lu  (hjl@gnu.org)

   It was fixed by introducing the call to bfd_get_sign_extend_vma into the 
   macro. Following this example one could fix this by changing binutils to 
   make bfd_get_sign_extend_vma return 1 for sparc solaris 64 bit.

   Function bfd_get_sign_extend_vma is also called in dwarf2read.c and I 
   don't know what the effects of such a change would be there. I'm also 
   not sure if this it the intended use of bfd_get_sign_extend_vma.

   As a temporary workaround I changed INTERNALIZE_SYMBOL to always call 
   bfd_h_get_signed_32 (see the appended dbxread.workaround).

Sorry but that change is unacceptable.  It's an obvious hack and might
break other targets.  It's not at all clear what values are supposed
to be sign-extended and what values are not, as mentioned in the
thread cited by you.

The real problem is that dbxread.c was initially written as 32-bit
only code.  The sign-extension problem you're seeing here can also be
interpreted as a 64-bit-dirty issue.

   *** Problem 2 *************************************************

   Function sparc64_frame_base_address in sparc64-tdep.c needs to be fixed:

      /* ??? Should we take BIAS into account here?  */
      return cache->base;


   The answer to the question in comment is yes, see the appended patch.

That sounds reasonable.  I'll commit that bit if it works with
GCC/DWARF too.

Mark

Re: [RFC]: patch #2 for Sun C compiled target programs

[Michael Mueller]

Mark Kettenis wrote:
>    Date: Fri, 18 Jun 2004 19:21:13 +0200
>    From: Michael Mueller <m.mueller99@kay-mueller.de>
> 
>    For Sun C compiled 64 bit target programs "print localvar" does not work 
>    (PR gdb/1669).
> 
>    I verified this against these compiler versions:
> 
>       Sun C 5.5 2003/03/12
>       Forte Developer 7 C 5.4 2002/03/09
>       Sun WorkShop 6 update 2 C 5.3 2001/05/15
>       Sun WorkShop 6 2000/04/07 C 5.1
> 
> Would it be difficult for you to test with GCC too?

I tested with gcc 3.4 and found no problem.


> Sorry but that change is unacceptable.  It's an obvious hack and might
> break other targets.  It's not at all clear what values are supposed
> to be sign-extended and what values are not, as mentioned in the
> thread cited by you.

I did not expect this. Of curse it's just a hack. Sorry for the 
misunderstanding.

> 
> The real problem is that dbxread.c was initially written as 32-bit
> only code.  The sign-extension problem you're seeing here can also be
> interpreted as a 64-bit-dirty issue.

So how can it be fixed?

> 
>    *** Problem 2 *************************************************
> 
>    Function sparc64_frame_base_address in sparc64-tdep.c needs to be fixed:
> 
>       /* ??? Should we take BIAS into account here?  */
>       return cache->base;
> 
> 
>    The answer to the question in comment is yes, see the appended patch.
> 
> That sounds reasonable.  I'll commit that bit if it works with
> GCC/DWARF too.

I debugged gcc3.4/dwarf. It does not call sparc64_frame_base_address. (I 
assume dwarf uses a location expression of it's own to describe a fp 
based address + BIAS as the location of a stack variable.)


> 
> Mark
> 

Re: [RFC]: patch #2 for Sun C compiled target programs

[Mark Kettenis]

   Date: Mon, 21 Jun 2004 17:04:33 +0200
   From: Michael Mueller <m.mueller99@kay-mueller.de>

   > 
   >    *** Problem 2 *************************************************
   > 
   >    Function sparc64_frame_base_address in sparc64-tdep.c needs to
   >    be fixed:
   > 
   >       /* ??? Should we take BIAS into account here?  */
   >       return cache->base;
   > 
   > 
   >    The answer to the question in comment is yes, see the appended patch.
   > 
   > That sounds reasonable.  I'll commit that bit if it works with
   > GCC/DWARF too.

   I debugged gcc3.4/dwarf. It does not call sparc64_frame_base_address. (I 
   assume dwarf uses a location expression of it's own to describe a fp 
   based address + BIAS as the location of a stack variable.)

Probably, yes.  I've committed the attached.

Thanks,

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	From Michael Mueller <m.mueller99@kay-mueller.de>:
	* sparc64-tdep.c (sparc64_frame_base_address): Take BIAS into
	account.

Index: sparc64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc64-tdep.c,v
retrieving revision 1.12
diff -u -p -r1.12 sparc64-tdep.c
--- sparc64-tdep.c 7 Jun 2004 02:02:55 -0000 1.12
+++ sparc64-tdep.c 24 Jun 2004 19:31:51 -0000
@@ -568,8 +568,7 @@ sparc64_frame_base_address (struct frame
   struct sparc_frame_cache *cache =
     sparc64_frame_cache (next_frame, this_cache);
 
-  /* ??? Should we take BIAS into account here?  */
-  return cache->base;
+  return cache->base + BIAS;
 }
 
 static const struct frame_base sparc64_frame_base =

Re: [RFC]: patch #2 for Sun C compiled target programs

[Mark Kettenis]

   Date: Mon, 21 Jun 2004 17:04:33 +0200
   From: Michael Mueller <m.mueller99@kay-mueller.de>

   > 
   > The real problem is that dbxread.c was initially written as 32-bit
   > only code.  The sign-extension problem you're seeing here can also be
   > interpreted as a 64-bit-dirty issue.

   So how can it be fixed?

Not sure.  As I said, it's not easy.  As far as I understand things,
INTERNALIZE_SYMBOL() should look at n_type to decide whether n_value
should be treated as a signed or an unsigned value.  The problem seems
to be that n_value can either be interpreted as an address or as an
offset.  Addresses should be sign-extended based on what
bfd_get_sign_extend_vma() tells us.  Offsets should probably always be
sign-extended.

Mark

Re: [RFC]: patch #2 for Sun C compiled target programs

[Michael Mueller]

Mark Kettenis wrote:
>    Date: Mon, 21 Jun 2004 17:04:33 +0200
>    From: Michael Mueller <m.mueller99@kay-mueller.de>
> 
>    > 
>    > The real problem is that dbxread.c was initially written as 32-bit
>    > only code.  The sign-extension problem you're seeing here can also be
>    > interpreted as a 64-bit-dirty issue.
> 
>    So how can it be fixed?
> 
> Not sure.  As I said, it's not easy.  As far as I understand things,
> INTERNALIZE_SYMBOL() should look at n_type to decide whether n_value
> should be treated as a signed or an unsigned value.  The problem seems
> to be that n_value can either be interpreted as an address or as an
> offset.  Addresses should be sign-extended based on what
> bfd_get_sign_extend_vma() tells us.  Offsets should probably always be
> sign-extended.
> 
> Mark
> 


How about sign extending it a bit alter in process_one_symbol when we 
find out it's a stack variable? I cannot imagine an architecture where a 
stack variable has an absolute address. But my imagination is limited.

Michael

Re: [RFC]: patch #2 for Sun C compiled target programs

[Michael Mueller]

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


> Not sure.  As I said, it's not easy.  As far as I understand things,
> INTERNALIZE_SYMBOL() should look at n_type to decide whether n_value
> should be treated as a signed or an unsigned value.  The problem seems
> to be that n_value can either be interpreted as an address or as an
> offset.  Addresses should be sign-extended based on what
> bfd_get_sign_extend_vma() tells us.  Offsets should probably always be
> sign-extended.
> 
> Mark
> 

The gdb stabs docu says:

    4.1 Automatic Variables Allocated on the Stack

    The value of the stab is the offset of the variable within the
    local variables. On most machines this is an offset from the
    frame pointer and is negative.

I think we could sign extend e_value if bfd_get_sign_extend_vma() tells 
us OR if n_type == N_LSYM.

N_LSYM is also used for types. But e_value seems to be unused and = 0 in 
this case.

[-- Attachment #2: dbxread.patch --]
[-- Type: text/plain, Size: 1256 bytes --]

Index: dbxread.c
===================================================================
RCS file: /cvs/src/src/gdb/dbxread.c,v
retrieving revision 1.67
diff -c -p -r1.67 dbxread.c
*** dbxread.c	10 Jun 2004 20:05:43 -0000	1.67
--- dbxread.c	28 Jun 2004 07:39:31 -0000
*************** stabs_seek (int sym_offset)
*** 852,858 ****
      (intern).n_type = bfd_h_get_8 (abfd, (extern)->e_type);		\
      (intern).n_strx = bfd_h_get_32 (abfd, (extern)->e_strx);		\
      (intern).n_desc = bfd_h_get_16 (abfd, (extern)->e_desc);  		\
!     if (bfd_get_sign_extend_vma (abfd))					\
        (intern).n_value = bfd_h_get_signed_32 (abfd, (extern)->e_value);	\
      else								\
        (intern).n_value = bfd_h_get_32 (abfd, (extern)->e_value);	\
--- 852,859 ----
      (intern).n_type = bfd_h_get_8 (abfd, (extern)->e_type);		\
      (intern).n_strx = bfd_h_get_32 (abfd, (extern)->e_strx);		\
      (intern).n_desc = bfd_h_get_16 (abfd, (extern)->e_desc);  		\
!     /* stack variable offsets (N_LSYM) are always signed */		\
!     if ((intern).n_type == N_LSYM || bfd_get_sign_extend_vma (abfd))	\
        (intern).n_value = bfd_h_get_signed_32 (abfd, (extern)->e_value);	\
      else								\
        (intern).n_value = bfd_h_get_32 (abfd, (extern)->e_value);	\