gdb code review, pointer madness

gdb code review, pointer madness

[NZG]

I'm trying to get gdb 6.1's machine interface to work correctly with the m68k 
arch, and I'm having some weird results.

I've noticed that if I do a (frame -1) command on gdb after connecting to 
remote gdb server, but before breaking in the main program, gdb goes crazy 
and starts requesting random memory locations from gdbserver until something 
crashes.

Sooo... I'm using regular gdb to debug the m68k-elf-gdb connection, and I'm 
seeing a problem "extract_unsigned_integer" (pasted below).

Specifically this loop doesn't seem to be executing correctly

 for (p = startaddr; p < endaddr; ++p)
	retval = (retval << 8) | *p;

In the function call I'm watching, 
endaddr = startaddr+4, 
yet, when I step through the function the loop executes 8 times and overshoots 
the array.

I don't see anything wrong with the code.
Can anyone else see anything weird in the pointer math below?

thx,
NZG



ULONGEST
extract_unsigned_integer (const void *addr, int len)
{
  ULONGEST retval;
  const unsigned char *p;
  const unsigned char *startaddr = addr;
  const unsigned char *endaddr = startaddr + len;

  if (len > (int) sizeof (ULONGEST))
    error ("\
That operation is not available on integers of more than %d bytes.",
	   (int) sizeof (ULONGEST));

  /* Start at the most significant end of the integer, and work towards
     the least significant.  */
  retval = 0;
  if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
    {
      for (p = startaddr; p < endaddr; ++p)
	retval = (retval << 8) | *p;
    }
  else
    {
      for (p = endaddr - 1; p >= startaddr; --p)
	retval = (retval << 8) | *p;
    }
  return retval;
}

Re: gdb code review, pointer madness

[Daniel Jacobowitz]

On Mon, Jan 23, 2006 at 02:38:25PM -0600, NZG wrote:
> I'm trying to get gdb 6.1's machine interface to work correctly with the m68k 
> arch, and I'm having some weird results.
> 
> I've noticed that if I do a (frame -1) command on gdb after connecting to 
> remote gdb server, but before breaking in the main program, gdb goes crazy 
> and starts requesting random memory locations from gdbserver until something 
> crashes.
> 
> Sooo... I'm using regular gdb to debug the m68k-elf-gdb connection, and I'm 
> seeing a problem "extract_unsigned_integer" (pasted below).
> 
> Specifically this loop doesn't seem to be executing correctly
> 
>  for (p = startaddr; p < endaddr; ++p)
> 	retval = (retval << 8) | *p;
> 
> In the function call I'm watching, 
> endaddr = startaddr+4, 
> yet, when I step through the function the loop executes 8 times and overshoots 
> the array.

You are probably just debugging optimized code.  Either that, or your
host compiler is completely broken; the loop is correct.

-- 
Daniel Jacobowitz
CodeSourcery

Re: gdb code review, pointer madness

[Jim Blandy]

On 1/23/06, NZG <ngustavson@emacinc.com> wrote:
> Specifically this loop doesn't seem to be executing correctly
>
>  for (p = startaddr; p < endaddr; ++p)
>         retval = (retval << 8) | *p;
>
> In the function call I'm watching,
> endaddr = startaddr+4,
> yet, when I step through the function the loop executes 8 times and overshoots
> the array.
>
> I don't see anything wrong with the code.
> Can anyone else see anything weird in the pointer math below?

I don't think there's anything wrong with the C code.  That's a
completely fundamental function in GDB, used everywhere, all the time.
 And it's hard for me to believe that both endiannesses aren't being
exercised.  If it were broken, we surely would have found out by now.

If the function is behaving as you say, then I'd say your compiler has
miscompiled GDB.  Why don't you look through the assembly code for the
function, and see if it's right?

Re: gdb code review, pointer madness

[NZG]

Intro:
This is a continuation of a thread attempting to patch gdb 61 to make it work 
correctly with Eclipse's MI interface. The problem is that the Eclipse CDT 
queries the frame list before it begins execution, which triggers a bug in 
the m68k port of gdb and smashes it.

> I don't think there's anything wrong with the C code.  That's a
> completely fundamental function in GDB, used everywhere, all the time.
>  And it's hard for me to believe that both endiannesses aren't being
> exercised.  If it were broken, we surely would have found out by now.
>
> If the function is behaving as you say, then I'd say your compiler has
> miscompiled GDB.  Why don't you look through the assembly code for the
> function, and see if it's right?
It appears the assembly code for this function is correct (pasted below) but I 
am no wizard at x68 assembly. If somebody else wants to take a gander it 
would be appreciated. 

I built it with
CC=gcc-3.3 ../gdb-6.1/configure  --target=m68k-bdm-elf
make CC=ggc-3.3

after applying the bdm patch;
http://www.uclinux.org/pub/uClinux/m68k-elf-tools/gcc-3/uclinux-tools-20040603/m68k-bdm-1.3.0.tar.bz2
 and replacing 2 files as detailed in:
http://mailman.uclinux.org/pipermail/uclinux-dev/2004-March/024885.html

I think the most likely culprit is some form of stack corruption (such as an 
improperly dereferenced pointer) above it.

The faulty call is being made directly after a call to  
 frame_unwind_register (next_frame, M68K_FP_REGNUM, buf);
is made on the sentinal frame.

I highly suspect something is not being initialized correctly in the m68k 
sentinal frame and that it's causing a dereference off into space from the 
linked list of frames.
Tracking that down though.... has been a challenge.

