Re: Post mortem debugging for Windows CE

[Pedro Alves <pedro@codesourcery.com>]

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.

-- 
Pedro Alves