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

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

[Roland Schwingel]

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

Hi...

When single stepping a 64bit windows application gdb at present does not 
step into
functions residing in a dll. This is due to the fact that handling of 
dll trampoline code
for win64 is missing.

I added a new function to amd64-windows-tdep.c to handle this similar to 
the existing function (i386_pe_skip_trampoline_code()). With some 
differences:

- On 32bit windows dll trampoline code is expressed as jmp *(dest) while on
   64bit windows this is expressed as jmp *<offset>(%rip). Took care of 
this.
- The jump destination is on 64bit windows of course 8 byte long. I 
could not
   find a function that transforms this into a CORE_ADDR like 
read_memory_unsigned_integer()
   it is doing in the 32bit case.  So I did the transformation on my 
own. While this is
   high performant it might not be the "official" gdb way. If someone 
can give me a
   hint on how to the transformation the "official" way I will adjust my 
patch - if wished.

Now single stepping into dll code works.

ChangeLog:

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.

Roland



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

--- amd64-windows-tdep.c_orig	2012-03-02 01:06:12.000000000 +0100
+++ amd64-windows-tdep.c	2012-03-14 13:31:39.815727600 +0100
@@ -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,59 @@
   return pc;
 }
 
+/* Stuff for WIN64 PE style DLL's but is pretty generic really.  */
+
+static CORE_ADDR
+amd64_windows_skip_trampoline_code (struct frame_info *frame, CORE_ADDR pc)
+{
+    struct gdbarch     *gdbarch = get_frame_arch (frame);
+    enum bfd_endian     byte_order = gdbarch_byte_order (gdbarch);
+
+   /* check for jmp *<offset>(%rip) */
+    if (pc && read_memory_unsigned_integer (pc, 2, byte_order) == 0x25ff)
+      {
+	  unsigned long indirect =
+	    read_memory_unsigned_integer (pc + 2, 4, byte_order);
+	  struct minimal_symbol *indsym =
+	    indirect ? lookup_minimal_symbol_by_pc (pc + indirect) : 0;
+	  const char *symname =
+	    indsym ? SYMBOL_LINKAGE_NAME (indsym) : 0;
+
+	  if (symname)
+	    {
+		if (strncmp (symname, "__imp_", 6) == 0
+		    || strncmp (symname, "_imp_", 5) == 0)
+		  {
+		      CORE_ADDR           destination;
+		      gdb_byte           *pos, addr[8];
+
+		      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;
+		  }
+	    }
+      }
+    return 0;			/* Not a trampoline.  */
+}
 
 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] Add dll trampoline code handling for windows 64bit

[Tom Tromey]

>>>>> "Roland" == Roland Schwingel <roland@onevision.com> writes:

Roland> - The jump destination is on 64bit windows of course 8 byte
Roland> long. I could not find a function that transforms this into a
Roland> CORE_ADDR like read_memory_unsigned_integer() it is doing in the
Roland> 32bit case.  So I did the transformation on my own. While this
Roland> is high performant it might not be the "official" gdb way. If
Roland> someone can give me a hint on how to the transformation the
Roland> "official" way I will adjust my patch - if wished.

Just use read_memory_unsigned_integer.
I think it is guaranteed that CORE_ADDR will not be larger than
ULONGEST.

I can't really comment on the rest of your patch.

I think it is big enough that it requires a copyright assignment.
If you don't have one, contact me off-list to get started.

Tom

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

[Joel Brobecker]

Hi Roland,

Thanks for the patch.

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

> +/* Stuff for WIN64 PE style DLL's but is pretty generic really.  */

It's great that you thought about adding a description comment to
document the new function, thank you! I just wish the description
was a little more precise. In fact, what this function does should
already be described in gdbarch.h.  For gdbarch callback/"methods",
we usually simply say:

/* Implement the "skip_trampoline_code" gdbarch method.  */

No need to repeat the hook documentation at each instance.

> +    struct gdbarch     *gdbarch = get_frame_arch (frame);
> +    enum bfd_endian     byte_order = gdbarch_byte_order (gdbarch);

Formatting nit: We do not align parameters in GDB code.