Call leading up to insane extract_unsigned_integer call
*************************************************************
static struct m68k_frame_cache *
m68k_frame_cache (struct frame_info *next_frame, void **this_cache)
{
  struct m68k_frame_cache *cache;
  char buf[4];
  int i;

  if (*this_cache)
    return *this_cache;

  cache = m68k_alloc_frame_cache ();
  *this_cache = cache;

  /* In principle, for normal frames, %fp holds the frame pointer,
     which holds the base address for the current stack frame.
     However, for functions that don't need it, the frame pointer is
     optional.  For these "frameless" functions the frame pointer is
     actually the frame pointer of the calling frame.  Signal
     trampolines are just a special case of a "frameless" function.
     They (usually) share their frame pointer with the frame that was
     in progress when the signal occurred.  */

  frame_unwind_register (next_frame, M68K_FP_REGNUM, buf);
  cache->base = extract_unsigned_integer (buf, 4);
(returns with some crazy random number obtained from shifting and adding 8 buf 
elements)


disassembly of extract_unsigned_integer
*************************************************************
00000110 <extract_unsigned_integer>:

ULONGEST
extract_unsigned_integer (const void *addr, int len)
{
 110:	55                   	push   %ebp
 111:	89 e5                	mov    %esp,%ebp
 113:	57                   	push   %edi
 114:	56                   	push   %esi
 115:	53                   	push   %ebx
 116:	83 ec 1c             	sub    $0x1c,%esp
 119:	8b 45 0c             	mov    0xc(%ebp),%eax
  ULONGEST retval;
  const unsigned char *p;
  const unsigned char *startaddr = addr;
  const unsigned char *endaddr = startaddr + len;
 11c:	8b 55 08             	mov    0x8(%ebp),%edx
 11f:	01 c2                	add    %eax,%edx

  if (len > (int) sizeof (ULONGEST))
 121:	83 f8 08             	cmp    $0x8,%eax
 124:	89 55 ec             	mov    %edx,0xffffffec(%ebp)
 127:	0f 8f 9b 00 00 00    	jg     1c8 <extract_unsigned_integer+0xb8>
    error ("\
That operation is not available on integers of more than %d bytes.",
	   (int) sizeof (ULONGEST));

  /* Start at the most significant end of the integer, and work towards
     the least significant.  */
  retval = 0;
  if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
 12d:	a1 00 00 00 00       	mov    0x0,%eax
 132:	31 f6                	xor    %esi,%esi
 134:	31 ff                	xor    %edi,%edi
 136:	89 04 24             	mov    %eax,(%esp)
 139:	e8 fc ff ff ff       	call   13a <extract_unsigned_integer+0x2a>
 13e:	85 c0                	test   %eax,%eax
 140:	75 4e                	jne    190 <extract_unsigned_integer+0x80>
    {
      for (p = startaddr; p < endaddr; ++p)
 142:	8b 45 08             	mov    0x8(%ebp),%eax
 145:	8b 55 ec             	mov    0xffffffec(%ebp),%edx
 148:	89 45 f0             	mov    %eax,0xfffffff0(%ebp)
 14b:	39 d0                	cmp    %edx,%eax
 14d:	73 31                	jae    180 <extract_unsigned_integer+0x70>
 14f:	90                   	nop    
	retval = (retval << 8) | *p;
 150:	89 fb                	mov    %edi,%ebx
 152:	89 f1                	mov    %esi,%ecx
 154:	c1 e1 08             	shl    $0x8,%ecx
 157:	0f a4 f3 08          	shld   $0x8,%esi,%ebx
 15b:	31 d2                	xor    %edx,%edx
 15d:	8b 75 f0             	mov    0xfffffff0(%ebp),%esi
 160:	89 df                	mov    %ebx,%edi
 162:	09 d7                	or     %edx,%edi
 164:	0f b6 06             	movzbl (%esi),%eax
 167:	89 ce                	mov    %ecx,%esi
 169:	ff 45 f0             	incl   0xfffffff0(%ebp)
 16c:	09 c6                	or     %eax,%esi
 16e:	8b 45 ec             	mov    0xffffffec(%ebp),%eax
 171:	39 45 f0             	cmp    %eax,0xfffffff0(%ebp)
 174:	72 da                	jb     150 <extract_unsigned_integer+0x40>
 176:	8d 76 00             	lea    0x0(%esi),%esi
 179:	8d bc 27 00 00 00 00 	lea    0x0(%edi),%edi
    }
  else
    {
      for (p = endaddr - 1; p >= startaddr; --p)
	retval = (retval << 8) | *p;
    }
  return retval;
}
 180:	83 c4 1c             	add    $0x1c,%esp
 183:	89 f0                	mov    %esi,%eax
 185:	89 fa                	mov    %edi,%edx
 187:	5b                   	pop    %ebx
 188:	5e                   	pop    %esi
 189:	5f                   	pop    %edi
 18a:	5d                   	pop    %ebp
 18b:	c3                   	ret    
 18c:	8d 74 26 00          	lea    0x0(%esi),%esi
 190:	8b 55 ec             	mov    0xffffffec(%ebp),%edx
 193:	8b 45 08             	mov    0x8(%ebp),%eax
 196:	4a                   	dec    %edx
 197:	89 55 f0             	mov    %edx,0xfffffff0(%ebp)
 19a:	39 c2                	cmp    %eax,%edx
 19c:	72 e2                	jb     180 <extract_unsigned_integer+0x70>
 19e:	89 f6                	mov    %esi,%esi
 1a0:	89 fb                	mov    %edi,%ebx
 1a2:	89 f1                	mov    %esi,%ecx
 1a4:	c1 e1 08             	shl    $0x8,%ecx
 1a7:	0f a4 f3 08          	shld   $0x8,%esi,%ebx
 1ab:	31 d2                	xor    %edx,%edx
 1ad:	8b 75 f0             	mov    0xfffffff0(%ebp),%esi
 1b0:	89 df                	mov    %ebx,%edi
 1b2:	09 d7                	or     %edx,%edi
 1b4:	0f b6 06             	movzbl (%esi),%eax
 1b7:	89 ce                	mov    %ecx,%esi
 1b9:	ff 4d f0             	decl   0xfffffff0(%ebp)
 1bc:	09 c6                	or     %eax,%esi
 1be:	8b 45 08             	mov    0x8(%ebp),%eax
 1c1:	39 45 f0             	cmp    %eax,0xfffffff0(%ebp)
 1c4:	73 da                	jae    1a0 <extract_unsigned_integer+0x90>
 1c6:	eb b8                	jmp    180 <extract_unsigned_integer+0x70>
 1c8:	c7 04 24 00 00 00 00 	movl   $0x0,(%esp)
 1cf:	ba 08 00 00 00       	mov    $0x8,%edx
 1d4:	89 54 24 04          	mov    %edx,0x4(%esp)
 1d8:	e8 fc ff ff ff       	call   1d9 <extract_unsigned_integer+0xc9>
 1dd:	8d 76 00             	lea    0x0(%esi),%esi

Re: gdb code review, pointer madness

[NZG]

Looks like this particular flaw is an optimization trick, digging down farther 
the correct pointer does appear to get there.

Something is wrong with the frame call on the sential frame (because it 
crashes the debugger) but this is not it.

NZG.

On Tuesday 24 January 2006 11:18 am, NZG wrote:
> Intro:
> This is a continuation of a thread attempting to patch gdb 61 to make it
> work correctly with Eclipse's MI interface. The problem is that the Eclipse
> CDT queries the frame list before it begins execution, which triggers a bug
> in the m68k port of gdb and smashes it.
>
> > I don't think there's anything wrong with the C code.  That's a
> > completely fundamental function in GDB, used everywhere, all the time.
> >  And it's hard for me to believe that both endiannesses aren't being
> > exercised.  If it were broken, we surely would have found out by now.
> >
> > If the function is behaving as you say, then I'd say your compiler has
> > miscompiled GDB.  Why don't you look through the assembly code for the
> > function, and see if it's right?
>
> It appears the assembly code for this function is correct (pasted below)
> but I am no wizard at x68 assembly. If somebody else wants to take a gander
> it would be appreciated.
>
> I built it with
> CC=gcc-3.3 ../gdb-6.1/configure  --target=m68k-bdm-elf
> make CC=ggc-3.3
>
> after applying the bdm patch;
> http://www.uclinux.org/pub/uClinux/m68k-elf-tools/gcc-3/uclinux-tools-20040
>603/m68k-bdm-1.3.0.tar.bz2 and replacing 2 files as detailed in:
> http://mailman.uclinux.org/pipermail/uclinux-dev/2004-March/024885.html
>
> I think the most likely culprit is some form of stack corruption (such as
> an improperly dereferenced pointer) above it.
>
> The faulty call is being made directly after a call to
>  frame_unwind_register (next_frame, M68K_FP_REGNUM, buf);
> is made on the sentinal frame.
>
> I highly suspect something is not being initialized correctly in the m68k
> sentinal frame and that it's causing a dereference off into space from the
> linked list of frames.
> Tracking that down though.... has been a challenge.
>
> Call leading up to insane extract_unsigned_integer call
> *************************************************************
> static struct m68k_frame_cache *
> m68k_frame_cache (struct frame_info *next_frame, void **this_cache)
> {
>   struct m68k_frame_cache *cache;
>   char buf[4];
>   int i;
>
>   if (*this_cache)
>     return *this_cache;
>
>   cache = m68k_alloc_frame_cache ();
>   *this_cache = cache;
>
>   /* In principle, for normal frames, %fp holds the frame pointer,
>      which holds the base address for the current stack frame.
>      However, for functions that don't need it, the frame pointer is
>      optional.  For these "frameless" functions the frame pointer is
>      actually the frame pointer of the calling frame.  Signal
>      trampolines are just a special case of a "frameless" function.
>      They (usually) share their frame pointer with the frame that was
>      in progress when the signal occurred.  */
>
>   frame_unwind_register (next_frame, M68K_FP_REGNUM, buf);
>   cache->base = extract_unsigned_integer (buf, 4);
> (returns with some crazy random number obtained from shifting and adding 8
> buf elements)
>
>
> disassembly of extract_unsigned_integer
> *************************************************************
> 00000110 <extract_unsigned_integer>:
>
> ULONGEST
> extract_unsigned_integer (const void *addr, int len)
> {
>  110:	55                   	push   %ebp
>  111:	89 e5                	mov    %esp,%ebp
>  113:	57                   	push   %edi
>  114:	56                   	push   %esi
>  115:	53                   	push   %ebx
>  116:	83 ec 1c             	sub    $0x1c,%esp
>  119:	8b 45 0c             	mov    0xc(%ebp),%eax
>   ULONGEST retval;
>   const unsigned char *p;
>   const unsigned char *startaddr = addr;
>   const unsigned char *endaddr = startaddr + len;
>  11c:	8b 55 08             	mov    0x8(%ebp),%edx
>  11f:	01 c2                	add    %eax,%edx
>
>   if (len > (int) sizeof (ULONGEST))
>  121:	83 f8 08             	cmp    $0x8,%eax
>  124:	89 55 ec             	mov    %edx,0xffffffec(%ebp)
>  127:	0f 8f 9b 00 00 00    	jg     1c8 <extract_unsigned_integer+0xb8>
>     error ("\
> That operation is not available on integers of more than %d bytes.",
> 	   (int) sizeof (ULONGEST));
>
>   /* Start at the most significant end of the integer, and work towards
>      the least significant.  */
>   retval = 0;
>   if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
>  12d:	a1 00 00 00 00       	mov    0x0,%eax
>  132:	31 f6                	xor    %esi,%esi
>  134:	31 ff                	xor    %edi,%edi
>  136:	89 04 24             	mov    %eax,(%esp)
>  139:	e8 fc ff ff ff       	call   13a <extract_unsigned_integer+0x2a>
>  13e:	85 c0                	test   %eax,%eax
>  140:	75 4e                	jne    190 <extract_unsigned_integer+0x80>
>     {
>       for (p = startaddr; p < endaddr; ++p)
>  142:	8b 45 08             	mov    0x8(%ebp),%eax
>  145:	8b 55 ec             	mov    0xffffffec(%ebp),%edx
>  148:	89 45 f0             	mov    %eax,0xfffffff0(%ebp)
>  14b:	39 d0                	cmp    %edx,%eax
>  14d:	73 31                	jae    180 <extract_unsigned_integer+0x70>
>  14f:	90                   	nop
> 	retval = (retval << 8) | *p;
>  150:	89 fb                	mov    %edi,%ebx
>  152:	89 f1                	mov    %esi,%ecx
>  154:	c1 e1 08             	shl    $0x8,%ecx
>  157:	0f a4 f3 08          	shld   $0x8,%esi,%ebx
>  15b:	31 d2                	xor    %edx,%edx
>  15d:	8b 75 f0             	mov    0xfffffff0(%ebp),%esi
>  160:	89 df                	mov    %ebx,%edi
>  162:	09 d7                	or     %edx,%edi
>  164:	0f b6 06             	movzbl (%esi),%eax
>  167:	89 ce                	mov    %ecx,%esi
>  169:	ff 45 f0             	incl   0xfffffff0(%ebp)
>  16c:	09 c6                	or     %eax,%esi
>  16e:	8b 45 ec             	mov    0xffffffec(%ebp),%eax
>  171:	39 45 f0             	cmp    %eax,0xfffffff0(%ebp)
>  174:	72 da                	jb     150 <extract_unsigned_integer+0x40>
>  176:	8d 76 00             	lea    0x0(%esi),%esi
>  179:	8d bc 27 00 00 00 00 	lea    0x0(%edi),%edi
>     }
>   else
>     {
>       for (p = endaddr - 1; p >= startaddr; --p)
> 	retval = (retval << 8) | *p;
>     }
>   return retval;
> }
>  180:	83 c4 1c             	add    $0x1c,%esp
>  183:	89 f0                	mov    %esi,%eax
>  185:	89 fa                	mov    %edi,%edx
>  187:	5b                   	pop    %ebx
>  188:	5e                   	pop    %esi
>  189:	5f                   	pop    %edi
>  18a:	5d                   	pop    %ebp
>  18b:	c3                   	ret
>  18c:	8d 74 26 00          	lea    0x0(%esi),%esi
>  190:	8b 55 ec             	mov    0xffffffec(%ebp),%edx
>  193:	8b 45 08             	mov    0x8(%ebp),%eax
>  196:	4a                   	dec    %edx
>  197:	89 55 f0             	mov    %edx,0xfffffff0(%ebp)
>  19a:	39 c2                	cmp    %eax,%edx
>  19c:	72 e2                	jb     180 <extract_unsigned_integer+0x70>
>  19e:	89 f6                	mov    %esi,%esi
>  1a0:	89 fb                	mov    %edi,%ebx
>  1a2:	89 f1                	mov    %esi,%ecx
>  1a4:	c1 e1 08             	shl    $0x8,%ecx
>  1a7:	0f a4 f3 08          	shld   $0x8,%esi,%ebx
>  1ab:	31 d2                	xor    %edx,%edx
>  1ad:	8b 75 f0             	mov    0xfffffff0(%ebp),%esi
>  1b0:	89 df                	mov    %ebx,%edi
>  1b2:	09 d7                	or     %edx,%edi
>  1b4:	0f b6 06             	movzbl (%esi),%eax
>  1b7:	89 ce                	mov    %ecx,%esi
>  1b9:	ff 4d f0             	decl   0xfffffff0(%ebp)
>  1bc:	09 c6                	or     %eax,%esi
>  1be:	8b 45 08             	mov    0x8(%ebp),%eax
>  1c1:	39 45 f0             	cmp    %eax,0xfffffff0(%ebp)
>  1c4:	73 da                	jae    1a0 <extract_unsigned_integer+0x90>
>  1c6:	eb b8                	jmp    180 <extract_unsigned_integer+0x70>
>  1c8:	c7 04 24 00 00 00 00 	movl   $0x0,(%esp)
>  1cf:	ba 08 00 00 00       	mov    $0x8,%edx
>  1d4:	89 54 24 04          	mov    %edx,0x4(%esp)
>  1d8:	e8 fc ff ff ff       	call   1d9 <extract_unsigned_integer+0xc9>
>  1dd:	8d 76 00             	lea    0x0(%esi),%esi

Re: gdb code review, pointer madness

[Jim Blandy]

On 1/24/06, NZG <ngustavson@emacinc.com> wrote:
> Looks like this particular flaw is an optimization trick, digging down farther
> the correct pointer does appear to get there.
>
> Something is wrong with the frame call on the sential frame (because it
> crashes the debugger) but this is not it.

Well, if you are able to come up with some reproduction instructions
that don't require exotic hardware, let us know.

The sentinel frame should be very simple to deal with: "unwinding" a
register from the sentinel frame simply returns its current value from
the chip.  If you're not getting the current registers' values unwound
from the sentinel frame, then GDB simply isn't getting the register
values right; you'll need to look at the communication between GDB and
the board.

Re: gdb code review, pointer madness

[NZG]

I think I'm getting ahead of myself and assuming too much.
I've bitten off a lot with this project and have a lot to learn yet.

First off, I think I'm getting confused between the innermost frame and the 
sentinal frame, which I was originally assuming were one and the same.

Digging deeper, it appears that the "innermost frame" as referred to in the 
comments is actually referring to frame 0, while the sentinal is frame -1.

Since my board is succesfully connecting and displaying the current frame, it 
would seem that the sentinal fetch is working correctly.

The problem is that when I run a backtrace it fetches the innermost frame, and 
then starts fetching garbage infinitely (get_prev_frame never returns NULL).

I'm trying to wade through the code to figure out why this is happening, but 
it's taking a while, function calls here are very, very deep.

In general, should this information (get_prev_frame=NULL) be coming from the 
target? Or should the host know this based on the id number?

If I'm asking the wrong list please point me the right direction, this seemed 
the closest to a developer list out of the choices.

thx,
NZG






On Tuesday 24 January 2006 3:27 pm, Jim Blandy wrote:
> On 1/24/06, NZG <ngustavson@emacinc.com> wrote:
> > Looks like this particular flaw is an optimization trick, digging down
> > farther the correct pointer does appear to get there.
> >
> > Something is wrong with the frame call on the sential frame (because it
> > crashes the debugger) but this is not it.
>
> Well, if you are able to come up with some reproduction instructions
> that don't require exotic hardware, let us know.
>
> The sentinel frame should be very simple to deal with: "unwinding" a
> register from the sentinel frame simply returns its current value from
> the chip.  If you're not getting the current registers' values unwound
> from the sentinel frame, then GDB simply isn't getting the register
> values right; you'll need to look at the communication between GDB and
> the board.

Re: gdb code review, pointer madness

[Daniel Jacobowitz]

On Tue, Jan 24, 2006 at 03:57:10PM -0600, NZG wrote:
> The problem is that when I run a backtrace it fetches the innermost frame, and 
> then starts fetching garbage infinitely (get_prev_frame never returns NULL).
> 
> I'm trying to wade through the code to figure out why this is happening, but 
> it's taking a while, function calls here are very, very deep.
> 
> In general, should this information (get_prev_frame=NULL) be coming from the 
> target? Or should the host know this based on the id number?

Normally this means that the unwinder in the tdep file is failing to
stop in some case where it ought to.

When you first connect to a target, it is often in an ambiguous /
uninitialized state.  It's pretty difficult to prevent GDB from reading
a certain amount of random memory if you ask for a backtrace when there
isn't a valid stack.

-- 
Daniel Jacobowitz
CodeSourcery

Re: gdb code review, pointer madness

[Jim Blandy]

On 1/24/06, NZG <ngustavson@emacinc.com> wrote:
> If I'm asking the wrong list please point me the right direction, this seemed
> the closest to a developer list out of the choices.

I think this is the right place.

> I think I'm getting ahead of myself and assuming too much.
> I've bitten off a lot with this project and have a lot to learn yet.

Hah!  You have only just begun to suffer!  :)

> First off, I think I'm getting confused between the innermost frame and the
> sentinal frame, which I was originally assuming were one and the same.

GDB's current frame model is the result of an incremental cleanup of
working code, not a fresh design, so it's a little weird.

The natural model would be to have a frame object, with methods that
read and write that frame's registers.  Reading and writing the
youngest frame's registers would simply read and write the processor's
current registers.  Older frames may have some registers saved on the
stack by younger frames, so accessing the older frames' registers
would do a mix of memory accesses and register accesses, as
appropriate.

GDB's model is that you "unwind" a frame's registers from the next
younger frame.  That is, to access the registers of frame #1 (the
next-to-youngest frame), you actually apply frame_unwind_register to
frame #0 (the youngest frame).  Okay, whatever.  But then the obvious
question is: how do you access the registers of the youngest frame
itself?  How do you 'unwind' them when they're not wound up?

