Re: Post mortem debugging for Windows CE

[Danny Backx <danny dot backx at scarlet dot be>]

Here is a first cut of my work. Please comment.

	Danny

On Sun, 2009-05-10 at 20:21 +0100, Pedro Alves wrote:
> On Thursday 07 May 2009 02:21:04, Danny Backx wrote:
> > On Wed, 2009-05-06 at 21:48 +0100, Pedro Alves wrote:
> > > A few suggestions:
> > > 
> > >  - Please forgive me if you know this already.
> > >    Your minidump bfd code should work on all hosts, 64-bit, 32-bit,
> > >    big endian or little endian.  This means that code like:
> > [..]
> > >    ... is unacceptable.  You need to use the bfd_get_32 and friends
> > >    to parse the data on the file.
> > 
> > I'll start doing all that. I didn't know about avoiding structs so
> > this'll be some work.
> > 
> > Should they be avoided completely ? I wrote code like the snippet below.
> > Should the sizeof (CEDUMP_THREAD_CALL_STACK_LIST) be replaced ? Should
> > the struct definitions be gone completely, or still be there in
> > comment ?
> 
> Take a look at src/include/coff/external.h and
> src/include/coff/internal.h, or grep for Internal and External
> in src/include/elf/, or grep for "swap" in bfd.  The external variants
> are the ones that are read out of file.  The internal variants
> are the ones used internally.  Presumably, you won't have that much
> code, so you may chose to always use "external" types, and extract
> the fields with bfd_get_foo.
> 
> > Also this'll work for CE based minidumps now, not the ones from desktop
> > Windows. This may be as simple as getting the code to handle not only
> > the 
> >         ceStreamThreadCallStackList = 0x8007
> > but also the desktop value. But life is usually not that simple.
> 
> If this is not useful to you, then you don't have to implement it.
> I'm sure someone else having an interest in desktop Windows will
> take of adding such functionality.
> 
> >   rva = minidump_core_locate_stream(abfd, ceStreamThreadCallStackList);
> >   if (bfd_seek (abfd, (file_ptr) rva, SEEK_SET) != 0)
> >     return;
> >   nread = bfd_bread (&tcsl, (bfd_size_type) sizeof
> > (CEDUMP_THREAD_CALL_STACK_LIST), abfd);
> >   if (nread != sizeof (CEDUMP_THREAD_CALL_STACK_LIST))
> >     {
> >       if (bfd_get_error () != bfd_error_system_call)
> >         bfd_set_error (bfd_error_wrong_format);
> >       return;
> >     }
> > 
> >   /*
> >    * Need to read all the CEDUMP_THREAD_CALL_STACK entries,
> >    * they're just after the CEDUMP_THREAD_CALL_STACK_LIST.
> >    * The CEDUMP_THREAD_CALL_STACK_FRAME fields are in potentially other
> > places though.
> >    * So allocate space for enough CEDUMP_THREAD_CALL_STACK_LIST entries.
> >    */
> >   sz = tcsl.NumberOfEntries * sizeof(CEDUMP_THREAD_CALL_STACK);
> 
> Right, this is exactly what you should *not* do.  That
> 'tcsl.NumberOfEntries' field will return garbage if e.g., the
> host machine running gdb is big endian, while the minidump data
> is supposedly stored in little endian (ARM).  Same for hidden
> padding the compiler may insert in the structure, influencing
> the tcsl fields offsets, and the size of the structure in the
> view of the host.
>

Attachment: diffs-v1.gz
Description: GNU Zip compressed data