Also, the indentation is wrong. We use 2 spaces, not 4, so the whole
function needs to be re-indented.

> +   /* check for jmp *<offset>(%rip) */

Also, the GNU Coding Style which GDB follows requires us to start
sentences with a capital letter, and to end them with a period.
(and 2 spaces after periods).  I would format the comment above as:

        /* Check for "jmp *<offset>(%rip)".  */

> +    if (pc && read_memory_unsigned_integer (pc, 2, byte_order) == 0x25ff)
> +      {
> +	  unsigned long indirect =
> +	    read_memory_unsigned_integer (pc + 2, 4, byte_order);

Can use use ULONGEST here, instead of "unsigned long"?

> +		      CORE_ADDR           destination;
> +		      gdb_byte           *pos, addr[8];

Same as above, please, no extra spacing to align the parameter names.

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

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.

Thanks,
-- 
Joel

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

[Roland Schwingel]

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

Hi...

Here is a new version of my patch from yesterday. I took (hopefully 
correct) all
suggestions regarding formatting and indention into account. If there is 
still
something wrong just tell me.

Regarding my byteswap code:
I spent nearly the whole day investigating that up and down. I can't use
the gdb read_memory functions here that do endianness conversion. I stepped
down many times the read_memory functions over all frames until the
point where the memory is read from the inferior using windows native
function ReadProcessMemory(). It appears that - in this case - the memory
returned is rotated by 2 bytes.

Example: Function pointer in a dll is eg: 0x0000000015027bba
In little endian it would be: 0xba7b021500000000
The ReadProcessMemory() call in windows-nat.c in function
windows_xfer_memory() always returns this:
0x021500000000ba7b
In all other cases the memory returned appears not to be rotated.
I also tried reading byte/word/int-wise but the result did not change.

I also queried the internet a long time but could not find any clear
explanation for this. So I left a comment in the patch at this point
as the result delivered by the new function represents the right
function pointer in any case I have checked.

Regarding Copyright Assignment:
As soon as I get the email from Tom I will do everything explained there 
and send it in.

ChangeLog:

2012-03-15  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.

Roland

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

--- amd64-windows-tdep.c_orig	2012-03-02 01:06:12.000000000 +0100
+++ amd64-windows-tdep.c	2012-03-15 15:37:45.647047400 +0100
@@ -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,65 @@
   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 indirect =
+        read_memory_unsigned_integer (pc + 2, 4, byte_order);
+      struct minimal_symbol *indsym =
+        indirect ? lookup_minimal_symbol_by_pc (pc + indirect) : 0;
+      const char *symname = indsym ? SYMBOL_LINKAGE_NAME (indsym) : 0;
+
+      if (symname)
+        {
+          if (strncmp (symname, "__imp_", 6) == 0
+              || strncmp (symname, "_imp_", 5) == 0)
+            {	   	     
+              gdb_byte *pos,addr[8];
+
+              read_memory(pc + indirect, addr, 8);
+              /* The data fetched from the inferior is in this
+                 case not little endian, 2 bytes from the
+                 beginning are rotated to the end. 
+                 Example: 
+                 function pointer expected in little endian:
+                 0xba7b021500000000 
+                 pointer fetched from inferior:
+                 0x021500000000ba7b  
+                 So I do byteswapping here on my own. */
+              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;
+}
 
 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 v2] Add dll trampoline code handling for windows 64bit

[Tom Tromey]

>>>>> "Roland" == Roland Schwingel <roland@onevision.com> writes:

Roland> Here is a new version of my patch from yesterday. I took
Roland> (hopefully correct) all suggestions regarding formatting and
Roland> indention into account.

It seems ok to me.

Roland> I can't use the gdb read_memory functions here that do
Roland> endianness conversion.

Kai Tietz read this patch -- he says "hello" and asked me to respond.

Roland> +      struct minimal_symbol *indsym =
Roland> +        indirect ? lookup_minimal_symbol_by_pc (pc + indirect) : 0;

Kai says that PC-relative expressions are relative to the end of the
instruction.  So, 'pc + indirect' is incorrect here and also in the
read_memory call.

Tom