[rfa] Re: The problem with stabs and sign extension

[rfa] Re: The problem with stabs and sign extension

[Daniel Jacobowitz]

On Wed, Aug 08, 2001 at 05:03:26PM -0700, H . J . Lu wrote:
> On Wed, Aug 08, 2001 at 04:53:59PM -0700, H . J . Lu wrote:
> > With this patch, I can do
> > 
> > # gdb vmlinux
> > ...
> > (gdb) print printk
> > $1 = {int (char *)} 0x8011c5b0 <printk>
> > (gdb) list printk
> > 250     printk.c: No such file or directory.
> >         in printk.c
> > 
> > 
> 
> Here is an update.

This isn't quite the right solution, I think.  In particular,

> 	(SWAP_SYMBOL): Removed.
> 	(INTERNALIZE_SYMBOL): Check sign extended vma.

are correct, but:

> 	* dbxread.c (COERCE32): New.
> 	(read_ofile_symtab): Set text_offset with COERCE32 if
> 	necessary.

are not.  The psymtab has the wrong offsets in it if you do this.

There's actually a clear bug here, once you know where to look.  A
CORE_ADDR is assigned to a long int in partial-stab.h.  There's a
warning flag in GCC for this, isn't there?

Please try the patch attached.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

2001-08-08  Daniel Jacobowitz  <drow@mvista.com>

	* partial-stab.h: valu should be a CORE_ADDR.

2001-08-08  H.J. Lu  (hjl@gnu.org)

	* dbxread.c (SWAP_SYMBOL): Removed.
	(INTERNALIZE_SYMBOL): Check sign extended vma.

Index: partial-stab.h
===================================================================
RCS file: /cvs/src/src/gdb/partial-stab.h,v
retrieving revision 1.11
diff -u -p -r1.11 partial-stab.h
--- partial-stab.h	2001/06/01 21:17:50	1.11
+++ partial-stab.h	2001/08/09 00:11:41
@@ -201,7 +201,7 @@ switch (CUR_SYMBOL_TYPE)
 
   case N_SO:
     {
-      unsigned long valu;
+      CORE_ADDR valu;
       static int prev_so_symnum = -10;
       static int first_so_symnum;
       char *p;
Index: dbxread.c
===================================================================
RCS file: /cvs/src/src/gdb/dbxread.c,v
retrieving revision 1.19
diff -u -p -r1.19 dbxread.c
--- dbxread.c	2001/07/07 17:19:50	1.19
+++ dbxread.c	2001/08/09 00:11:41
@@ -944,22 +944,15 @@
   symbuf_read += nbytes;
 }
 
-#define SWAP_SYMBOL(symp, abfd) \
-  { \
-    (symp)->n_strx = bfd_h_get_32(abfd,			\
-				(unsigned char *)&(symp)->n_strx);	\
-    (symp)->n_desc = bfd_h_get_16 (abfd,			\
-				(unsigned char *)&(symp)->n_desc);  	\
-    (symp)->n_value = bfd_h_get_32 (abfd,			\
-				(unsigned char *)&(symp)->n_value); 	\
-  }
-
 #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);  		\
