This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Patch] Fix windows 64 unwinding issues


Hello,

yet another patch to fix incorrect unwinding in system dlls.  Was simply manually tested.

Ok to commit ?

Tristan.

commit da3b5213dc072fca195451a04f35a2eb6342bb62
Author: Tristan Gingold <gingold@adacore.com>
Date:   Thu Mar 5 14:36:32 2015 +0100

    Fix amd64 windows unwinding issues within MS dlls.
    
    Unwind info in system dlls uses almost all possible codes, contrary to unwind
    info generated by gcc.  A few issues have been discovered: incorrect handling
    of SAVE_NONVOL opcodes and incorrect in prologue range checks.  Furthermore I
    added comments not to forget what has been investigated.
    
    gdb/ChangeLog:
    	* amd64-windows-tdep.c (amd64_windows_find_unwind_info): Move
    	redirection code to ...
    	(amd64_windows_frame_decode_insns): ... Here.  Fix in prologue
    	checks.  Fix SAVE_NONVOL operations.  Add debug code and comments.

diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 2aa10a1..9278a26 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -621,9 +621,47 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
   CORE_ADDR cur_sp = cache->sp;
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  int j;
+  int first = 1;
+
+  /* There are at least 3 possibilities to share an unwind info entry:
+     1. Two different runtime_function entries (in .pdata) can point to the
+        same unwind info entry.  There is no such indication while unwinding,
+	so we don't really care about that case.  We suppose this scheme is
+	used to save memory when the unwind entries are exactly the same.
+     2. Chained unwind_info entries, with no unwind codes (no prologue).
+        There is a major difference with the previous case: the pc range for
+	the function is different (in case 1, the pc range comes from the
+	runtime_function entry; in case 2, the pc range for the chained entry
+	comes from the first unwind entry).  Case 1 cannot be used instead as
+	the pc is not in the prologue.  This case is officially documented.
+	(There might be unwind code in the first unwind entry to handle
+	additionnal unwinding).  GCC (at least until gcc 5.0) doesn't chain
+	entries.
+     3. Undocumented unwind info redirection.  Hard to know the exact purpose,
+        so it is considered as a memory optimization of case 2.
+  */
 