Thus the sentinel frame, the "-1st" frame.

If I remember right, the 'where' command should be printing out the
frames as it discovers them.  So if you're getting a lot of memory
traffic and no actual frames being listed, you're probably getting
stuck unwinding a single frame, as Daniel says.

Re: gdb code review, pointer madness

[Eli Zaretskii]

> Date: Tue, 24 Jan 2006 16:00:55 -0800
> From: Jim Blandy <jimb@red-bean.com>
> Cc: gdb-patches@sourceware.org
> 
> GDB's current frame model is the result of an incremental cleanup of
> working code, not a fresh design, so it's a little weird.
> 
> The natural model would be to have a frame object, with methods that
> read and write that frame's registers.  Reading and writing the
> youngest frame's registers would simply read and write the processor's
> current registers.  Older frames may have some registers saved on the
> stack by younger frames, so accessing the older frames' registers
> would do a mix of memory accesses and register accesses, as
> appropriate.
> 
> GDB's model is that you "unwind" a frame's registers from the next
> younger frame.  That is, to access the registers of frame #1 (the
> next-to-youngest frame), you actually apply frame_unwind_register to
> frame #0 (the youngest frame).  Okay, whatever.  But then the obvious
> question is: how do you access the registers of the youngest frame
> itself?  How do you 'unwind' them when they're not wound up?
> 
> Thus the sentinel frame, the "-1st" frame.

