Re: Post mortem debugging for Windows CE

Danny Backx
2009-05-11T14:03:42Z

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

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 #2: diffs-v1.gz --]
[-- Type: application/x-gzip, Size: 14649 bytes --]