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

Kai Tietz
2012-07-25T17:57:55Z

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