Ah! wouldn't it be swell if this description finds its way to
gdbint.texinfo?

Re: gdb code review, pointer madness

[Jim Blandy]

On 1/24/06, Eli Zaretskii <eliz@gnu.org> wrote:
> Ah! wouldn't it be swell if this description finds its way to
> gdbint.texinfo?

You watch for these things, don't you?

Where should it go?

Re: gdb code review, pointer madness

[Jim Blandy]

On 1/24/06, Jim Blandy <jimb@red-bean.com> wrote:
> On 1/24/06, Eli Zaretskii <eliz@gnu.org> wrote:
> > Ah! wouldn't it be swell if this description finds its way to
> > gdbint.texinfo?
>
> You watch for these things, don't you?
>
> Where should it go?

I'm being lazy.  How's this?

gdb/doc/ChangeLog:
2006-01-24  Jim Blandy  <jimb@redhat.com>

	* gdbint.texinfo (Frames): Document the basics of GDB's register
	unwinding model, and explain the existence of the "sentinel" frame.

Index: gdb/doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.235
diff -c -p -r1.235 gdbint.texinfo
*** gdb/doc/gdbint.texinfo	20 Jan 2006 21:58:16 -0000	1.235
--- gdb/doc/gdbint.texinfo	25 Jan 2006 05:25:04 -0000
*************** they use.
*** 231,236 ****
--- 231,261 ----
  A frame is a construct that @value{GDBN} uses to keep track of calling
  and called functions.

+ @value{GDBN}'s current frame model is the result of an incremental
+ cleanup of working code, not a fresh design, so it's a little weird.
+
+ The natural model would have a frame object, with methods that read
+ and write that frame's registers.  Reading or writing the youngest
+ frame's registers would simply read or write the processor's current
+ registers, since the youngest frame is running directly on the
+ processor.  Older frames might have some registers saved on the stack
+ by younger frames, so accessing the older frames' registers would do a
+ mix of memory accesses and register accesses, as appropriate.
+
+ Instead, @value{GDBN}'s model is that you find a frame's registers by
+ ``unwinding'' them from the next younger frame.  That is, to access
+ the registers of frame #1 (the next-to-youngest frame), you actually
+ apply @code{frame_register_unwind} to frame #0 (the youngest frame).
+ But then the obvious question is: how do you access the registers of
+ the youngest frame itself?  How do you ``unwind'' them when they're
+ not wound up?
+
+ To answer this question, GDB has the @dfn{sentinel} frame, the
+ ``-1st'' frame.  Unwinding registers from the sentinel frame gives you
+ the current values of the youngest real frame's registers.  If @var{f}
+ is a sentinel frame, then @code{get_frame_type (@var{f}) ==
+ SENTINEL_FRAME}.
+
  @findex create_new_frame
  @vindex FRAME_FP
  @code{FRAME_FP} in the machine description has no meaning to the

Re: gdb code review, pointer madness

[Eli Zaretskii]

> Date: Tue, 24 Jan 2006 20:59:45 -0800
> From: Jim Blandy <jimb@red-bean.com>
> Cc: ngustavson@emacinc.com, gdb-patches@sourceware.org
> 
> On 1/24/06, Eli Zaretskii <eliz@gnu.org> wrote:
> > Ah! wouldn't it be swell if this description finds its way to
> > gdbint.texinfo?
> 
> You watch for these things, don't you?

Of course! why else should I read this list? ;-)

Re: gdb code review, pointer madness

[Eli Zaretskii]

> Date: Tue, 24 Jan 2006 21:25:44 -0800
> From: Jim Blandy <jimb@red-bean.com>
> Cc: ngustavson@emacinc.com, gdb-patches@sourceware.org
> 
> I'm being lazy.  How's this?

Almost perfect.  If you add a "@cindex sentinel frame" and a
"@findex frame_register_unwind", it will be _just_ perfect.

Actually, maybe also "@cindex frame, unwind".

Thanks!

Re: gdb code review, pointer madness

[Jim Blandy]

On 1/25/06, Eli Zaretskii <eliz@gnu.org> wrote:
> > Date: Tue, 24 Jan 2006 21:25:44 -0800
> > From: Jim Blandy <jimb@red-bean.com>
> > Cc: ngustavson@emacinc.com, gdb-patches@sourceware.org
> >
> > I'm being lazy.  How's this?
>
> Almost perfect.  If you add a "@cindex sentinel frame" and a
> "@findex frame_register_unwind", it will be _just_ perfect.
>
> Actually, maybe also "@cindex frame, unwind".

I've committed the below, adding @vindex SENTINEL_FRAME:

gdb/doc/ChangeLog:
2006-01-24  Jim Blandy  <jimb@redhat.com>

	* gdbint.texinfo (Frames): Document the basics of GDB's register
	unwinding model, and explain the existence of the "sentinel" frame.

Index: gdb/doc/gdbint.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdbint.texinfo,v
retrieving revision 1.235
diff -c -p -r1.235 gdbint.texinfo
*** gdb/doc/gdbint.texinfo	20 Jan 2006 21:58:16 -0000	1.235
--- gdb/doc/gdbint.texinfo	25 Jan 2006 18:45:33 -0000
*************** they use.
*** 231,236 ****
--- 231,266 ----
  A frame is a construct that @value{GDBN} uses to keep track of calling
  and called functions.

