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]

Ping: [Patch] Fix windows 64 unwinding issues


Apparently this patch should be reviewed by a global maintainer as there
is no platform maintainer for Windows.

Tristan.

> On 05 Mar 2015, at 14:42, Tristan Gingold <gingold@adacore.com> wrote:
> 
> 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]