-  for (j = 0; ; j++)
+  if (unwind_info & 1)
+    {
+      /* Unofficially documented unwind info redirection, when UNWIND_INFO
+	 address is odd (http://www.codemachine.com/article_x64deepdive.html).
+      */
+      struct external_pex64_runtime_function d;
+      CORE_ADDR sa, ea;
+
+      if (target_read_memory (cache->image_base + (unwind_info & ~1),
+			      (gdb_byte *) &d, sizeof (d)) != 0)
+	return;
+
+      cache->start_rva =
+	extract_unsigned_integer (d.rva_BeginAddress, 4, byte_order);
+      cache->end_rva =
+	extract_unsigned_integer (d.rva_EndAddress, 4, byte_order);
+      unwind_info =
+	extract_unsigned_integer (d.rva_UnwindData, 4, byte_order);
+    }
+
+  while (1)
     {
       struct external_pex64_unwind_info ex_ui;
       /* There are at most 256 16-bit unwind insns.  */
@@ -633,6 +671,7 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
       unsigned char codes_count;
       unsigned char frame_reg;
       unsigned char frame_off;
+      CORE_ADDR start;
 
       /* Read and decode header.  */
       if (target_read_memory (cache->image_base + unwind_info,
@@ -653,12 +692,13 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
 	  && PEX64_UWI_VERSION (ex_ui.Version_Flags) != 2)
 	return;
 
-      if (j == 0
-	  && (cache->pc >=
-	      cache->image_base + cache->start_rva + ex_ui.SizeOfPrologue))
+      start = cache->image_base + cache->start_rva;
+      if (first
+	  && !(cache->pc >= start && cache->pc < start + ex_ui.SizeOfPrologue))
 	{
-	  /* Not in the prologue.  We want to detect if the PC points to an
-	     epilogue. If so, the epilogue detection+decoding function is
+	  /* We want to detect if the PC points to an epilogue.  This needs
+	     to be checked only once, and an epilogue can be anywhere but in
+	     the prologue.  If so, the epilogue detection+decoding function is
 	     sufficient.  Otherwise, the unwinder will consider that the PC
 	     is in the body of the function and will need to decode unwind
 	     info.  */
@@ -711,19 +751,24 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
 	{
 	  int reg;
 
-	  if (frame_debug)
-	    fprintf_unfiltered
-	      (gdb_stdlog, "   op #%u: off=0x%02x, insn=0x%02x\n",
-	       (unsigned) (p - insns), p[0], p[1]);
-
-	  /* Virtually execute the operation.  */
-	  if (cache->pc >= cache->image_base + cache->start_rva + p[0])
+	  /* Virtually execute the operation if the pc is after the
+	     corresponding instruction (that does matter in case of break
+	     within the prologue).  Note that for chained info (!first), the
+	     prologue has been fully executed.  */
+	  if (cache->pc >= start + p[0] || cache->pc < start)
 	    {
+	      if (frame_debug)
+		fprintf_unfiltered
+		  (gdb_stdlog, "   op #%u: off=0x%02x, insn=0x%02x\n",
+		   (unsigned) (p - insns), p[0], p[1]);
+
 	      /* If there is no frame registers defined, the current value of
 		 rsp is used instead.  */
 	      if (frame_reg == 0)
 		save_addr = cur_sp;
 
+	      reg = -1;
+
 	      switch (PEX64_UNWCODE_CODE (p[1]))
 		{
 		case UWOP_PUSH_NONVOL:
@@ -751,12 +796,12 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
 		case UWOP_SAVE_NONVOL:
 		  reg = amd64_windows_w2gdb_regnum[PEX64_UNWCODE_INFO (p[1])];
 		  cache->prev_reg_addr[reg] = save_addr
-		    - 8 * extract_unsigned_integer (p + 2, 2, byte_order);
+		    + 8 * extract_unsigned_integer (p + 2, 2, byte_order);
 		  break;
 		case UWOP_SAVE_NONVOL_FAR:
 		  reg = amd64_windows_w2gdb_regnum[PEX64_UNWCODE_INFO (p[1])];
 		  cache->prev_reg_addr[reg] = save_addr
-		    - 8 * extract_unsigned_integer (p + 2, 4, byte_order);
+		    + 8 * extract_unsigned_integer (p + 2, 4, byte_order);
 		  break;
 		case UWOP_SAVE_XMM128:
 		  cache->prev_xmm_addr[PEX64_UNWCODE_INFO (p[1])] =
@@ -787,6 +832,13 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
 		default:
 		  return;
 		}
+
+	      /* Display address where the register was saved.  */
+	      if (frame_debug && reg >= 0)
+		fprintf_unfiltered
+		  (gdb_stdlog, "     [reg %s at %s]\n",
+		   gdbarch_register_name (gdbarch, reg),
+		   paddress (gdbarch, cache->prev_reg_addr[reg]));
 	    }
 
 	  /* Adjust with the length of the opcode.  */
@@ -818,19 +870,29 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
 	    }
 	}
       if (PEX64_UWI_FLAGS (ex_ui.Version_Flags) != UNW_FLAG_CHAININFO)
-	break;
+	{
+	  /* End of unwind info.  */
+	  break;
+	}
       else
 	{
 	  /* Read the chained unwind info.  */
 	  struct external_pex64_runtime_function d;
 	  CORE_ADDR chain_vma;
 
+	  /* Not anymore the first entry.  */
+	  first = 0;
+
+	  /* Stay aligned on word boundary.  */
 	  chain_vma = cache->image_base + unwind_info
 	    + sizeof (ex_ui) + ((codes_count + 1) & ~1) * 2;
 
 	  if (target_read_memory (chain_vma, (gdb_byte *) &d, sizeof (d)) != 0)
 	    return;
 
+	  /* Decode begin/end.  This may be different from .pdata index, as
+	     an unwind info may be shared by several functions (in particular
+	     if many functions have the same prolog and handler.  */
 	  cache->start_rva =
 	    extract_unsigned_integer (d.rva_BeginAddress, 4, byte_order);
 	  cache->end_rva =
@@ -940,25 +1002,6 @@ amd64_windows_find_unwind_info (struct gdbarch *gdbarch, CORE_ADDR pc,
        "amd64_windows_find_unwind_data:  image_base=%s, unwind_data=%s\n",
        paddress (gdbarch, base), paddress (gdbarch, *unwind_info));
 
-  if (*unwind_info & 1)
-    {
-      /* Unofficially documented unwind info redirection, when UNWIND_INFO
-	 address is odd (http://www.codemachine.com/article_x64deepdive.html).
-      */
-      struct external_pex64_runtime_function d;
-      CORE_ADDR sa, ea;
-
-      if (target_read_memory (base + (*unwind_info & ~1),
-			      (gdb_byte *) &d, sizeof (d)) != 0)
-	return -1;
-
-      *start_rva =
-	extract_unsigned_integer (d.rva_BeginAddress, 4, byte_order);
-      *end_rva = extract_unsigned_integer (d.rva_EndAddress, 4, byte_order);
-      *unwind_info =
-	extract_unsigned_integer (d.rva_UnwindData, 4, byte_order);
-
-    }
   return 0;
 }
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]