-    (intern).n_value = bfd_h_get_32 (abfd, (extern)->e_value);		\
+    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);	\
   }
 
 /* Invariant: The symbol pointed to by symbuf_idx is the first one

Re: [rfa] Re: The problem with stabs and sign extension

[H . J . Lu]

On Wed, Aug 08, 2001 at 05:16:16PM -0700, Daniel Jacobowitz wrote:
> > 	necessary.
> 
> are not.  The psymtab has the wrong offsets in it if you do this.
> 
> There's actually a clear bug here, once you know where to look.  A
> CORE_ADDR is assigned to a long int in partial-stab.h.  There's a
> warning flag in GCC for this, isn't there?
> 
> Please try the patch attached.

(gdb) list printk
250     printk.c: No such file or directory.
        in printk.c

works. But

(gdb) print printk
During symbol reading, inner block (0x802ac9d4-0xffffffff) not inside outer block (0x802aca18-0xffffffff).
$1 = {int (char *)} 0x8011c5b0 <printk>

Something is still wrong.


H.J.

Re: [rfa] Re: The problem with stabs and sign extension

[Andrew Cagney]

>  * dbxread.c (SWAP_SYMBOL): Removed.


I'll avoid the rest (not my area).  However, this bit caught my 
attention.  As far as I can tell it is obvious (the macro is not used?) 
and hence can be factored out of the patch being discussed.

Another macro bites the dust! :-)

	Andrew

Re: [rfa] Re: The problem with stabs and sign extension

[Daniel Jacobowitz]

On Thu, Aug 09, 2001 at 03:35:59PM -0400, Andrew Cagney wrote:
> >  * dbxread.c (SWAP_SYMBOL): Removed.
> 
> 
> I'll avoid the rest (not my area).  However, this bit caught my 
> attention.  As far as I can tell it is obvious (the macro is not used?) 
> and hence can be factored out of the patch being discussed.
> 
> Another macro bites the dust! :-)

Yep :)  The changelogs mention its introduction, but not its removal
from use.  Probably just slipped away gracefully...

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] Re: The problem with stabs and sign extension

[Daniel Jacobowitz]

On Fri, Aug 10, 2001 at 03:31:49PM -0700, H . J . Lu wrote:
> On Wed, Aug 08, 2001 at 05:16:16PM -0700, Daniel Jacobowitz wrote:
> > 
> > There's actually a clear bug here, once you know where to look.  A
> > CORE_ADDR is assigned to a long int in partial-stab.h.  There's a
> > warning flag in GCC for this, isn't there?
> > 
> 
> You miised one CORE_ADDR in partial-stab.h. This patch seems to fix
> my problem. Any comments?

This looks right to me, though I can't approve anything - Jim?  Elena?

> 
> Thanks.
> 
> 
> H.J.
> -----
> 2001-08-10  Daniel Jacobowitz  <drow@mvista.com>
> 	    H.J. Lu  (hjl@gnu.org)
> 
> 	* partial-stab.h: valu should be a CORE_ADDR.
> 
> 2001-08-08  H.J. Lu  (hjl@gnu.org)
> 
> 	* dbxread.c (SWAP_SYMBOL): Removed.
> 	(INTERNALIZE_SYMBOL): Check sign extended vma.
> 
> --- gdb/dbxread.c.vma	Fri Jul 13 12:12:34 2001
> +++ gdb/dbxread.c	Wed Aug  8 21:02:29 2001
> @@ -946,22 +946,15 @@ fill_symbuf (bfd *sym_bfd)
>    symbuf_read += nbytes;
>  }
>  
> -#define SWAP_SYMBOL(symp, abfd) \
> -  { \
> -    (symp)->n_strx = bfd_h_get_32(abfd,			\
> -				(unsigned char *)&(symp)->n_strx);	\
> -    (symp)->n_desc = bfd_h_get_16 (abfd,			\
> -				(unsigned char *)&(symp)->n_desc);  	\
> -    (symp)->n_value = bfd_h_get_32 (abfd,			\
> -				(unsigned char *)&(symp)->n_value); 	\
> -  }
> -
>  #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);  		\
> -    (intern).n_value = bfd_h_get_32 (abfd, (extern)->e_value);		\
> +    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);	\
>    }
>  
>  /* Invariant: The symbol pointed to by symbuf_idx is the first one
> --- gdb/partial-stab.h.vma	Fri Jul 13 12:12:54 2001
> +++ gdb/partial-stab.h	Fri Aug 10 15:25:10 2001
> @@ -201,7 +201,7 @@ switch (CUR_SYMBOL_TYPE)
>  
>    case N_SO:
>      {
> -      unsigned long valu;
> +      CORE_ADDR valu;
>        static int prev_so_symnum = -10;
>        static int first_so_symnum;
>        char *p;
> @@ -399,7 +399,7 @@ switch (CUR_SYMBOL_TYPE)
>      /* See if this is an end of function stab.  */
>      if (pst && CUR_SYMBOL_TYPE == N_FUN && *namestring == '\000')
>        {
> -	unsigned long valu;
> +	CORE_ADDR valu;
>  
>  	/* It's value is the size (in bytes) of the function for
>  	   function relative stabs, or the address of the function's
> 

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [rfa] Re: The problem with stabs and sign extension

[Elena Zannoni]

Daniel Jacobowitz writes:
 > On Wed, Aug 08, 2001 at 05:03:26PM -0700, H . J . Lu wrote:
 > > On Wed, Aug 08, 2001 at 04:53:59PM -0700, H . J . Lu wrote:
 > > > With this patch, I can do
 > > > 
 > > > # gdb vmlinux
 > > > ...
 > > > (gdb) print printk
 > > > $1 = {int (char *)} 0x8011c5b0 <printk>
 > > > (gdb) list printk
 > > > 250     printk.c: No such file or directory.
 > > >         in printk.c
 > > > 
 > > > 
 > > 
 > > Here is an update.
 > 
 > This isn't quite the right solution, I think.  In particular,
 > 
 > > 	(SWAP_SYMBOL): Removed.
 > > 	(INTERNALIZE_SYMBOL): Check sign extended vma.
 > 
 > are correct, but:
 > 
 > > 	* dbxread.c (COERCE32): New.
 > > 	(read_ofile_symtab): Set text_offset with COERCE32 if
 > > 	necessary.
 > 
 > are not.  The psymtab has the wrong offsets in it if you do this.
 > 
 > There's actually a clear bug here, once you know where to look.  A
 > CORE_ADDR is assigned to a long int in partial-stab.h.  There's a
 > warning flag in GCC for this, isn't there?
 > 
 > Please try the patch attached.
 > 
 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer
 > 

This looks ok to me, can you commit it? Should we commit it to the 5.1
branch as well? How many targets is it going to affect? Just mips, right?
Probably yes then. 

Elena
 


 > 2001-08-08  Daniel Jacobowitz  <drow@mvista.com>
 > 
 > 	* partial-stab.h: valu should be a CORE_ADDR.
 > 
 > 2001-08-08  H.J. Lu  (hjl@gnu.org)
 > 
 > 	* dbxread.c (SWAP_SYMBOL): Removed.
 > 	(INTERNALIZE_SYMBOL): Check sign extended vma.
 > 
 > Index: partial-stab.h
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/partial-stab.h,v
 > retrieving revision 1.11
 > diff -u -p -r1.11 partial-stab.h
 > --- partial-stab.h	2001/06/01 21:17:50	1.11
 > +++ partial-stab.h	2001/08/09 00:11:41
 > @@ -201,7 +201,7 @@ switch (CUR_SYMBOL_TYPE)
 >  
 >    case N_SO:
 >      {
 > -      unsigned long valu;
 > +      CORE_ADDR valu;
 >        static int prev_so_symnum = -10;
 >        static int first_so_symnum;
 >        char *p;
 > Index: dbxread.c
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/dbxread.c,v
 > retrieving revision 1.19
 > diff -u -p -r1.19 dbxread.c
 > --- dbxread.c	2001/07/07 17:19:50	1.19
 > +++ dbxread.c	2001/08/09 00:11:41
 > @@ -944,22 +944,15 @@
 >    symbuf_read += nbytes;
 >  }
 >  
 > -#define SWAP_SYMBOL(symp, abfd) \
 > -  { \
 > -    (symp)->n_strx = bfd_h_get_32(abfd,			\
 > -				(unsigned char *)&(symp)->n_strx);	\
 > -    (symp)->n_desc = bfd_h_get_16 (abfd,			\
 > -				(unsigned char *)&(symp)->n_desc);  	\
 > -    (symp)->n_value = bfd_h_get_32 (abfd,			\
 > -				(unsigned char *)&(symp)->n_value); 	\
 > -  }
 > -
 >  #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);  		\
 > -    (intern).n_value = bfd_h_get_32 (abfd, (extern)->e_value);		\
 > +    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);	\
 >    }
 >  
 >  /* Invariant: The symbol pointed to by symbuf_idx is the first one

Re: [rfa] Re: The problem with stabs and sign extension

[H . J . Lu]

On Wed, Aug 15, 2001 at 01:06:44AM -0400, Elena Zannoni wrote:
> 
> This looks ok to me, can you commit it? Should we commit it to the 5.1

I checked it in.

> branch as well? How many targets is it going to affect? Just mips, right?
> Probably yes then. 
> 

Only mips has sign extended vma. I'd like to see it in gdb 5.1. I have
to apply it when I use gdb on mips.


H.J.

Re: [rfa] Re: The problem with stabs and sign extension

[Elena Zannoni]

H . J . Lu writes:
 > On Wed, Aug 15, 2001 at 01:06:44AM -0400, Elena Zannoni wrote:
 > > 
 > > This looks ok to me, can you commit it? Should we commit it to the 5.1
 > 
 > I checked it in.
 > 
 > > branch as well? How many targets is it going to affect? Just mips, right?
 > > Probably yes then. 
 > > 
 > 
 > Only mips has sign extended vma. I'd like to see it in gdb 5.1. I have
 > to apply it when I use gdb on mips.


It is probably OK. Do you or Daniel want to check it in?

Elena


 > 
 > 
 > H.J.

Re: [rfa] Re: The problem with stabs and sign extension

[H . J . Lu]

On Wed, Aug 15, 2001 at 03:09:24PM -0400, Elena Zannoni wrote:
> H . J . Lu writes:
>  > On Wed, Aug 15, 2001 at 01:06:44AM -0400, Elena Zannoni wrote:
>  > > 
>  > > This looks ok to me, can you commit it? Should we commit it to the 5.1
>  > 
>  > I checked it in.
>  > 
>  > > branch as well? How many targets is it going to affect? Just mips, right?
>  > > Probably yes then. 
>  > > 
>  > 
>  > Only mips has sign extended vma. I'd like to see it in gdb 5.1. I have
>  > to apply it when I use gdb on mips.
> 
> 
> It is probably OK. Do you or Daniel want to check it in?

I don't have the setup for gdb 5.1. Daniel, can you check it in?

Thanks.


H.J.

Re: [rfa] Re: The problem with stabs and sign extension

[Daniel Jacobowitz]

On Wed, Aug 15, 2001 at 12:04:45PM -0700, H . J . Lu wrote:
> On Wed, Aug 15, 2001 at 03:09:24PM -0400, Elena Zannoni wrote:
> > H . J . Lu writes:
> >  > On Wed, Aug 15, 2001 at 01:06:44AM -0400, Elena Zannoni wrote:
> >  > > 
> >  > > This looks ok to me, can you commit it? Should we commit it to the 5.1
> >  > 
> >  > I checked it in.
> >  > 
> >  > > branch as well? How many targets is it going to affect? Just mips, right?
> >  > > Probably yes then. 
> >  > > 
> >  > 
> >  > Only mips has sign extended vma. I'd like to see it in gdb 5.1. I have
> >  > to apply it when I use gdb on mips.
> > 
> > 
> > It is probably OK. Do you or Daniel want to check it in?
> 
> I don't have the setup for gdb 5.1. Daniel, can you check it in?

Doing that right now.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer