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

[Kai Tietz <ktietz70 at googlemail dot com>]

Hello,

I have now this patch for a while on my radar.  I contacted Roland
Schwingel about the status of his paperwork with FSF, and he told me
that he completed it.  So I want to ping this patch for him, as he
isn't able to ping on that patch himself for the next week(s).

Regards,
Kai


2012/4/2 Pedro Alves <palves@redhat.com>:
> On 04/02/2012 03:53 PM, Roland Schwingel wrote:
>
>>> Please send an updated patch, so we have in the archives the exact
>>> patch as what is
>>> checked in, and in case some other maintainer wants to take a look,
>>> best have him look at
>>> the refreshed patch.   In fact, if you had sent it already in that
>>> email, there'd have been
>>> no extra noise, right?  ;-)
>> Regarding the noise right. But not regarding generating additional work.
>
>
> Sorry, I can't be sympathetic to that.  You would be making the changes anyway.
> I can't believe that pasting a patch at the end of an email is extra work by
> any valid measure.  What's real extra work is someone reading an out of
> date patch, and trying to figure out from several messages in a thread what
> would be the final state of the patch.
>
>> 2012-04-02  Roland Schwingel <roland.schwingel@onevision.com>
>
>                               ^
>
> Should be two spaces after your name.
>
>>
>>          * amd64-windows-tdep.c: #include "frame.h".
>>          (amd64_windows_skip_trampoline_code): New function.
>>          (amd64_windows_init_abi): Add trampoline registration.
>
>
> On 04/02/2012 03:53 PM, Roland Schwingel wrote:
>> +/* Check win64 DLL jmp trampolines and find jump destination.  */
>
> The correct spelling is "Win64" capitalized.
>
>>  static void
>>  amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>  {
>>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>>
>> +  /* Register trampoline handling code.  */
>> +  set_gdbarch_skip_trampoline_code (gdbarch, amd64_windows_skip_trampoline_code);
>
> A nit, but it'd be cleaner/clearer to put this after the amd64_init_abi call, or
> better, near the end of the function, after set_gdbarch_skip_main_prologue.  The current
> code reads "initialize the base arch, then install overrides.".  This new call
> here breaks that flow.
>
>> +
>>    amd64_init_abi (info, gdbarch);
>>
>>    /* On Windows, "long"s are only 32bit.  */
>
>
> Having once written the equivalent arm-wince-tdep.c:arm_pe_skip_trampoline_code
> for ARM WinCE, this generally looks good to me too.
>
> --
> Pedro Alves



-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination