Re: [PATCH] Add dll trampoline code handling for windows 64bit

[Roland Schwingel <roland.schwingel@onevision.com>]

Hi Joel,

Thanks for you reply and suggestions. I will prepare a new patch
tomorrow when back at the office with all your suggestions.

Joel Brobecker <brobecker@adacore.com> wrote on 14.03.2012 17:13:03:
 > In addition to Tom's answer, I have a few minor comments. The real
 > review will have to come from our Windows Maintainer for your patch
 > to be approved...
 >
 > > 2012-03-14  Roland Schwingel<roland.schwingel@onevision.com>
 > >
 > >         * amd64-windows-tdep.c: #include "frame.h"
 > >         (amd64_windows_skip_trampoline_code): New function.
 > >    (amd64_windows_init_abi): Add trampoline registration.
 >
 > (missing period at the end of the first line).
OK

There is a script called gdb_indent.sh in gdb's root dir. I applied
it on amd64-windows-nat.c ahead of getting out my patch thinking
that this would be the correct way. Some misunderstanding as it
produced most of your indention concerns.

My patch is based upon the implementation for windows 32 bit in
i386-tdep.c (function  i386_pe_skip_trampoline_code()). I made
my patch as close as possible to the implementation there thinking
that would make it easier to be accepted. If you take a look there
you will see home similar my patch is.

Your suggestions are newer, so I will take them into account.

 > > +            read_memory (pc + indirect, addr, 8);
 > > +            pos = (gdb_byte *) &destination;
 > > +            pos[0] = addr[6];
 > > +            pos[1] = addr[7];
 > > +            pos[2] = addr[0];
 > > +            pos[3] = addr[1];
 > > +            pos[4] = addr[2];
 > > +            pos[5] = addr[3];
 > > +            pos[6] = addr[4];
 > > +            pos[7] = addr[5];
 > > +
 > > +            return destination;
 >
 > Yeah, Tom's suggestion is a better suggestion. I think you are going
 > to have endianness issues this way.  You could use
 > read_memory_typed_address as well, but it's a little more involved,
 > and I don't think it's necessary here.  But otherwise, to me, the
 > latter is the function to be used for reading addresses from inferior
 > memory.
Before doing it "my" way I already played around with both
read_memory_typed_address and read_memory_unsigned_integer
but did not get the correct CORE_ADDR. The bytes were always
in the wrong order believing that these are endianess issues. Will
reinvestigate that when back at the office. Anyhow my approach
appears to be working, too. I succesfully single stepped thru
many dlls using my patch on win64 which was not possible
before.

 > I am sorry if it feels like it's a lot of little rules. It is. But
 > it should be easy to learn them and it allows us to have a consistent
 > style for our code.
Sure. No problem!

Thanks for taking a look at my patch,

Roland