+ @cindex frame, unwind
+ @value{GDBN}'s current frame model is the result of an incremental
+ cleanup of working code, not a fresh design, so it's a little weird.
+
+ The natural model would have a frame object, with methods that read
+ and write that frame's registers.  Reading or writing the youngest
+ frame's registers would simply read or write the processor's current
+ registers, since the youngest frame is running directly on the
+ processor.  Older frames might have some registers saved on the stack
+ by younger frames, so accessing the older frames' registers would do a
+ mix of memory accesses and register accesses, as appropriate.
+
+ @findex frame_register_unwind
+ Instead, @value{GDBN}'s model is that you find a frame's registers by
+ ``unwinding'' them from the next younger frame.  That is, to access
+ the registers of frame #1 (the next-to-youngest frame), you actually
+ apply @code{frame_register_unwind} to frame #0 (the youngest frame).
+ But then the obvious question is: how do you access the registers of
+ the youngest frame itself?  How do you ``unwind'' them when they're
+ not wound up?
+
+ @cindex sentinel frame
+ @findex get_frame_type
+ @vindex SENTINEL_FRAME
+ To answer this question, GDB has the @dfn{sentinel} frame, the
+ ``-1st'' frame.  Unwinding registers from the sentinel frame gives you
+ the current values of the youngest real frame's registers.  If @var{f}
+ is a sentinel frame, then @code{get_frame_type (@var{f}) ==
+ SENTINEL_FRAME}.
+
  @findex create_new_frame
  @vindex FRAME_FP
  @code{FRAME_FP} in the machine description has no meaning to the

Re: gdb code review, pointer madness

[Eli Zaretskii]

> Date: Wed, 25 Jan 2006 10:49:25 -0800
> From: Jim Blandy <jimb@red-bean.com>
> Cc: ngustavson@emacinc.com, gdb-patches@sourceware.org
> 
> On 1/25/06, Eli Zaretskii <eliz@gnu.org> wrote:
> > > Date: Tue, 24 Jan 2006 21:25:44 -0800
> > > From: Jim Blandy <jimb@red-bean.com>
> > > Cc: ngustavson@emacinc.com, gdb-patches@sourceware.org
> > >
> > > I'm being lazy.  How's this?
> >
> > Almost perfect.  If you add a "@cindex sentinel frame" and a
> > "@findex frame_register_unwind", it will be _just_ perfect.
> >
> > Actually, maybe also "@cindex frame, unwind".
> 
> I've committed the below, adding @vindex SENTINEL_FRAME:

Thanks.

Re: gdb code review, pointer madness

[NZG]

So, if frame 0 is actually the lowest level frame, 
would it be correct to say that 

for frame->level=0
frame->this_id->value->stack_addr =0

for all time?
Or is this an overgeneralization.

This appears to have to be true since in get_prev_frame

  if (this_frame->level >= 0 && !frame_id_p (get_frame_id (this_frame)))

appears to be the only conditional preventing an unwind at level=0 if no 
legacy_get_prev_frame is used.


This doesn't seem to be in keeping with the description of the ID
>   A frame ID provides an invariant that can be used to re-identify an
>   instance of a frame.  It is a combination of the frame's `base' and
>   the frame's function's code address.

but I'm not seeing anything else in get_prev_frame that would prevent an 
unwind.

You shouldn't be able to unwind frame->level=0 should you?

thanks for all your help,
NZG

Re: gdb code review, pointer madness

[Daniel Jacobowitz]

On Thu, Jan 26, 2006 at 10:17:41AM -0600, NZG wrote:
> So, if frame 0 is actually the lowest level frame, 
> would it be correct to say that 
> 
> for frame->level=0
> frame->this_id->value->stack_addr =0
> 
> for all time?
> Or is this an overgeneralization.

You should try GDB on some other target to see how this works. 
Frame 0 is the innermost (i.e. current) frame.  It unwinds to frame #1.

-- 
Daniel Jacobowitz
CodeSourcery

frame theory, was pointer madness

[NZG]

Lets see if I have this right.
Assuming a linked list with 5 frames, they should look like this

level	description
-1		sentinal frame(virtual)
0		youngest frame (the deepest function call and current frame)
1		older
2		even older
3		oldest

And the list should look like this

prev->frame->next

NULL->3->2->1->0->-1->-1->-1........

I think, and I have yet to successfully verify this, that the highest level 
frame should have an id of zero, or at least should.

Normal gdb appears to work by caching the frame of the highest level to a NULL 
prev value, which appears to be happening somewhere when the frame first 
enters existance. In the case of a gdbremote connection this should be on 
connection to the remote server. When frame 0 is the highest and lowest real 
level it will "unwind" frame zero  by accessing -1. which should in turn 
actually read the registers from remote device.  Then ->black magic - black 
magic -> gdb realizes there is no higher level frame and caches a NULL there.

Now the magic.

NZG

Re: frame theory, was pointer madness

[Daniel Jacobowitz]

On Thu, Jan 26, 2006 at 01:54:12PM -0600, NZG wrote:
> Lets see if I have this right.
> Assuming a linked list with 5 frames, they should look like this
> 
> level	description
> -1		sentinal frame(virtual)
> 0		youngest frame (the deepest function call and current frame)
> 1		older
> 2		even older
> 3		oldest
> 
> And the list should look like this
> 
> prev->frame->next
> 
> NULL->3->2->1->0->-1->-1->-1........
> 
> I think, and I have yet to successfully verify this, that the highest level 
> frame should have an id of zero, or at least should.

It's important to use consistent terminology when talking about this.
What do you mean by "highest"?

The innermost frame is always #0; it is the first frame "unwound"
from the sentinel frame.  Then the next frame is unwound from frame 0.

The outermost frame should have an invalid frame ID.

> Normal gdb appears to work by caching the frame of the highest level to a NULL 
> prev value, which appears to be happening somewhere when the frame first 
> enters existance. In the case of a gdbremote connection this should be on 
> connection to the remote server.

No, not until someone types "backtrace".

-- 
Daniel Jacobowitz
CodeSourcery

Re: frame theory, was pointer madness

[NZG]

On Thursday 26 January 2006 1:59 pm, Daniel Jacobowitz wrote:
> It's important to use consistent terminology when talking about this.
> What do you mean by "highest"?
Largest integer number in the frames "level" element.

> It's important to use consistent terminology
I agree, but it's been somewhat difficult to unravel the terminology so far.
There doesn't really seem to be any master document defining it.
I'm an EE, so if this is standard CS jargon please forgive me.

inner refers to ->next
and outer refers to prev<-

The farther in, the younger your function call.

Is this right?;

thx,
NZG.

Re: frame theory, was pointer madness

[Daniel Jacobowitz]

On Thu, Jan 26, 2006 at 02:15:41PM -0600, NZG wrote:
> On Thursday 26 January 2006 1:59 pm, Daniel Jacobowitz wrote:
> > It's important to use consistent terminology when talking about this.
> > What do you mean by "highest"?
> Largest integer number in the frames "level" element.
> 
> > It's important to use consistent terminology
> I agree, but it's been somewhat difficult to unravel the terminology so far.
> There doesn't really seem to be any master document defining it.
> I'm an EE, so if this is standard CS jargon please forgive me.
> 
> inner refers to ->next
> and outer refers to prev<-
> 
> The farther in, the younger your function call.
> 
> Is this right?;

Yes, that's how we generally use the terms in GDB.

-- 
Daniel Jacobowitz
CodeSourcery

Re: frame theory, was pointer madness

[Mark Kettenis]

> From: NZG <ngustavson@emacinc.com>
> Date: Thu, 26 Jan 2006 13:54:12 -0600
> 
> Lets see if I have this right.
> Assuming a linked list with 5 frames, they should look like this
> 
> level	description
> -1		sentinal frame(virtual)
> 0		youngest frame (the deepest function call and current frame)
> 1		older
> 2		even older
> 3		oldest
> 
> And the list should look like this
> 
> prev->frame->next
> 
> NULL->3->2->1->0->-1->-1->-1........

I get the feeling that you've got a wrong picture.  I'd view things
as:

-1 -> 0 -> 1 -> 2 -> 3 -> NULL

> I think, and I have yet to successfully verify this, that the highest level 
> frame should have an id of zero, or at least should.
> 
> Normal gdb appears to work by caching the frame of the highest level
> to a NULL prev value, which appears to be happening somewhere when
> the frame first enters existance. In the case of a gdbremote
> connection this should be on connection to the remote server. When
> frame 0 is the highest and lowest real level it will "unwind" frame
> zero by accessing -1. which should in turn actually read the
> registers from remote device.  Then ->black magic - black magic ->
> gdb realizes there is no higher level frame and caches a NULL there.

