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

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

[Roland Schwingel]

Hi Pedro and Kai,

Thanks for checking it in. I am still absolutely swamped with other 
things so I had no time at present to push this forward.

Roland

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

[Roland Schwingel]

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

Hi,

Pedro Alves wrote on 30.03.2012 11:14:03:

 > 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. 
But ok... Here is now generation 4 of my patch. It incorporates all 
formatting wishes of Joel.

I am still waiting for my copyright assignment stuff to be processed. It 
is now more than 2 weeks. What is the official way to ping it?

ChangeLog:

2012-04-02  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.

Thanks,

Roland

[-- Attachment #2: amd64-windows-tdep.c.patch --]
[-- Type: text/plain, Size: 2004 bytes --]

--- amd64-windows-tdep.c.orig	2012-03-02 01:06:12.000000000 +0100
+++ amd64-windows-tdep.c	2012-03-30 13:03:23.944573800 +0200
@@ -23,6 +23,7 @@
 #include "gdbtypes.h"
 #include "gdbcore.h"
 #include "regcache.h"
+#include "frame.h"
 
 /* The registers used to pass integer arguments during a function call.  */
 static int amd64_windows_dummy_call_integer_regs[] =
@@ -153,12 +154,49 @@
   return pc;
 }
 
+/* Check win64 DLL jmp trampolines and find jump destination.  */
+
+static CORE_ADDR
+amd64_windows_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
+{
+  CORE_ADDR destination = 0;
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+  /* Check for jmp *<offset>(%rip) (jump near, absolute indirect (/4)).  */
+  if (pc && read_memory_unsigned_integer (pc, 2, byte_order) == 0x25ff)
+    {
+      /* Get opcode offset and see if we can find a reference in our data.  */
+      ULONGEST offset =
+        read_memory_unsigned_integer (pc + 2, 4, byte_order);
+
+      /* Get address of function pointer at end of pc.  */
+      CORE_ADDR indirect_addr = pc + offset + 6;
+
+      struct minimal_symbol *indsym =
+        indirect_addr ? lookup_minimal_symbol_by_pc (indirect_addr) : NULL;
+      const char *symname = indsym ? SYMBOL_LINKAGE_NAME (indsym) : NULL;
+
+      if (symname)
+        {
+          if (strncmp (symname, "__imp_", 6) == 0
+              || strncmp (symname, "_imp_", 5) == 0)
+            destination =
+                read_memory_unsigned_integer (indirect_addr, 8, byte_order);
+        }
+    }
+
+  return destination;
+}
 
 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);
+
   amd64_init_abi (info, gdbarch);
 
   /* On Windows, "long"s are only 32bit.  */

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

[Pedro Alves]

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

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

[Kai Tietz]

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

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

[Pedro Alves]

Hi Kai,

On 07/25/2012 06:57 PM, Kai Tietz wrote:
> 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.  

Thanks.

> 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).

A patch with the comments addressed would have been better than just a ping.  ;-)

>>> 2012-04-02  Roland Schwingel <roland.schwingel@onevision.com>
>>
>> Should be two spaces after your name.
>>

>>> +/* 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);

I confirm his paperwork is okay.  I've done the changes mentioned above, and
also a few other formatting fixes.  I've checked it in, as below.

2012-07-27  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.

---

 gdb/amd64-windows-tdep.c |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 41e0efa..528fbb6 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -24,6 +24,7 @@
 #include "gdbcore.h"
 #include "regcache.h"
 #include "windows-tdep.h"
+#include "frame.h"

 /* The registers used to pass integer arguments during a function call.  */
 static int amd64_windows_dummy_call_integer_regs[] =
@@ -154,6 +155,40 @@ amd64_skip_main_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
   return pc;
 }

+/* Check Win64 DLL jmp trampolines and find jump destination.  */
+
+static CORE_ADDR
+amd64_windows_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
+{
+  CORE_ADDR destination = 0;
+  struct gdbarch *gdbarch = get_frame_arch (frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+  /* Check for jmp *<offset>(%rip) (jump near, absolute indirect (/4)).  */
+  if (pc && read_memory_unsigned_integer (pc, 2, byte_order) == 0x25ff)
+    {
+      /* Get opcode offset and see if we can find a reference in our data.  */
+      ULONGEST offset
+	= read_memory_unsigned_integer (pc + 2, 4, byte_order);
+
+      /* Get address of function pointer at end of pc.  */
+      CORE_ADDR indirect_addr = pc + offset + 6;
+
+      struct minimal_symbol *indsym
+	= indirect_addr ? lookup_minimal_symbol_by_pc (indirect_addr) : NULL;
+      const char *symname = indsym ? SYMBOL_LINKAGE_NAME (indsym) : NULL;
+
+      if (symname)
+	{
+	  if (strncmp (symname, "__imp_", 6) == 0
+	      || strncmp (symname, "_imp_", 5) == 0)
+	    destination
+	      = read_memory_unsigned_integer (indirect_addr, 8, byte_order);
+	}
+    }
+
+  return destination;
+}

 static void
 amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
@@ -174,6 +209,8 @@ amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   tdep->integer_param_regs_saved_in_caller_frame = 1;
   set_gdbarch_return_value (gdbarch, amd64_windows_return_value);
   set_gdbarch_skip_main_prologue (gdbarch, amd64_skip_main_prologue);
+  set_gdbarch_skip_trampoline_code (gdbarch,
+				    amd64_windows_skip_trampoline_code);

   set_gdbarch_iterate_over_objfiles_in_search_order
     (gdbarch, windows_iterate_over_objfiles_in_search_order);