[patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Paul Pluzhnikov]

Greetings,

This came up while debugging one of my core files.
The symptom is:

GNU gdb (GDB) 6.8.50.20081209-cvs
...
This GDB was configured as "x86_64-unknown-linux-gnu".
...
Core was generated by `/tmp/a.out'.
Program terminated with signal 11, Segmentation fault.
[New process 7763]
[New process 13974]
[New process 7773]
#0  chunk_alloc (ar_ptr=Cannot access memory at address 0xffffbf88      <<< What's that?
) at malloc.c:2896
2896    malloc.c: No such file or directory.
       in malloc.c
(gdb) p ar_ptr
Cannot access memory at address 0xffffbf88

32-bit gdb doesn't have a problem on the same core:

Program terminated with signal 11, Segmentation fault.
[New process 7763]
[New process 13974]
[New process 7773]
#0  chunk_alloc (ar_ptr=0x1184f00, nb=10249) at malloc.c:2896

The problem is that in findvar.c:

    case LOC_ARG:
      if (frame == NULL)
	return 0;
      addr = get_frame_args_address (frame);
      if (!addr)
	return 0;
      addr += SYMBOL_VALUE (var);
      break;

What happens if sizeof(addr) == 8 (64-bit gdb), len == 4 (32-bit target),
get_frame_args_address() returns 0xffffbf98 (typical stack address)
and SYMBOL_VALUE() returns -16? 

We end up with an impossible target address of 0x1ffffbf88.

Attached patch fixes this.
OK?

--
Paul Pluzhnikov

2008-12-08  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* findvar.c (read_var_value): Truncate addr to target len.
	

Index: findvar.c
===================================================================
RCS file: /cvs/src/src/gdb/findvar.c,v
retrieving revision 1.119
diff -u -p -u -r1.119 findvar.c
--- findvar.c	2 Dec 2008 14:51:00 -0000	1.119
+++ findvar.c	9 Dec 2008 01:32:10 -0000
@@ -565,6 +565,10 @@ read_var_value (struct symbol *var, stru
       break;
     }
 
+  if (sizeof (addr) > len)
+    /* Truncate address to target length.  */
+    addr &= (1L << (8 * len)) - 1;
+
   VALUE_ADDRESS (v) = addr;
   set_value_lazy (v, 1);
   return v;

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Daniel Jacobowitz]

On Mon, Dec 08, 2008 at 05:32:52PM -0800, Paul Pluzhnikov wrote:
> The problem is that in findvar.c:
> 
>     case LOC_ARG:
>       if (frame == NULL)
> 	return 0;
>       addr = get_frame_args_address (frame);
>       if (!addr)
> 	return 0;
>       addr += SYMBOL_VALUE (var);
>       break;
> 
> What happens if sizeof(addr) == 8 (64-bit gdb), len == 4 (32-bit target),
> get_frame_args_address() returns 0xffffbf98 (typical stack address)
> and SYMBOL_VALUE() returns -16? 
> 
> We end up with an impossible target address of 0x1ffffbf88.

Conclusion doesn't follow from example, but I think I'm just
confused...  Do you mean that SYMBOL_VALUE is -16U or -16UL, and
that's where the wrapping comes from?  But SYMBOL_VALUE is a long,
signed.  Is SYMBOL_VALUE (long) 0xfffffff0?  If that's the case then
the debug reader might be to blame.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Paul Pluzhnikov]

On Mon, Dec 8, 2008 at 6:07 PM, Daniel Jacobowitz <drow@false.org> wrote:

>> We end up with an impossible target address of 0x1ffffbf88.
>
> Conclusion doesn't follow from example

Well, that's the value I observed in the debugger...

> Do you mean that SYMBOL_VALUE is -16U or -16UL, and
> that's where the wrapping comes from?  But SYMBOL_VALUE is a long,
> signed.  Is SYMBOL_VALUE (long) 0xfffffff0?

Yes:

(top) p *var
$1 = {ginfo = {name = 0x16feaa0 "ar_ptr", value = {ivalue =
4294967280, block = 0xfffffff0,
      bytes = 0xfffffff0 <Address 0xfffffff0 out of bounds>, address =
4294967280, chain = 0xfffffff0}, language_specific = {cplus_specific =
{
        demangled_name = 0x0}}, language = language_c, section = 0,
obj_section = 0x0}, type = 0x16fe418, symtab = 0x1702900,
  domain = VAR_DOMAIN, aclass = LOC_ARG, is_argument = 1, line = 2742,
ops = 0x0, aux_value = 0x0, hash_next = 0x0}

That value was set here:

#0  define_symbol (valu=4294967280, string=0x2aaaabd3285c
"ar_ptr:p(0,32)", desc=2742, type=160, objfile=0xed4070)
    at ../../src/gdb/stabsread.c:936
#1  0x0000000000545f98 in process_one_symbol (type=160, desc=2742,
valu=4294967280, name=0x2aaaabd3285c "ar_ptr:p(0,32)",
    section_offsets=0xe9bcf0, objfile=0xed4070) at ../../src/gdb/dbxread.c:3178
#2  0x0000000000545001 in read_ofile_symtab (pst=0xf9cde8) at
../../src/gdb/dbxread.c:2600
...

And that (AFAICT) is coming from BFD.

So it appears that BFD gives us 0x00000000fffffff0, but GDB expects
0xfffffffffffffff0

> If that's the case then the debug reader might be to blame.

This is coming from a rather old glibc-2.2.2, compiled with
non-exstent :) old compiler:

  GNU CC version 2.96 20000731 (Red Hat Linux 7.1 2.96-79).

Here is what 'objdump -g /lib/i686/libc.so.6' has to say:

static mchunkptr chunk_alloc (arena *ar_ptr /* 0x00000000fffffff0 */,
size_t nb /* 0x00000000ffffffec */)
{ /* 0x000000000007f2f0 */
  { /* 0x000000000007f2f0 */
    register mbinptr q /* 0x0000000000000002 */;
    register mchunkptr bck /* 0x0000000000000007 */;
...

And here is 'objdump --stabs':

105431 FUN    0      2746   0007f2f0 317480 chunk_alloc:f(0,25)
105432 PSYM   0      2742   fffffff0 317500 ar_ptr:p(0,32)
105433 PSYM   0      2742   ffffffec 317515 nb:p(3,2)


-- 
Paul Pluzhnikov

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Peter Schauer]

I had the same problem a very long time ago when trying to debug
32 bit stabs executables with a sparc Solaris 64 bit GDB.

I came up with the following local hack, but didn't dare to RFA it
back then, as it didn't look clean enough.
We abandoned sparc Solaris development later on and were no longer
interested in a proper fix.

*** ./stabsread.c.orig  Sat Jul 20 11:51:50 2002
--- ./stabsread.c       Sun Jul 28 10:43:45 2002
***************
*** 927,932 ****
--- 927,935 ----
      case 'l':
        SYMBOL_TYPE (sym) = read_type (&p, objfile);
        SYMBOL_CLASS (sym) = LOC_LOCAL;
+       /* Sign extend value for 64x32 GDB.  */
+       if ((sizeof (CORE_ADDR) > 4) && ((valu & ~0x7fffffff) == 0x80000000))
+ 	valu = (valu ^ 0x80000000) - 0x80000000;
        SYMBOL_VALUE (sym) = valu;
        SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
        add_symbol_to_list (sym, &local_symbols);
***************
*** 947,952 ****
--- 950,958 ----
  	SYMBOL_TYPE (sym) = read_type (&p, objfile);
  
        SYMBOL_CLASS (sym) = LOC_ARG;
+       /* Sign extend value for 64x32 GDB.  */
+       if ((sizeof (CORE_ADDR) > 4) && ((valu & ~0x7fffffff) == 0x80000000))
+ 	valu = (valu ^ 0x80000000) - 0x80000000;
        SYMBOL_VALUE (sym) = valu;
        SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
        add_symbol_to_list (sym, &local_symbols);

> On Mon, Dec 8, 2008 at 6:07 PM, Daniel Jacobowitz <drow@false.org> wrote:
> 
> >> We end up with an impossible target address of 0x1ffffbf88.
> >
> > Conclusion doesn't follow from example
> 
> Well, that's the value I observed in the debugger...
> 
> > Do you mean that SYMBOL_VALUE is -16U or -16UL, and
> > that's where the wrapping comes from?  But SYMBOL_VALUE is a long,
> > signed.  Is SYMBOL_VALUE (long) 0xfffffff0?
> 
> Yes:
> 
> (top) p *var
> $1 = {ginfo = {name = 0x16feaa0 "ar_ptr", value = {ivalue =
> 4294967280, block = 0xfffffff0,
>       bytes = 0xfffffff0 <Address 0xfffffff0 out of bounds>, address =
> 4294967280, chain = 0xfffffff0}, language_specific = {cplus_specific =
> {
>         demangled_name = 0x0}}, language = language_c, section = 0,
> obj_section = 0x0}, type = 0x16fe418, symtab = 0x1702900,
>   domain = VAR_DOMAIN, aclass = LOC_ARG, is_argument = 1, line = 2742,
> ops = 0x0, aux_value = 0x0, hash_next = 0x0}
> 
> That value was set here:
> 
> #0  define_symbol (valu=4294967280, string=0x2aaaabd3285c
> "ar_ptr:p(0,32)", desc=2742, type=160, objfile=0xed4070)
>     at ../../src/gdb/stabsread.c:936
> #1  0x0000000000545f98 in process_one_symbol (type=160, desc=2742,
> valu=4294967280, name=0x2aaaabd3285c "ar_ptr:p(0,32)",
>     section_offsets=0xe9bcf0, objfile=0xed4070) at ../../src/gdb/dbxread.c:3178
> #2  0x0000000000545001 in read_ofile_symtab (pst=0xf9cde8) at
> ../../src/gdb/dbxread.c:2600
> ...
> 
> And that (AFAICT) is coming from BFD.
> 
> So it appears that BFD gives us 0x00000000fffffff0, but GDB expects
> 0xfffffffffffffff0
> 
> > If that's the case then the debug reader might be to blame.
> 
> This is coming from a rather old glibc-2.2.2, compiled with
> non-exstent :) old compiler:
> 
>   GNU CC version 2.96 20000731 (Red Hat Linux 7.1 2.96-79).
> 
> Here is what 'objdump -g /lib/i686/libc.so.6' has to say:
> 
> static mchunkptr chunk_alloc (arena *ar_ptr /* 0x00000000fffffff0 */,
> size_t nb /* 0x00000000ffffffec */)
> { /* 0x000000000007f2f0 */
>   { /* 0x000000000007f2f0 */
>     register mbinptr q /* 0x0000000000000002 */;
>     register mchunkptr bck /* 0x0000000000000007 */;
> ...
> 
> And here is 'objdump --stabs':
> 
> 105431 FUN    0      2746   0007f2f0 317480 chunk_alloc:f(0,25)
> 105432 PSYM   0      2742   fffffff0 317500 ar_ptr:p(0,32)
> 105433 PSYM   0      2742   ffffffec 317515 nb:p(3,2)
> 
> 
> -- 
> Paul Pluzhnikov
> 


-- 
Peter Schauer			Peter.Schauer@mytum.de

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Daniel Jacobowitz]

On Mon, Dec 08, 2008 at 06:42:52PM -0800, Paul Pluzhnikov wrote:
> That value was set here:
> 
> #0  define_symbol (valu=4294967280, string=0x2aaaabd3285c
> "ar_ptr:p(0,32)", desc=2742, type=160, objfile=0xed4070)
>     at ../../src/gdb/stabsread.c:936

I was afraid we were going to come back to the stabs reader somehow.

The bit of GDB you're patching is not at fault; either BFD or the
stabs reader is.  The right fix is to end up with a -16 in the symbol.
Something like Peter's patch, I suppose; although I thought something
else had been posted for this recently, maybe by Joel...

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Joel Brobecker]

> The bit of GDB you're patching is not at fault; either BFD or the
> stabs reader is.  The right fix is to end up with a -16 in the symbol.
> Something like Peter's patch, I suppose; although I thought something
> else had been posted for this recently, maybe by Joel...

Not me, no. It's been a while since I last touched stabs and I'm crossing
my fingers...

-- 
Joel

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Paul Pluzhnikov]

On Wed, Dec 10, 2008 at 3:13 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> The bit of GDB you're patching is not at fault; either BFD or the
>> stabs reader is.  The right fix is to end up with a -16 in the symbol.
>> Something like Peter's patch, I suppose; although I thought something
>> else had been posted for this recently, maybe by Joel...
>
> Not me, no. It's been a while since I last touched stabs and I'm crossing
> my fingers...

I think it's pretty clear that BFD is to blame.
I'll try to make a patch and send it there.

Thanks,
-- 
Paul Pluzhnikov

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Paul Pluzhnikov]

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

On Wed, Dec 10, 2008 at 7:58 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:

> I think it's pretty clear that BFD is to blame.

Well, may be not a BFD problem after all...

Does the patch below seem reasonable?

Thanks,
-- 
Paul Pluzhnikov


2008-12-10  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* dbxread.c (stabs_seek): Sign-extend 32-bit STABS values for
	64-bit GDB.

[-- Attachment #2: gdb-1526195-20081210.txt --]
[-- Type: text/plain, Size: 756 bytes --]

Index: dbxread.c
===================================================================
RCS file: /cvs/src/src/gdb/dbxread.c,v
retrieving revision 1.98
diff -u -p -u -r1.98 dbxread.c
--- dbxread.c	1 Oct 2008 16:41:27 -0000	1.98
+++ dbxread.c	10 Dec 2008 22:41:35 -0000
@@ -855,7 +855,7 @@ stabs_seek (int sym_offset)
     (intern).n_type = bfd_h_get_8 (abfd, (extern)->e_type);		\
     (intern).n_other = 0;						\
     (intern).n_desc = bfd_h_get_16 (abfd, (extern)->e_desc);  		\
-    if (bfd_get_sign_extend_vma (abfd))					\
+    if (sizeof (bfd_vma) > 4 || 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);	\

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Andreas Schwab]

Paul Pluzhnikov <ppluzhnikov@google.com> writes:

> 2008-12-10  Paul Pluzhnikov  <ppluzhnikov@google.com>
>
> 	* dbxread.c (stabs_seek): Sign-extend 32-bit STABS values for
> 	64-bit GDB.
> Index: dbxread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dbxread.c,v
> retrieving revision 1.98
> diff -u -p -u -r1.98 dbxread.c
> --- dbxread.c	1 Oct 2008 16:41:27 -0000	1.98
> +++ dbxread.c	10 Dec 2008 22:41:35 -0000
> @@ -855,7 +855,7 @@ stabs_seek (int sym_offset)
>      (intern).n_type = bfd_h_get_8 (abfd, (extern)->e_type);		\
>      (intern).n_other = 0;						\
>      (intern).n_desc = bfd_h_get_16 (abfd, (extern)->e_desc);  		\
> -    if (bfd_get_sign_extend_vma (abfd))					\
> +    if (sizeof (bfd_vma) > 4 || 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);	\

I don't think this does the right thing for genuine addresses beyond
0x80000000.  You can do that only for values that are truely signed,
which stabs has no way to express.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Mark Kettenis]

> Date: Wed, 10 Dec 2008 15:20:37 -0800
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
>
> On Wed, Dec 10, 2008 at 7:58 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> 
> > I think it's pretty clear that BFD is to blame.
> 
> Well, may be not a BFD problem after all...
> 
> Does the patch below seem reasonable?

Sorry, it doesn't to me:
> 
> SW5kZXg6IGRieHJlYWQuYwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09
> PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09ClJDUyBm
> aWxlOiAvY3ZzL3NyYy9zcmMvZ2RiL2RieHJlYWQuYyx2CnJldHJpZXZpbmcg
> cmV2aXNpb24gMS45OApkaWZmIC11IC1wIC11IC1yMS45OCBkYnhyZWFkLmMK
> LS0tIGRieHJlYWQuYwkxIE9jdCAyMDA4IDE2OjQxOjI3IC0wMDAwCTEuOTgK
> KysrIGRieHJlYWQuYwkxMCBEZWMgMjAwOCAyMjo0MTozNSAtMDAwMApAQCAt
> ODU1LDcgKzg1NSw3IEBAIHN0YWJzX3NlZWsgKGludCBzeW1fb2Zmc2V0KQog
> ICAgIChpbnRlcm4pLm5fdHlwZSA9IGJmZF9oX2dldF84IChhYmZkLCAoZXh0
> ZXJuKS0+ZV90eXBlKTsJCVwKICAgICAoaW50ZXJuKS5uX290aGVyID0gMDsJ
> CQkJCQlcCiAgICAgKGludGVybikubl9kZXNjID0gYmZkX2hfZ2V0XzE2IChh
> YmZkLCAoZXh0ZXJuKS0+ZV9kZXNjKTsgIAkJXAotICAgIGlmIChiZmRfZ2V0
> X3NpZ25fZXh0ZW5kX3ZtYSAoYWJmZCkpCQkJCQlcCisgICAgaWYgKHNpemVv
> ZiAoYmZkX3ZtYSkgPiA0IHx8IGJmZF9nZXRfc2lnbl9leHRlbmRfdm1hIChh
> YmZkKSkJCVwKICAgICAgIChpbnRlcm4pLm5fdmFsdWUgPSBiZmRfaF9nZXRf
> c2lnbmVkXzMyIChhYmZkLCAoZXh0ZXJuKS0+ZV92YWx1ZSk7CVwKICAgICBl
> bHNlCQkJCQkJCQlcCiAgICAgICAoaW50ZXJuKS5uX3ZhbHVlID0gYmZkX2hf
> Z2V0XzMyIChhYmZkLCAoZXh0ZXJuKS0+ZV92YWx1ZSk7CVwK

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Paul Pluzhnikov]

On Thu, Dec 11, 2008 at 12:57 AM, Andreas Schwab <schwab@suse.de> wrote:

> Paul Pluzhnikov <ppluzhnikov@google.com> writes:
...
>> -    if (bfd_get_sign_extend_vma (abfd))                                      \
>> +    if (sizeof (bfd_vma) > 4 || bfd_get_sign_extend_vma (abfd))              \
>
> I don't think this does the right thing for genuine addresses beyond
> 0x80000000.  You can do that only for values that are truely signed,
> which stabs has no way to express.

Hmm, I guess if a STABS executable is linked at above 0x80000000,
this could happen.

So, the choices appear to be:

- leave GDB broken and wait for newer glibc :(
- do sign extension, but only for N_LSYM and N_PSYM symbols as
 a heuristic.
- pre-scan the objfile to determine highest link address, and
 do sign extension if that address is (well) below 0x80000000

Suggestions?

Thanks,
--
Paul Pluzhnikov

[Andreas, sorry for the repeat: clicked reply instead of reply-all]

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Joel Brobecker]

> So, the choices appear to be:
> 
> - leave GDB broken and wait for newer glibc :(

I'm just realizing that this might be a very very visible problem,
if most offsets are negative. Is that the case?

> - do sign extension, but only for N_LSYM and N_PSYM symbols as
>  a heuristic.

It seems to me that this should work without negative effect
(we have to be a little careful because there is a 64bit stabs
extension on Tru64).  After all, values for these types of symbols
are always offsets to some location inside a frame, and I can't see
a frame being that big.

> - pre-scan the objfile to determine highest link address, and
>  do sign extension if that address is (well) below 0x80000000

Seems too much work, IMO.

-- 
Joel

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Paul Pluzhnikov]

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

On Sat, Dec 13, 2008 at 8:01 AM, Joel Brobecker <brobecker@adacore.com> wrote:

> > - leave GDB broken and wait for newer glibc :(
>
> I'm just realizing that this might be a very very visible problem,
> if most offsets are negative. Is that the case?

Now that you've mentioned it, I see that there is a trivial test case:

int bar(int x) { return x; }
int foo(int x) { int y = x; return bar(y); }
int main() { return foo(0); }

gcc -gstabs -m32 t.c && gdb64 -q ./a.out

(gdb) b bar
Breakpoint 1 at 0x8048303: file t.c, line 1.
(gdb) r

Breakpoint 1, bar (x=0) at t.c:1
1	int bar(int x) { return x; }
(gdb) up
#1  0x0804831f in foo (x=0) at t.c:2
2	int foo(int x) { int y = x; return bar(y); }
(gdb) info locals
y = Cannot access memory at address 0xffffd954


> > - do sign extension, but only for N_LSYM and N_PSYM symbols as
> >  a heuristic.
>
> It seems to me that this should work without negative effect
> (we have to be a little careful because there is a 64bit stabs
> extension on Tru64).

AFAICT, Tru64 goes through ECOFF reader in mdebugread.c, and so
wouldn't be affected by a change to dbxread.c

> After all, values for these types of symbols
> are always offsets to some location inside a frame, and I can't see
> a frame being that big.

Especially in a 32-bit executable ...

> > - pre-scan the objfile to determine highest link address, and
> >  do sign extension if that address is (well) below 0x80000000
>
> Seems too much work, IMO.

Right.

So here is a 3rd attempt.

Thanks,
--
Paul Pluzhnikov


2008-12-12  Paul Pluzhnikov  <ppluzhnikov@google.com>

        * dbxread.c (read_ofile_symtab): Sign-extend 32-bit N_LSYM and
        N_PSYM STABS values for 64-bit GDB.

[-- Attachment #2: gdb-1526195-20081212.txt --]
[-- Type: text/plain, Size: 721 bytes --]

Index: dbxread.c
===================================================================
RCS file: /cvs/src/src/gdb/dbxread.c,v
retrieving revision 1.98
diff -u -p -u -r1.98 dbxread.c
--- dbxread.c	1 Oct 2008 16:41:27 -0000	1.98
+++ dbxread.c	13 Dec 2008 18:07:31 -0000
@@ -2597,6 +2597,11 @@ read_ofile_symtab (struct partial_symtab
 
       if (type & N_STAB)
 	{
+	  if (sizeof (nlist.n_value) > 4
+	      && (type == N_LSYM || type == N_PSYM))
+	    /* These are very likely to be 32-bit negative.
+	       Sign-extend them for 64-bit GDB.  */
+	    nlist.n_value = (nlist.n_value ^ 0x80000000) - 0x80000000;
 	  process_one_symbol (type, nlist.n_desc, nlist.n_value,
 			      namestring, section_offsets, objfile);
 	}

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Joel Brobecker]

> > It seems to me that this should work without negative effect
> > (we have to be a little careful because there is a 64bit stabs
> > extension on Tru64).
> 
> AFAICT, Tru64 goes through ECOFF reader in mdebugread.c, and so
> wouldn't be affected by a change to dbxread.c

You're probably right. I'm a little fuzzy now about what each module
does and I'm a little tied up to investigate further. Regardless,
I think that this won't affect Tru64 anyway.

> 2008-12-12  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
>         * dbxread.c (read_ofile_symtab): Sign-extend 32-bit N_LSYM and
>         N_PSYM STABS values for 64-bit GDB.

This looks reasonable to me, but I'd really like to have a more detailed
comment:

> +	  if (sizeof (nlist.n_value) > 4
> +	      && (type == N_LSYM || type == N_PSYM))
> +	    /* These are very likely to be 32-bit negative.
> +	       Sign-extend them for 64-bit GDB.  */
> +	    nlist.n_value = (nlist.n_value ^ 0x80000000) - 0x80000000;

In particular:

  . Why are we testing the size of nlist.n_value? (to detect that we
    are using a 64bit debugger)
  . You mention that this is very likely to be 32-bit negative, but
    offsets on some platforms may be positive.

I'd like to see the comment detail a little the situation that we are
trying to solve (64bit gdb debugging a 32bit app). Just as a starting
point, I suggest for instance:

  if (sizeof (nlist.n_value) > 4)
    /* We are a 64bit debugger debugging a 32bit program.
       We have to be a little careful with the n_value in the case
       of N_LSYM and N_PSYM entries, because they are signed offsets
       that we actually read as an unsigned 32bit value. This is
       not a problem with 32bit debuggers where negative values end
       up being interpreted correctly.  But we need to sign-extend
       the sign bit on 64bit debuggers.  Otherwise, we'll end up
       interpreting negative values as very large positive values...  */
    if (type == N_LSYM || type == N_PSYM)
      nlist.n_value = (nlist.n_value ^ 0x80000000) - 0x80000000;

-- 
Joel

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Paul Pluzhnikov]

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

On Mon, Dec 15, 2008 at 12:58 AM, Joel Brobecker <brobecker@adacore.com> wrote:

> I'd like to see the comment detail a little the situation that we are
> trying to solve (64bit gdb debugging a 32bit app). Just as a starting
> point, I suggest for instance:
>
>  if (sizeof (nlist.n_value) > 4)
>    /* We are a 64bit debugger debugging a 32bit program.
>       We have to be a little careful with the n_value in the case
>       of N_LSYM and N_PSYM entries, because they are signed offsets
>       that we actually read as an unsigned 32bit value. This is
>       not a problem with 32bit debuggers where negative values end
>       up being interpreted correctly.  But we need to sign-extend
>       the sign bit on 64bit debuggers.  Otherwise, we'll end up
>       interpreting negative values as very large positive values...  */
>    if (type == N_LSYM || type == N_PSYM)
>      nlist.n_value = (nlist.n_value ^ 0x80000000) - 0x80000000;

Thanks, I slightly re-worded this.

OK to commit?


-- 
Paul Pluzhnikov

2008-12-15  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* dbxread.c (read_ofile_symtab): Sign-extend 32-bit N_LSYM and
	N_PSYM STABS values for 64-bit GDB.

[-- Attachment #2: gdb-1526195-20081215.txt --]
[-- Type: text/plain, Size: 1163 bytes --]

Index: dbxread.c
===================================================================
RCS file: /cvs/src/src/gdb/dbxread.c,v
retrieving revision 1.98
diff -u -r1.98 dbxread.c
--- dbxread.c	1 Oct 2008 16:41:27 -0000	1.98
+++ dbxread.c	15 Dec 2008 21:42:41 -0000
@@ -2597,6 +2597,19 @@
 
       if (type & N_STAB)
 	{
+	  if (sizeof (nlist.n_value) > 4
+	      /* We are a 64-bit debugger debugging a 32-bit program.  */
+	      && (type == N_LSYM || type == N_PSYM))
+	      /* We have to be careful with the n_value in the case of N_LSYM
+		 and N_PSYM entries, because they are signed offsets from frame
+		 pointer, but we actually read them as unsigned 32-bit values.
+		 This is not a problem for 32-bit debuggers, for which negative
+		 values end up being interpreted correctly (as negative
+		 offsets) due to integer overflow.
+		 But we need to sign-extend the value for 64-bit debuggers,
+		 or we'll end up interpreting negative values as very large
+		 positive offsets.  */
+	    nlist.n_value = (nlist.n_value ^ 0x80000000) - 0x80000000;
 	  process_one_symbol (type, nlist.n_desc, nlist.n_value,
 			      namestring, section_offsets, objfile);
 	}

Re: [patch] Fix a glitch in debugging 32-bit process with 64-bit GDB.

[Joel Brobecker]

> 2008-12-15  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* dbxread.c (read_ofile_symtab): Sign-extend 32-bit N_LSYM and
> 	N_PSYM STABS values for 64-bit GDB.

OK!

Thanks,
-- 
Joel