Sorry but what you say here doesn't make any sense to me.  GDB unwinds
the stack from the registers values that are currently found in the
cpu.  Unwinding involves interpreting debug info or analyzing code to
determine where the register values for the previous frames are
stored.  This process can terminate (i.e. if we hit main, or if the
value for the frame pointer register for a frame is zero[1]) but isn't
guaranteed to.  Actually the unwinding process is difficult to get
completely right, and might send us off into the wrong direction,
resulting in rubish frames.

I'm not sure what your problem is, only that GDB seems to work pretty
well on OpenBSD/mac68k, so I doubt there is a fundamental problem in
the generic m68k unwinder code.

Mark

[1] If the ISA/ABI has a "hard" frame pointer register

Re: frame theory, was pointer madness

[NZG]

> > prev->frame->next
> >
> > NULL->3->2->1->0->-1->-1->-1........
>
> I get the feeling that you've got a wrong picture.  I'd view things
> as:
>
> -1 -> 0 -> 1 -> 2 -> 3 -> NULL
Is this with respect the to legend above(prev->frame->next)?
Bacause that would imply that the ->next frame after 0 is 1, but it's not, 
it's -1 correct?

prev decrements the level next increments it.

It's far easier for me to imply next with ->
and previous with <-
This seems more natural. 

> Sorry but what you say here doesn't make any sense to me.  GDB unwinds
> the stack from the registers values that are currently found in the
> cpu.
And from cached values of it's read registers. In my case I suspect the 
problem is that the regcache is being shown as initialized when it in fact is 
not. It does not read the registers on every access.

> Unwinding involves interpreting debug info or analyzing code to 
> determine where the register values for the previous frames are
> stored.  This process can terminate (i.e. if we hit main, or if the
> value for the frame pointer register for a frame is zero[1]) but isn't
> guaranteed to.  Actually the unwinding process is difficult to get
> completely right, and might send us off into the wrong direction,
> resulting in rubish frames.
In my case it goes into an infinite loop and continually prints rubbish frames 
to the screen.

> I'm not sure what your problem is, only that GDB seems to work pretty
> well on OpenBSD/mac68k, so I doubt there is a fundamental problem in
> the generic m68k unwinder code.
Well that depends on what you mean by "generic".
This is being applied to remotely debug a Coldfire core which I suspect has 
some differences from your machine.
If it only works on your machine and not mine, then it has failed to be 
"generic".

Also note that this is a very specific crash that only happens when you try to 
unwind the frames after connecting to a remote target before beginning 
execution. Have you attempted to do that?

NZG

Re: frame theory, was pointer madness

[Mark Kettenis]

> From: NZG <ngustavson@emacinc.com>
> Date: Thu, 26 Jan 2006 15:52:46 -0600
> 
> > > prev->frame->next
> > >
> > > NULL->3->2->1->0->-1->-1->-1........
> >
> > I get the feeling that you've got a wrong picture.  I'd view things
> > as:
> >
> > -1 -> 0 -> 1 -> 2 -> 3 -> NULL

> Is this with respect the to legend above(prev->frame->next)?

No it isn't.

> Bacause that would imply that the ->next frame after 0 is 1, but it's not, 
> it's -1 correct?
>
> prev decrements the level next increments it.

Sure, but that's a pretty meaningless way of thinking about frames.
You can only unwind (i.e. go in the prev direction).

> > Sorry but what you say here doesn't make any sense to me.  GDB unwinds
> > the stack from the registers values that are currently found in the
> > cpu.
> And from cached values of it's read registers. In my case I suspect the 
> problem is that the regcache is being shown as initialized when it in fact is 
> not. It does not read the registers on every access.

Sure but that's irrelevant for understanding how GDB uses frames.
(It may be relevant to bugs in your configuration though).

> > Unwinding involves interpreting debug info or analyzing code to 
> > determine where the register values for the previous frames are
> > stored.  This process can terminate (i.e. if we hit main, or if the
> > value for the frame pointer register for a frame is zero[1]) but isn't
> > guaranteed to.  Actually the unwinding process is difficult to get
> > completely right, and might send us off into the wrong direction,
> > resulting in rubish frames.

> In my case it goes into an infinite loop and continually prints
> rubbish frames to the screen.

As long as the frame number for each frame is different, that's not
totally unexpected.  If you're stack is thrashed (or if you don't have
a valid stack at all), you can't expect gdb to produce a valid
backtrace.

> > I'm not sure what your problem is, only that GDB seems to work pretty
> > well on OpenBSD/mac68k, so I doubt there is a fundamental problem in
> > the generic m68k unwinder code.
> Well that depends on what you mean by "generic".

Heh.  Well, I guess the code basically assumes it's using an ancient
Unix on an origional m68k cpu.

> This is being applied to remotely debug a Coldfire core which I suspect has 
> some differences from your machine.

Dunno, but if the Coldfire still has the same basic instruction set as
the origional m68k and uses the same registers as %fp and %sp, the
unwind code should be ok.

> If it only works on your machine and not mine, then it has failed to be 
> "generic".
> 
> Also note that this is a very specific crash that only happens when
> you try to unwind the frames after connecting to a remote target
> before beginning execution. Have you attempted to do that?

I'm talking about a native OpenBSD/m68k GDB, so there os no remote
target to connect to.

To me, it looks like you're connecting to a buggy stub.

Mark

Re: frame theory, was pointer madness

[Daniel Jacobowitz]

On Thu, Jan 26, 2006 at 11:40:07PM +0100, Mark Kettenis wrote:
> To me, it looks like you're connecting to a buggy stub.

He's connecting to basically a standard gdbserver, poised at
the first instruction of the program.  Memory has garbage
and/or is invalid - no MMU so reading from garbage memory
is a bit more serious than is typical for GDB.

The best thing here would be, if the stub can find out from
the kernel what constitutes "valid" RAM, to refuse reads to
it.  Then ignore the ugliness when you type backtrace and
don't have a stack yet - it's not real surprising that doesn't
work!

-- 
Daniel Jacobowitz
CodeSourcery

Re: frame theory, was pointer madness

[Jim Blandy]

I've read the conversation with Daniel, but I'm going to pick at your
use of terminology even more carefully, because I think that's
actually a central source of confusion.

As you've worked out, "the next frame" in GDB-speak means "the next
younger frame".  The "next frame" of some frame F is the frame for the
function F called.  There is no standard terminology for this that I
know of, which is one source of the confusion.  It's worth pointing
out that 1) I got the terminology backwards the first time I typed
this paragraph, and 2) Daniel got it backwards too when he said:

>The innermost frame is always #0; it is the first frame "unwound"
>from the sentinel frame.  Then the next frame is unwound from frame 0.

He should have said "the PREVIOUS frame is unwound from frame 0".

The frame chain in GDB is a doubly-linked list.  Every frame points to
the next and previous frame.  When you write things like this:

> prev->frame->next

that's not meaningful, because it isn't clear whether you're talking
about the chain as defined by the "next" pointers, or by the "prev"
pointers.  Don't talk about what points to what; use the terms "next"
and "previous".  (And check what you type.  :) )

> level   description
> -1              sentinal frame(virtual)
> 0               youngest frame (the deepest function call and current frame)
> 1               older
> 2               even older
> 3               oldest
>
> And the list should look like this
>
> prev->frame->next
>
> NULL->3->2->1->0->-1->-1->-1........
>
> I think, and I have yet to successfully verify this, that the highest level
> frame should have an id of zero, or at least should.

Don't confuse "level" with "id".  The level is generally what the user
uses to identify frames when using the "frame" command; it's what
follows the # mark in "where" output.  The frame ID, on the other
hand, is a pair (or on the IA-64, a triplet) of addresses that is
meant to identify a frame throughout its lifetime.  They're usually a
code address and a stack address.

Every time the inferior runs, even for a single-step, GDB throws away
all its frame structures, and reconstructs them when the inferior
stops next.  It constructs its own frame_info structures starting with
the *youngest* frame, at the top of the stack, and works its way
towards the bottom of the stack.  GDB constructs frame_info structures
lazily; it doesn't bother to figure them out until you actually refer
to them (via "up" or "where").  So a NULL "prev" link in a frame_info
structure does not mean that the stack has ended; it simply means that
GDB hasn't produced the next frame yet.  What *does* indicate that the
stack has ended is if frame_unwind_id returns the null_frame_id.  Note
that, since 'struct frame_id' is a structure, it doesn't make sense to
talk about a "frame id of zero".  It's not a number.

