Re: gdb code review, pointer madness

NZG
2006-01-24T19:29:52Z

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