It's very good that GDB reconstructs the frame chain from scratch
every time the inferior stops.  This avoids bugs caused by out-of-date
information.  But it does mean that 'struct frame' objects are useless
for referring to a particular frame while the program runs.  For
example, when we 'next' over a function call, we single-step into the
call, and then set a breakpoint at the return address and let the
program continue until we hit it.  However, if there's recursion going
on, we might hit that breakpoint when we return to some recursive
invocation of the calling function --- but we're not through with the
call the user asked us to 'next' over.  So in addition to setting the
breakpoint at the return address, we need to remember which *frame*
we're looking for a return to.  Struct frame objects don't work here;
we throw them away while the inferior runs.

A frame ID is supposed to be a robust, lightweight way to refer to a
particular frame.  It's usually the address of the base of the frame
(often, the value the SP had when we entered the function), and the
address of the function itself.  When we produce frame_info
structures, we record a frame ID for each one.  Then, commands like
"next" compare frame ID's to see whether they've found the frame they
need.

These comparisons can be fooled, of course.  If you record a frame's
ID, and then the frame gets popped but the caller calls the same
function again, you're liable to get a new frame with the same ID as
the old frame.  It's very difficult to avoid this, and GDB doesn't
bother.

So frame levels (user interface), frame id's (long-term frame
identification), and frame_info structures (detailed but transient
records) are all distinct things to keep track of.

> Normal gdb appears to work by caching the frame of the highest level to a NULL
> prev value,

I'm not sure what this means.  GDB starts with the youngest (current)
frame, and then works towards the older frames, producing caller from
callee, until (if it's lucky) it reaches the oldest frame on the
stack.

> When frame 0 is the highest and lowest real
> level it will "unwind" frame zero  by accessing -1. which should in turn
> actually read the registers from remote device.  Then ->black magic - black
> magic -> gdb realizes there is no higher level frame and caches a NULL there.

You'll see GDB calling the "frame_unwind_register" functions all the
time, not just the first time it constructs the frame_info object.

A NULL "prev" pointer in a frame_info structure simply indicates that
GDB hasn't (yet) produced any older frames.  This could be because
there are no older frames, or it could simply be that GDB hasn't
bothered to find them yet.

There isn't any master document defining all this, but I'll see if I
can pirate some of what's been written into this thread for
gdbint.texinfo.

remote connection crash, was frame theory

[NZG]

On Thursday 26 January 2006 4:44 pm, Daniel Jacobowitz wrote:
> On Thu, Jan 26, 2006 at 11:40:07PM +0100, Mark Kettenis wrote:
> > To me, it looks like you're connecting to a buggy stub.
>
> He's connecting to basically a standard gdbserver, poised at
> the first instruction of the program.  Memory has garbage
> and/or is invalid - no MMU so reading from garbage memory
> is a bit more serious than is typical for GDB.
righto, it crashes the remote kernel and sends my host into an infinite loop 
in gdb. 

> The best thing here would be, if the stub can find out from
> the kernel what constitutes "valid" RAM, to refuse reads to
> it.  Then ignore the ugliness when you type backtrace and
> don't have a stack yet - it's not real surprising that doesn't
> work!
It needs to do something predictable that doesn't destroy the system, that's 
my only criteria.
Understand that It's not just for my personal vanity that this needs to work. 
I am trying to be compatible with the standard Eclipse CDT.
On of the first things the Eclipse CDT does upon connecting to a gdbserver is 
to use gdb's MI interface to pull down all the current frames. (which has the 
same effect as backtrace since it parses through the linked list of frames)

This can't be that outlandish a thing to do, because it works just fine on x86 
systems.

Yes, I could modify the Eclipse CDT not to do this, but I think the proper 
thing to fix is gdb, since it's desired behavior cannot possibly be a kernel 
oops.

Stepping through the code I can see that remote_fetch_registers is never 
called before this crash happens, all the data being used in the backtrace 
appears to be transferred in the initial connection and cached.

It's this cached data that doesn't create a valid enviornment for a backtrace.
If I can learn a little more about it I may wind up altering the gdbserver 
program to connect differently, or modifying gdb to parse it differently, 
whatever makes more sense. 

In any case, since this error occurs based on data transferred solely on the 
special case of the initial connection, I suspect it is fixable.

NZG.

 

Re: remote connection crash, was frame theory

[Daniel Jacobowitz]

On Thu, Jan 26, 2006 at 05:26:10PM -0600, NZG wrote:
> > The best thing here would be, if the stub can find out from
> > the kernel what constitutes "valid" RAM, to refuse reads to
> > it.  Then ignore the ugliness when you type backtrace and
> > don't have a stack yet - it's not real surprising that doesn't
> > work!

Can you do this?  It is the most reliable way to prevent the oops and
will continue to do so in other circumstances, e.g. stupid users and
bad pointers.

> Stepping through the code I can see that remote_fetch_registers is never 
> called before this crash happens, all the data being used in the backtrace 
> appears to be transferred in the initial connection and cached.

Did you look at the remote protocol output (set debug remote 1)? 
Several registers are usually transfered in the initial connection,
e.g. sp and pc.  It's just an optimization.

-- 
Daniel Jacobowitz
CodeSourcery

Re: frame theory, was pointer madness

[Jim Blandy]

On 1/26/06, Jim Blandy <jimb@red-bean.com> wrote:
> this paragraph, and 2) Daniel got it backwards too when he said:
>
> >The innermost frame is always #0; it is the first frame "unwound"
> >from the sentinel frame.  Then the next frame is unwound from frame 0.
>
> He should have said "the PREVIOUS frame is unwound from frame 0".

Or, as Jason has pointed out to me, probably Daniel meant "the next
frame" in the sense of "the next frame to be constructed", which is
perfectly correct.  But I do think it's ambiguous.

Re: remote connection crash, was frame theory

[Jim Blandy]

On 1/26/06, NZG <ngustavson@emacinc.com> wrote:
> On Thursday 26 January 2006 4:44 pm, Daniel Jacobowitz wrote:
> > On Thu, Jan 26, 2006 at 11:40:07PM +0100, Mark Kettenis wrote:
> > > To me, it looks like you're connecting to a buggy stub.
> >
> > He's connecting to basically a standard gdbserver, poised at
> > the first instruction of the program.  Memory has garbage
> > and/or is invalid - no MMU so reading from garbage memory
> > is a bit more serious than is typical for GDB.
> righto, it crashes the remote kernel and sends my host into an infinite loop
> in gdb.
>
> > The best thing here would be, if the stub can find out from
> > the kernel what constitutes "valid" RAM, to refuse reads to
> > it.  Then ignore the ugliness when you type backtrace and
> > don't have a stack yet - it's not real surprising that doesn't
> > work!

The PC is valid at this point, right?  If there were clean Dwarf CFI
for the entry point, marking it as the oldest frame, would that calm
GDB down?  That could be another approach.

Re: remote connection crash, was frame theory

[Daniel Jacobowitz]

On Thu, Jan 26, 2006 at 03:42:02PM -0800, Jim Blandy wrote:
> On 1/26/06, NZG <ngustavson@emacinc.com> wrote:
> > On Thursday 26 January 2006 4:44 pm, Daniel Jacobowitz wrote:
> > > On Thu, Jan 26, 2006 at 11:40:07PM +0100, Mark Kettenis wrote:
> > > > To me, it looks like you're connecting to a buggy stub.
> > >
> > > He's connecting to basically a standard gdbserver, poised at
> > > the first instruction of the program.  Memory has garbage
> > > and/or is invalid - no MMU so reading from garbage memory
> > > is a bit more serious than is typical for GDB.
> > righto, it crashes the remote kernel and sends my host into an infinite loop
> > in gdb.
> >
> > > The best thing here would be, if the stub can find out from
> > > the kernel what constitutes "valid" RAM, to refuse reads to
> > > it.  Then ignore the ugliness when you type backtrace and
> > > don't have a stack yet - it's not real surprising that doesn't
> > > work!
> 
> The PC is valid at this point, right?  If there were clean Dwarf CFI
> for the entry point, marking it as the oldest frame, would that calm
> GDB down?  That could be another approach.

Yes, that should generally work.

-- 
Daniel Jacobowitz
CodeSourcery

Re: frame theory, was pointer madness

[Accounts]

Daniel Jacobowitz wrote:

>He's connecting to basically a standard gdbserver, poised at
>the first instruction of the program.  Memory has garbage
>and/or is invalid - no MMU so reading from garbage memory
>is a bit more serious than is typical for GDB.
>
>The best thing here would be, if the stub can find out from
>the kernel what constitutes "valid" RAM, to refuse reads to
>it.  Then ignore the ugliness when you type backtrace and
>don't have a stack yet - it's not real surprising that doesn't
>work!
>
>  
>
This sort of issue can be a BIG problem for embedded targets (it has
been for every embedded target ive used ARM and PowerPC/MPC8XX).

Ive talked about it before in previous posts.  Its worse when you use a
hardware interface for your "Stub" like a JTAG pod, BDM, or the like,
because then there is no Kernel to tell you if you have valid memory or
not (The kernel hasnt run yet).  There are also times on embedded
devices when the nominal stack pointer register isnt a stack pointer
register at all (for example immediately after reset of a cpu, before
the boot code has had a chance to set up the system), and dereferences
of it are "Bad News", can cause corruption of the target under the
certain circumstances and generally make embedded systems development
hell (at that level).

On Thu, Jan 26, 2006 at 11:40:07PM +0100, Mark Kettenis wrote:

>To me, it looks like you're connecting to a buggy stub.
>  
>
I dont think its fair to say these things are "Buggy Stubs" just because
GDB lately has the view there is ALWAYS a stack, which just isnt
correct. Also, the stubs probably pre-date GDB's new frame unwinding
code, so really its GDB that has changed in an incompatible way with
previously working stubs. Having to work around this in a stub is a real
pain.  The issue is the stub cant determine whats a memory access
command the user wants to do, and what is an automatic GDB generated
memory access, as it does a frame unwind.  So there is no nice way to
make a stub say "No Stack memory yet" even if it knew it.

Ive often thought GDB should have a "No ABI" option that completely
prevents any stack/frame unwinding operations from working, so these
sorts of problems go away when there is no ABI, and then when the user
knows the target is properly set up, the user can re-enable the targets
ABI, which allows these stack memory accesses to occur.  Ive even looked
at how it could be done, but it seems a very complicated thing, and i
could never quite work out a way to achieve it. (Due to a complete lack
of understanding of the frame code).

Also, for the record, my experience is GDB does this stuff even if you
dont execute a backtrace, it is done automatically when the stub
connects.  Its like it is unwinding the frame to set up a cache or
something.

Steven Johnson

Re: remote connection crash, was frame theory

[Jim Blandy]

On 1/26/06, Daniel Jacobowitz <drow@false.org> wrote:
> On Thu, Jan 26, 2006 at 03:42:02PM -0800, Jim Blandy wrote:
> > On 1/26/06, NZG <ngustavson@emacinc.com> wrote:
> > > On Thursday 26 January 2006 4:44 pm, Daniel Jacobowitz wrote:
> > > > On Thu, Jan 26, 2006 at 11:40:07PM +0100, Mark Kettenis wrote:
> > > > > To me, it looks like you're connecting to a buggy stub.
> > > >
> > > > He's connecting to basically a standard gdbserver, poised at
> > > > the first instruction of the program.  Memory has garbage
> > > > and/or is invalid - no MMU so reading from garbage memory
> > > > is a bit more serious than is typical for GDB.
> > > righto, it crashes the remote kernel and sends my host into an infinite loop
> > > in gdb.
> > >
> > > > The best thing here would be, if the stub can find out from
> > > > the kernel what constitutes "valid" RAM, to refuse reads to
> > > > it.  Then ignore the ugliness when you type backtrace and
> > > > don't have a stack yet - it's not real surprising that doesn't
> > > > work!
> >
> > The PC is valid at this point, right?  If there were clean Dwarf CFI
> > for the entry point, marking it as the oldest frame, would that calm
> > GDB down?  That could be another approach.
>
> Yes, that should generally work.

For reference, I added Dwarf CFI for the _start function in crt0.s for
the m32c libgloss port:

http://sourceware.org/cgi-bin/cvsweb.cgi/src/libgloss/m32c/crt0.S?rev=1.2&content-type=text/x-cvsweb-markup&cvsroot=src

You'll need to look at the CFI section of the Dwarf spec to adapt this
to your processor.  (Fortunately, Dwarf CFI is pretty independent of
the rest of Dwarf, so don't be discouraged by the overall size of the
spec.)

Re: remote connection crash, was frame theory

[NZG]

> For reference, I added Dwarf CFI for the _start function in crt0.s for
> the m32c libgloss port:
>
> http://sourceware.org/cgi-bin/cvsweb.cgi/src/libgloss/m32c/crt0.S?rev=1.2&c
>ontent-type=text/x-cvsweb-markup&cvsroot=src
>
> You'll need to look at the CFI section of the Dwarf spec to adapt this
> to your processor.  (Fortunately, Dwarf CFI is pretty independent of
> the rest of Dwarf, so don't be discouraged by the overall size of the
> spec.)
Thanks Jim, I'll take a look at her tomorrow.
I'm goin home.

thx for all you help today,
Gnight.

Re: frame theory, was pointer madness

[Eli Zaretskii]

> Date: Thu, 26 Jan 2006 15:16:19 -0800
> From: Jim Blandy <jimb@red-bean.com>
> Cc: gdb-patches@sourceware.org, Daniel Jacobowitz <drow@false.org>
> 
> There isn't any master document defining all this, but I'll see if I
> can pirate some of what's been written into this thread for
> gdbint.texinfo.

Thanks!

5282 gdb Eclipse MI support, was remote connection crash

[NZG]

> > For reference, I added Dwarf CFI for the _start function in crt0.s for
> > the m32c libgloss port:
Interesting, but it may be beyond what I have a timeline for at this point.
I have had some luck simply by enabling the section of code below, which 
appears to have been designed specifically for this purpose.

Everything appears to be working now, although a little bit slower than I 
like.
Anyway progress is being made.

thank you for all your help,
Nathan


  /* If we're inside the entry file, it isn't valid.  Don't apply this
     test to a dummy frame - dummy frame PC's typically land in the
     entry file.  Don't apply this test to the sentinel frame.
     Sentinel frames should always be allowed to unwind.  */
  /* NOTE: drow/2002-12-25: should there be a way to disable this
     check?  It assumes a single small entry file, and the way some
     debug readers (e.g.  dbxread) figure out which object is the
     entry file is somewhat hokey.  */
  /* NOTE: cagney/2003-01-10: If there is a way of disabling this test
     then it should probably be moved to before the ->prev_p test,
     above.  */
  /* NOTE: vinschen/2003-04-01: Disabled.  It turns out that the call
     to deprecated_inside_entry_file destroys a meaningful backtrace
     under some conditions.  E. g. the backtrace tests in the
     asm-source testcase are broken for some targets.  In this test
     the functions are all implemented as part of one file and the
     testcase is not necessarily linked with a start file (depending
     on the target).  What happens is, that the first frame is printed
     normaly and following frames are treated as being inside the
     enttry file then.  This way, only the #0 frame is printed in the
     backtrace output.  */
     /**NZG enabled for expermentation**/
     
     
  if (1
      && this_frame->type != DUMMY_FRAME && this_frame->level >= 0
      && deprecated_inside_entry_file (get_frame_pc (this_frame)))
    {
      if (frame_debug)
	{
	  fprintf_unfiltered (gdb_stdlog, "-> ");
	  fprint_frame (gdb_stdlog, NULL);
	  fprintf_unfiltered (gdb_stdlog, " // inside entry file }\n");
	}
      return NULL;
    }




On Thursday 26 January 2006 6:03 pm, NZG wrote:
> > For reference, I added Dwarf CFI for the _start function in crt0.s for
> > the m32c libgloss port:
> >
> > http://sourceware.org/cgi-bin/cvsweb.cgi/src/libgloss/m32c/crt0.S?rev=1.2
> >&c ontent-type=text/x-cvsweb-markup&cvsroot=src
> >
> > You'll need to look at the CFI section of the Dwarf spec to adapt this
> > to your processor.  (Fortunately, Dwarf CFI is pretty independent of
> > the rest of Dwarf, so don't be discouraged by the overall size of the
> > spec.)
>
> Thanks Jim, I'll take a look at her tomorrow.
> I'm goin home.
>
> thx for all you help today,
> Gnight.