Add Windows x64 SEH unwinder (take 2)

Add Windows x64 SEH unwinder (take 2)

[Joel Brobecker]

Hello,

This patch series adds an unwinder using the native Windows x64 SEH
data. This dramatically improves the behavior as soon as system
calls are involved. For instance, after switching to a different
thread, we now get...

    (gdb) bt
    #0  0x0000000077a46eba in ntdll!ZwWaitForSingleObject ()
       from C:\Windows\system32\ntdll.dll
    #1  0x00000000778fb380 in WaitForSingleObjectEx ()
       from C:\Windows\system32\kernel32.dll
    #2  0x000000000040a7e9 in system__task_primitives__operations__cond_wait ()
    #3  0x0000000000407c3b in system__tasking__rendezvous__accept_trivial ()
    #4  0x0000000000401f3c in task_switch.callee (<_task>=0x394410)
        at task_switch.adb:29
    #5  0x0000000000406637 in system__tasking__stages__task_wrapper ()
    [...]

... instead of:

    #0  0x0000000077a46eba in ntdll!ZwWaitForSingleObject ()
       from C:\Windows\system32\ntdll.dll
    #1  0x00000000778fb380 in WaitForSingleObjectEx ()
       from C:\Windows\system32\kernel32.dll
    #2  0x000007fffffdc000 in ?? ()
    #3  0x0000000077a266e4 in ntdll!LdrInitializeThunk ()
       from C:\Windows\system32\ntdll.dll
    #4  0x0000000000000000 in ?? ()

[RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Joel Brobecker]

Hello,

This is the main part of the patch series, which actually
adds the unwinder.

One element worth mentioning, perhaps, is the fact that we prepend
the unwinder, and the sniffer is the default_frame_sniffer which
always returns 1 (absence of SEH info is normal and means it is
a leaf function).  This effectively means that all other unwinders
effectively get shunted. And in particular, I do not think that
the DWARF unwinder will kick in even if DWARF unwind info is found.

The problem is that we want to be ahead of the default amd64-tdep
unwinders, which is kind of a last-resort unwinder doing a good
job under limited situations only. We'd like to be behind the DWARF
unwinder if we could, but I don't think there is a way to ensure
that yet.

In practice, this shouldn't be a problem, since this unwinder
has been very reliable for us so far.  But it does assume that
the compiler is recent enough to generate native SEH data which,
for GCC, means GCC 4.7 (I have been told). On the other hand,
it looks like the first GCC release to support x64-windows was
GCC 4.6.

I don't really see a real way of supporting both old and new versions
of GCC, unless we have a way of more finely ordering the unwinders.
Worse case scenario, we stop supporting code generated by GCC 4.6.
Or an alternative option is to provide a setting to disable this
unwinder.

gdb/ChangeLog (from Tristan Gingold and Joel Brobecker):

        * amd64-windows-tdep.c: #include "objfiles.h", "frame-unwind.h",
        "coff/internal.h", "coff/i386.h", "coff/pe.h" and "libcoff.h".
        (struct amd64_windows_frame_cache): New struct.
        (amd64_windows_w2gdb_regnum): New global.
        (amd64_windows_frame_decode_epilogue)
        (amd64_windows_frame_decode_insns, amd64_windows_find_unwind_info)
        (amd64_windows_frame_cache, amd64_windows_frame_prev_register)
        (amd64_windows_frame_this_id): New functions.
        (amd64_windows_frame_unwind): New static global.
        (amd64_windows_skip_prologue): New function.
        (amd64_windows_init_abi): Call frame_unwind_prepend_unwinder
        with amd64_windows_frame_unwind. Call set_gdbarch_skip_prologue
        with amd64_windows_skip_prologue.

        * NEWS: Add entry mentioning support for native Windows x64
        SEH data.

Tested on x86_64-windows. I will wait until the end of the week
for comments and suggestions...

Thank you :)
-- 
Joel

---
 gdb/NEWS                 |    2 +
 gdb/amd64-windows-tdep.c |  688 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 690 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 3451505..d981ac5 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -134,6 +134,8 @@ show print type typedefs
   feature to be enabled.  For more information, see:
       http://fedoraproject.org/wiki/Features/MiniDebugInfo
 
+* GDB can now use Windows x64 unwinding data.
+
 *** Changes in GDB 7.5
 
 * GDB now supports x32 ABI.  Visit <http://sites.google.com/site/x32abi/>
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index e7addfc..90f7628 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -25,6 +25,12 @@
 #include "regcache.h"
 #include "windows-tdep.h"
 #include "frame.h"
+#include "objfiles.h"
+#include "frame-unwind.h"
+#include "coff/internal.h"
+#include "coff/i386.h"
+#include "coff/pe.h"
+#include "libcoff.h"
 
 /* The registers used to pass integer arguments during a function call.  */
 static int amd64_windows_dummy_call_integer_regs[] =
@@ -198,6 +204,685 @@ amd64_windows_auto_wide_charset (void)
   return "UTF-16";
 }
 
+struct amd64_windows_frame_cache
+{
+  /* ImageBase for the module.  */
+  CORE_ADDR image_base;
+
+  /* Function start rva.  */
+  CORE_ADDR start_rva;
+
+  /* Next instruction to be executed.  */
+  CORE_ADDR pc;
+
+  /* Current sp.  */
+  CORE_ADDR sp;
+
+  /* Address of saved integer and xmm registers.  */
+  CORE_ADDR prev_reg_addr[16];
+  CORE_ADDR prev_xmm_addr[16];
+
+  /* These two next fields are set only for machine info frames.  */
+
+  /* Likewise for RIP.  */
+  CORE_ADDR prev_rip_addr;
+
+  /* Likewise for RSP.  */
+  CORE_ADDR prev_rsp_addr;
+
+  /* Address of the previous frame.  */
+  CORE_ADDR prev_sp;
+};
+
+/* Convert a Windows register number to gdb.  */
+static const enum amd64_regnum amd64_windows_w2gdb_regnum[] =
+{
+  AMD64_RAX_REGNUM,
+  AMD64_RCX_REGNUM,
+  AMD64_RDX_REGNUM,
+  AMD64_RBX_REGNUM,
+  AMD64_RSP_REGNUM,
+  AMD64_RBP_REGNUM,
+  AMD64_RSI_REGNUM,
+  AMD64_RDI_REGNUM,
+  AMD64_R8_REGNUM,
+  AMD64_R9_REGNUM,
+  AMD64_R10_REGNUM,
+  AMD64_R11_REGNUM,
+  AMD64_R12_REGNUM,
+  AMD64_R13_REGNUM,
+  AMD64_R14_REGNUM,
+  AMD64_R15_REGNUM
+};
+
+/* Try to recognize and decode an epilogue sequence.
+
+   Return -1 if we fail to read the instructions for any reason.
+   Return 1 if an epilogue sequence was recognized, 0 otherwise.  */
+
+static int
+amd64_windows_frame_decode_epilogue (struct frame_info *this_frame,
+				     struct amd64_windows_frame_cache *cache)
+{
+  /* Not in a prologue, so maybe in an epilogue.  For the rules, cf
+     http://msdn.microsoft.com/en-us/library/tawsa7cb.aspx
+     Furthermore, according to RtlVirtualUnwind, the complete list of
+     epilog marker is:
+     - ret                      [c3]
+     - ret n                    [c2 imm16]
+     - rep ret                  [f3 c3]
+     - jmp imm8 | imm32         [eb rel8] or [e9 rel32]
+     - jmp qword ptr imm32                 - not handled
+     - rex jmp reg              [4X ff eY]
+     I would add:
+     -  pop reg                 [41 58-5f] or [58-5f]
+
+     We don't care about the instruction deallocating the frame:
+     if it hasn't been executed, we can safely decode the insns;
+     if it has been executed, the following epilog decoding will
+     work.  */
+  CORE_ADDR pc = cache->pc;
+  CORE_ADDR cur_sp = cache->sp;
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+  while (1)
+    {
+      gdb_byte op;
+      gdb_byte rex;
+
+      if (target_read_memory (pc, &op, 1) != 0)
+	return -1;
+
+      if (op == 0xc3)
+	{
+	  /* Ret.  */
+	  cache->prev_rip_addr = cur_sp;
+	  cache->prev_sp = cur_sp + 8;
+	  return 1;
+	}
+      else if (op == 0xeb)
+	{
+	  /* jmp rel8  */
+	  gdb_byte rel8;
+
+	  if (target_read_memory (pc + 1, &rel8, 1) != 0)
+	    return -1;
+	  pc = pc + 2 + (signed char) rel8;
+	}
+      else if (op == 0xec)
+	{
+	  /* jmp rel32  */
+	  gdb_byte rel32[4];
+
+	  if (target_read_memory (pc + 1, rel32, 4) != 0)
+	    return -1;
+	  pc = pc + 5 + extract_signed_integer (rel32, 4, byte_order);
+	}
+      else if (op >= 0x58 && op <= 0x5f)
+	{
+	  /* pop reg  */
+	  cache->prev_reg_addr[amd64_windows_w2gdb_regnum[op & 0x0f]] = cur_sp;
+	  cur_sp += 8;
+	}
+      else if (op == 0xc2)
+	{
+	  /* ret n  */
+	  gdb_byte imm16[2];
+
+	  if (target_read_memory (pc + 1, imm16, 2) != 0)
+	    return -1;
+	  cache->prev_rip_addr = cur_sp;
+	  cache->prev_sp = cur_sp
+	    + extract_unsigned_integer (imm16, 4, byte_order);
+	  return 1;
+	}
+      else if (op == 0xf3)
+	{
+	  /* rep; ret  */
+	  gdb_byte op1;
+
+	  if (target_read_memory (pc + 2, &op1, 1) != 0)
+	    return -1;
+	  if (op1 != 0xc3)
+	    return 0;
+
+	  cache->prev_rip_addr = cur_sp;
+	  cache->prev_sp = cur_sp + 8;
+	  return 1;
+	}
+      else if (op < 0x40 || op > 0x4f)
+	{
+	  /* Not REX, so unknown.  */
+	  return 0;
+	}
+
+      /* Got a REX prefix, read next byte.  */
+      rex = op;
+      if (target_read_memory (pc + 1, &op, 1) != 0)
+	return -1;
+
+      if (op >= 0x58 && op <= 0x5f)
+	{
+	  /* pop reg  */
+	  unsigned int reg;
+
+	  reg = (op & 0x0f) | ((rex & 1) << 3);
+	  cache->prev_reg_addr[amd64_windows_w2gdb_regnum[reg]] = cur_sp;
+	  cur_sp += 8;
+	}
+      else if (op == 0xff)
+	{
+	  /* rex jmp reg  */
+	  gdb_byte op1;
+	  unsigned int reg;
+	  gdb_byte buf[8];
+
+	  if (target_read_memory (pc + 2, &op1, 1) != 0)
+	    return -1;
+	  if ((op1 & 0xf8) != 0xe0)
+	    return 0;
+	  reg = (op1 & 0x0f) | ((rex & 1) << 3);
+
+	  get_frame_register (this_frame,
+			      amd64_windows_w2gdb_regnum[reg], buf);
+	  pc = extract_unsigned_integer (buf, 8, byte_order);
+	}
+      else
+	return 0;
+
+      /* Allow the user to break this loop.  */
+      QUIT;
+    }
+}
+
+/* Decode and execute unwind insns at UNWIND_INFO.  */
+
+static void
+amd64_windows_frame_decode_insns (struct frame_info *this_frame,
+				  struct amd64_windows_frame_cache *cache,
+				  CORE_ADDR unwind_info)
+{
+  CORE_ADDR save_addr = 0;
+  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;
+
+  for (j = 0; ; j++)
+    {
+      struct external_pex64_unwind_info ex_ui;
+      /* There are at most 256 16-bit unwind insns.  */
+      gdb_byte insns[2 * 256];
+      gdb_byte *p;
+      gdb_byte *end_insns;
+      unsigned char codes_count;
+      unsigned char frame_reg;
+      unsigned char frame_off;
+
+      /* Read and decode header.  */
+      if (target_read_memory (cache->image_base + unwind_info,
+			      (gdb_byte *) &ex_ui, sizeof (ex_ui)) != 0)
+	return;
+
+      if (unwind_debug)
+	fprintf_unfiltered
+	  (gdb_stdlog,
+	   "amd64_windows_frame_play_insn: "
+	   "%s: ver: %02x, plgsz: %02x, cnt: %02x, frame: %02x\n",
+	   paddress (gdbarch, unwind_info),
+	   ex_ui.Version_Flags, ex_ui.SizeOfPrologue,
+	   ex_ui.CountOfCodes, ex_ui.FrameRegisterOffset);
+
+      /* Check version.  */
+      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) != 1)
+	return;
+
+      if (j == 0
+	  && (cache->pc >=
+	      cache->image_base + cache->start_rva + ex_ui.SizeOfPrologue))
+	{
+	  /* Not in the prologue; try to decode an epilog.  */
+	  if (amd64_windows_frame_decode_epilogue (this_frame, cache) == 1)
+	    return;
+
+	  /* Not in an epilog.  Clear possible side effects.  */
+	  memset (cache->prev_reg_addr, 0, sizeof (cache->prev_reg_addr));
+	}
+
+      codes_count = ex_ui.CountOfCodes;
+      frame_reg = PEX64_UWI_FRAMEREG (ex_ui.FrameRegisterOffset);
+
+      if (frame_reg != 0)
+	{
+	  /* According to msdn:
+	     If an FP reg is used, then any unwind code taking an offset must
+	     only be used after the FP reg is established in the prolog.  */
+	  gdb_byte buf[8];
+	  int frreg = amd64_windows_w2gdb_regnum[frame_reg];
+
+	  get_frame_register (this_frame, frreg, buf);
+	  save_addr = extract_unsigned_integer (buf, 8, byte_order);
+
+	  if (unwind_debug)
+	    fprintf_unfiltered (gdb_stdlog, "   frame_reg=%s, val=%s\n",
+				gdbarch_register_name (gdbarch, frreg),
+				paddress (gdbarch, save_addr));
+	}
+
+      /* Read opcodes.  */
+      if (codes_count != 0
+	  && target_read_memory (cache->image_base + unwind_info
+				 + sizeof (ex_ui),
+				 insns, codes_count * 2) != 0)
+	return;
+
+      end_insns = &insns[codes_count * 2];
+      for (p = insns; p < end_insns; p += 2)
+	{
+	  int reg;
+
+	  if (unwind_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])
+	    {
+	      /* If there is no frame registers defined, the current value of
+		 rsp is used instead.  */
+	      if (frame_reg == 0)
+		save_addr = cur_sp;
+
+	      switch (PEX64_UNWCODE_CODE (p[1]))
+		{
+		case UWOP_PUSH_NONVOL:
+		  /* Push pre-decrements RSP.  */
+		  reg = amd64_windows_w2gdb_regnum[PEX64_UNWCODE_INFO (p[1])];
+		  cache->prev_reg_addr[reg] = cur_sp;
+		  cur_sp += 8;
+		  break;
+		case UWOP_ALLOC_LARGE:
+		  if (PEX64_UNWCODE_INFO (p[1]) == 0)
+		    cur_sp +=
+		      8 * extract_unsigned_integer (p + 2, 2, byte_order);
+		  else if (PEX64_UNWCODE_INFO (p[1]) == 1)
+		    cur_sp += extract_unsigned_integer (p + 2, 4, byte_order);
+		  else
+		    return;
+		  break;
+		case UWOP_ALLOC_SMALL:
+		  cur_sp += 8 + 8 * PEX64_UNWCODE_INFO (p[1]);
+		  break;
+		case UWOP_SET_FPREG:
+		  cur_sp = save_addr
+		    - PEX64_UWI_FRAMEOFF (ex_ui.FrameRegisterOffset) * 16;
+		  break;
+		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);
+		  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);
+		  break;
+		case UWOP_SAVE_XMM128:
+		  cache->prev_xmm_addr[PEX64_UNWCODE_INFO (p[1])] =
+		    save_addr
+		    - 16 * extract_unsigned_integer (p + 2, 2, byte_order);
+		  break;
+		case UWOP_SAVE_XMM128_FAR:
+		  cache->prev_xmm_addr[PEX64_UNWCODE_INFO (p[1])] =
+		    save_addr
+		    - 16 * extract_unsigned_integer (p + 2, 4, byte_order);
+		  break;
+		case UWOP_PUSH_MACHFRAME:
+		  if (PEX64_UNWCODE_INFO (p[1]) == 0)
+		    {
+		      cache->prev_rip_addr = cur_sp + 0;
+		      cache->prev_rsp_addr = cur_sp + 24;
+		      cur_sp += 40;
+		    }
+		  else if (PEX64_UNWCODE_INFO (p[1]) == 1)
+		    {
+		      cache->prev_rip_addr = cur_sp + 8;
+		      cache->prev_rsp_addr = cur_sp + 32;
+		      cur_sp += 48;
+		    }
+		  else
+		    return;
+		  break;
+		default:
+		  return;
+		}
+	    }
+
+	  /* Adjust with the length of the opcode.  */
+	  switch (PEX64_UNWCODE_CODE (p[1]))
+	    {
+	    case UWOP_PUSH_NONVOL:
+	    case UWOP_ALLOC_SMALL:
+	    case UWOP_SET_FPREG:
+	    case UWOP_PUSH_MACHFRAME:
+	      break;
+	    case UWOP_ALLOC_LARGE:
+	      if (PEX64_UNWCODE_INFO (p[1]) == 0)
+		p += 2;
+	      else if (PEX64_UNWCODE_INFO (p[1]) == 1)
+		p += 4;
+	      else
+		return;
+	      break;
+	    case UWOP_SAVE_NONVOL:
+	    case UWOP_SAVE_XMM128:
+	      p += 2;
+	      break;
+	    case UWOP_SAVE_NONVOL_FAR:
+	    case UWOP_SAVE_XMM128_FAR:
+	      p += 4;
+	      break;
+	    default:
+	      return;
+	    }
+	}
+      if (PEX64_UWI_FLAGS (ex_ui.Version_Flags) != UNW_FLAG_CHAININFO)
+	break;
+      else
+	{
+	  /* Read the chained unwind info.  */
+	  struct external_pex64_runtime_function d;
+	  CORE_ADDR chain_vma;
+
+	  chain_vma = cache->image_base + unwind_info
+	    + sizeof (ex_ui) + ((codes_count + 1) & ~1) * 2 + 8;
+
+	  if (target_read_memory (chain_vma, (gdb_byte *) &d, sizeof (d)) != 0)
+	    return;
+
+	  cache->start_rva =
+	    extract_unsigned_integer (d.rva_BeginAddress, 4, byte_order);
+	  unwind_info =
+	    extract_unsigned_integer (d.rva_EndAddress, 4, byte_order);
+	}
+
+      /* Allow the user to break this loop.  */
+      QUIT;
+    }
+  /* PC is saved by the call.  */
+  if (cache->prev_rip_addr == 0)
+    cache->prev_rip_addr = cur_sp;
+  cache->prev_sp = cur_sp + 8;
+
+  if (unwind_debug)
+    fprintf_unfiltered (gdb_stdlog, "   prev_sp: %s, prev_pc @%s\n",
+			paddress (gdbarch, cache->prev_sp),
+			paddress (gdbarch, cache->prev_rip_addr));
+}
+
+/* Find SEH unwind info for PC, returning 0 on success.
+
+   UNWIND_INFO is set to the rva of unwind info address, IMAGE_BASE
+   to the base address of the corresponding image, and START_RVA
+   to the rva of the function containing PC.  */
+
+static int
+amd64_windows_find_unwind_info (struct gdbarch *gdbarch, CORE_ADDR pc,
+				CORE_ADDR *unwind_info,
+				CORE_ADDR *image_base,
+				CORE_ADDR *start_rva)
+{
+  struct obj_section *sec;
+  pe_data_type *pe;
+  IMAGE_DATA_DIRECTORY *dir;
+  struct objfile *objfile;
+  unsigned long lo, hi;
+  CORE_ADDR base;
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+  /* Get the corresponding exception directory.  */
+  sec = find_pc_section (pc);
+  if (sec == NULL)
+    return -1;
+  objfile = sec->objfile;
+  pe = pe_data (sec->objfile->obfd);
+  dir = &pe->pe_opthdr.DataDirectory[PE_EXCEPTION_TABLE];
+
+  base = pe->pe_opthdr.ImageBase
+    + ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+  *image_base = base;
+
+  /* Find the entry.
+
+     Note: This does not handle dynamically added entries (for JIT
+     engines).  For this, we would need to ask the kernel directly,
+     which means getting some info from the native layer.  For the
+     rest of the code, however, it's probably faster to search
+     the entry ourselves.  */
+  lo = 0;
+  hi = dir->Size / sizeof (struct external_pex64_runtime_function);
+  *unwind_info = 0;
+  while (lo <= hi)
+    {
+      unsigned long mid = lo + (hi - lo) / 2;
+      struct external_pex64_runtime_function d;
+      CORE_ADDR sa, ea;
+
+      if (target_read_memory (base + dir->VirtualAddress + mid * sizeof (d),
+			      (gdb_byte *) &d, sizeof (d)) != 0)
+	return -1;
+
+      sa = extract_unsigned_integer (d.rva_BeginAddress, 4, byte_order);
+      ea = extract_unsigned_integer (d.rva_EndAddress, 4, byte_order);
+      if (pc < base + sa)
+	hi = mid - 1;
+      else if (pc >= base + ea)
+	lo = mid + 1;
+      else if (pc >= base + sa && pc < base + ea)
+	{
+	  /* Got it.  */
+	  *start_rva = sa;
+	  *unwind_info =
+	    extract_unsigned_integer (d.rva_UnwindData, 4, byte_order);
+	  break;
+	}
+      else
+	break;
+    }
+
+  if (unwind_debug)
+    fprintf_unfiltered
+      (gdb_stdlog,
+       "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.  */
+      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);
+      *unwind_info =
+	extract_unsigned_integer (d.rva_EndAddress, 4, byte_order);
+    }
+  return 0;
+}
+
+/* Fill THIS_CACHE using the native amd64-windows unwinding data
+   for THIS_FRAME.  */
+
+static struct amd64_windows_frame_cache *
+amd64_windows_frame_cache (struct frame_info *this_frame, void **this_cache)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct amd64_windows_frame_cache *cache;
+  char buf[8];
+  struct obj_section *sec;
+  pe_data_type *pe;
+  IMAGE_DATA_DIRECTORY *dir;
+  CORE_ADDR image_base;
+  CORE_ADDR pc;
+  struct objfile *objfile;
+  unsigned long lo, hi;
+  CORE_ADDR unwind_info = 0;
+
+  if (*this_cache)
+    return *this_cache;
+
+  cache = FRAME_OBSTACK_ZALLOC (struct amd64_windows_frame_cache);
+  *this_cache = cache;
+
+  /* Get current PC and SP.  */
+  pc = get_frame_pc (this_frame);
+  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
+  cache->sp = extract_unsigned_integer (buf, 8, byte_order);
+  cache->pc = pc;
+
+  if (amd64_windows_find_unwind_info (gdbarch, pc, &unwind_info,
+				      &cache->image_base, &cache->start_rva))
+    return cache;
+
+  if (unwind_info == 0)
+    {
+      /* Assume a leaf function.  */
+      cache->prev_sp = cache->sp + 8;
+      cache->prev_rip_addr = cache->sp;
+    }
+  else
+    {
+      /* Decode unwind insns to compute saved addresses.  */
+      amd64_windows_frame_decode_insns (this_frame, cache, unwind_info);
+    }
+  return cache;
+}
+
+/* Implement the "prev_register" method of struct frame_unwind
+   using the standard Windows x64 SEH info.  */
+
+static struct value *
+amd64_windows_frame_prev_register (struct frame_info *this_frame,
+				   void **this_cache, int regnum)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct amd64_windows_frame_cache *cache =
+    amd64_windows_frame_cache (this_frame, this_cache);
+  struct value *val;
+  CORE_ADDR prev;
+
+  if (unwind_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"amd64_windows_frame_prev_register %s for sp=%s\n",
+			gdbarch_register_name (gdbarch, regnum),
+			paddress (gdbarch, cache->prev_sp));
+
+  if (regnum >= AMD64_XMM0_REGNUM && regnum <= AMD64_XMM0_REGNUM + 15)
+      prev = cache->prev_xmm_addr[regnum - AMD64_XMM0_REGNUM];
+  else if (regnum == AMD64_RSP_REGNUM)
+    {
+      prev = cache->prev_rsp_addr;
+      if (prev == 0)
+	return frame_unwind_got_constant (this_frame, regnum, cache->prev_sp);
+    }
+  else if (regnum >= AMD64_RAX_REGNUM && regnum <= AMD64_R15_REGNUM)
+    prev = cache->prev_reg_addr[regnum - AMD64_RAX_REGNUM];
+  else if (regnum == AMD64_RIP_REGNUM)
+    prev = cache->prev_rip_addr;
+  else
+    prev = 0;
+
+  if (prev && unwind_debug)
+    fprintf_unfiltered (gdb_stdlog, "  -> at %s\n", paddress (gdbarch, prev));
+
+  if (prev)
+    {
+      /* Register was saved.  */
+      return frame_unwind_got_memory (this_frame, regnum, prev);
+    }
+  else
+    {
+      /* Register is either volatile or not modified.  */
+      return frame_unwind_got_register (this_frame, regnum, regnum);
+    }
+}
+
+/* Implement the "this_id" method of struct frame_unwind using
+   the standard Windows x64 SEH info.  */
+
+static void
+amd64_windows_frame_this_id (struct frame_info *this_frame, void **this_cache,
+		   struct frame_id *this_id)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  struct amd64_windows_frame_cache *cache =
+    amd64_windows_frame_cache (this_frame, this_cache);
+
+  *this_id = frame_id_build (cache->prev_sp,
+			     cache->image_base + cache->start_rva);
+}
+
+/* Windows x64 SEH unwinder.  */
+
+static const struct frame_unwind amd64_windows_frame_unwind =
+{
+  NORMAL_FRAME,
+  default_frame_unwind_stop_reason,
+  &amd64_windows_frame_this_id,
+  &amd64_windows_frame_prev_register,
+  NULL,
+  default_frame_sniffer
+};
+
+/* Implement the "skip_prologue" gdbarch method.  */
+
+static CORE_ADDR
+amd64_windows_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  CORE_ADDR func_addr;
+  CORE_ADDR unwind_info = 0;
+  CORE_ADDR image_base, start_rva;
+  struct external_pex64_unwind_info ex_ui;
+
+  /* Use prologue size from unwind info.  */
+  if (amd64_windows_find_unwind_info (gdbarch, pc, &unwind_info,
+				      &image_base, &start_rva) == 0)
+    {
+      if (unwind_info == 0)
+	{
+	  /* Leaf function.  */
+	  return pc;
+	}
+      else if (target_read_memory (image_base + unwind_info,
+				   (gdb_byte *) &ex_ui, sizeof (ex_ui)) == 0
+	       && PEX64_UWI_VERSION (ex_ui.Version_Flags) == 1)
+	return max (pc, image_base + start_rva + ex_ui.SizeOfPrologue);
+    }
+
+  /* See if we can determine the end of the prologue via the symbol
+     table.  If so, then return either the PC, or the PC after
+     the prologue, whichever is greater.  */
+  if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
+    {
+      CORE_ADDR post_prologue_pc
+	= skip_prologue_using_sal (gdbarch, func_addr);
+
+      if (post_prologue_pc != 0)
+	return max (pc, post_prologue_pc);
+    }
+
+  return pc;
+}
+
 static void
 amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -225,6 +910,9 @@ amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   set_gdbarch_auto_wide_charset (gdbarch, amd64_windows_auto_wide_charset);
 
+  frame_unwind_prepend_unwinder (gdbarch, &amd64_windows_frame_unwind);
+  set_gdbarch_skip_prologue (gdbarch, amd64_windows_skip_prologue);
+
   set_solib_ops (gdbarch, &solib_target_so_ops);
 }
 
-- 
1.7.10.4

Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Pedro Alves]

On 01/09/2013 10:53 AM, Joel Brobecker wrote:

> This is the main part of the patch series, which actually
> adds the unwinder.
>
> One element worth mentioning, perhaps, is the fact that we prepend
> the unwinder, and the sniffer is the default_frame_sniffer which
> always returns 1 (absence of SEH info is normal and means it is
> a leaf function).

Meaning you want to use this unwinder for leaf functions
too, right?

>  This effectively means that all other unwinders
> effectively get shunted. And in particular, I do not think that
> the DWARF unwinder will kick in even if DWARF unwind info is found.
>
> The problem is that we want to be ahead of the default amd64-tdep
> unwinders, which is kind of a last-resort unwinder doing a good
> job under limited situations only. We'd like to be behind the DWARF
> unwinder if we could, but I don't think there is a way to ensure
> that yet.
>
> In practice, this shouldn't be a problem, since this unwinder
> has been very reliable for us so far.  But it does assume that
> the compiler is recent enough to generate native SEH data which,
> for GCC, means GCC 4.7 (I have been told). On the other hand,
> it looks like the first GCC release to support x64-windows was
> GCC 4.6.
>
> I don't really see a real way of supporting both old and new versions
> of GCC, unless we have a way of more finely ordering the unwinders.

What specific finer order where you considering would be needed to
fix this?

> Worse case scenario, we stop supporting code generated by GCC 4.6.

Yuck.

> Or an alternative option is to provide a setting to disable this
> unwinder.

That'd be my worse case scenario.  :-)

Maybe detect if the whole module (exe/dll) the PC points at
contains any SEH (even if not for PC), and skip the SEH unwinder
if not?  Is that possible?

>  
> +struct amd64_windows_frame_cache
> +{
> +  /* ImageBase for the module.  */
> +  CORE_ADDR image_base;
> +
> +  /* Function start rva.  */
> +  CORE_ADDR start_rva;
> +
> +  /* Next instruction to be executed.  */
> +  CORE_ADDR pc;
> +
> +  /* Current sp.  */
> +  CORE_ADDR sp;
> +
> +  /* Address of saved integer and xmm registers.  */
> +  CORE_ADDR prev_reg_addr[16];
> +  CORE_ADDR prev_xmm_addr[16];
> +
> +  /* These two next fields are set only for machine info frames.  */
> +
> +  /* Likewise for RIP.  */

"next two fields", and then immediately "likewise"
makes be believe there used to be two fields here that
have since been removed?

> +  CORE_ADDR prev_rip_addr;
> +
> +  /* Likewise for RSP.  */
> +  CORE_ADDR prev_rsp_addr;
> +
> +  /* Address of the previous frame.  */
> +  CORE_ADDR prev_sp;
> +};
> +



> +/* Try to recognize and decode an epilogue sequence.
> +
> +   Return -1 if we fail to read the instructions for any reason.
> +   Return 1 if an epilogue sequence was recognized, 0 otherwise.  */
> +
> +static int
> +amd64_windows_frame_decode_epilogue (struct frame_info *this_frame,
> +				     struct amd64_windows_frame_cache *cache)
> +{
> +  /* Not in a prologue, so maybe in an epilogue.  For the rules, cf

OOW, what does "cf" stand for?

> +     http://msdn.microsoft.com/en-us/library/tawsa7cb.aspx
> +     Furthermore, according to RtlVirtualUnwind, the complete list of
> +     epilog marker is:
> +     - ret                      [c3]
> +     - ret n                    [c2 imm16]
> +     - rep ret                  [f3 c3]
> +     - jmp imm8 | imm32         [eb rel8] or [e9 rel32]
> +     - jmp qword ptr imm32                 - not handled
> +     - rex jmp reg              [4X ff eY]
> +     I would add:
> +     -  pop reg                 [41 58-5f] or [58-5f]

If you add "pop", and you'd add "add" and "lea" as well,
right?  It's not super clear what "marker" means, but all
the instructions listed by RtlVirtualUnwind's docs are
control flow instructions.  And then, in the url you point
above, we see:

"These are the only legal forms for an epilog. It must consist of either
 an add RSP,constant or lea RSP,constant[FPReg], followed by a series
 of zero or more 8-byte register pops and a return or a jmp. (Only
 a subset of jmp statements are allowable in the epilog."

So from both docs it seems to me that "marker" is always the
last instruction of the epilogue, and that "pop" is not called
a marker.

Furthermore,

> +
> +     We don't care about the instruction deallocating the frame:
> +     if it hasn't been executed, we can safely decode the insns;
> +     if it has been executed, the following epilog decoding will
> +     work.  */
> +  CORE_ADDR pc = cache->pc;
> +  CORE_ADDR cur_sp = cache->sp;
> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +
> +  while (1)
> +    {
> +      gdb_byte op;
> +      gdb_byte rex;
> +
> +      if (target_read_memory (pc, &op, 1) != 0)
> +	return -1;
> +
> +      if (op == 0xc3)
> +	{
> +	  /* Ret.  */
> +	  cache->prev_rip_addr = cur_sp;
> +	  cache->prev_sp = cur_sp + 8;
> +	  return 1;
> +	}
> +      else if (op == 0xeb)
> +	{
> +	  /* jmp rel8  */
> +	  gdb_byte rel8;
> +
> +	  if (target_read_memory (pc + 1, &rel8, 1) != 0)
> +	    return -1;
> +	  pc = pc + 2 + (signed char) rel8;

this implementation follows jumps, and keeps looping
at the jump destination.  I see no hint that such thing
as an epilogue with a jump is a valid epilogue, only that
a jmp is only valid as replacement for ret.
Can you show an example where following the jmp until
you see a ret is necessary?


> +/* Decode and execute unwind insns at UNWIND_INFO.  */
> +
> +static void
> +amd64_windows_frame_decode_insns (struct frame_info *this_frame,
> +				  struct amd64_windows_frame_cache *cache,
> +				  CORE_ADDR unwind_info)
> +{
...

> +      if (unwind_debug)
> +	fprintf_unfiltered
> +	  (gdb_stdlog,
> +	   "amd64_windows_frame_play_insn: "

Is this supposed to be a function name?  If so, it's already
out of sync.

> +	   "%s: ver: %02x, plgsz: %02x, cnt: %02x, frame: %02x\n",
> +	   paddress (gdbarch, unwind_info),
> +	   ex_ui.Version_Flags, ex_ui.SizeOfPrologue,
> +	   ex_ui.CountOfCodes, ex_ui.FrameRegisterOffset);



> +	      switch (PEX64_UNWCODE_CODE (p[1]))
> +		{
> +		case UWOP_PUSH_NONVOL:
> +		  /* Push pre-decrements RSP.  */
> +		  reg = amd64_windows_w2gdb_regnum[PEX64_UNWCODE_INFO (p[1])];
> +		  cache->prev_reg_addr[reg] = cur_sp;
> +		  cur_sp += 8;
> +		  break;
> +		case UWOP_ALLOC_LARGE:
> +		  if (PEX64_UNWCODE_INFO (p[1]) == 0)
> +		    cur_sp +=

Operator goes on next line (multiple instances in the patch,
including with '=').

> +		      8 * extract_unsigned_integer (p + 2, 2, byte_order);
> +		  else if (PEX64_UNWCODE_INFO (p[1]) == 1)
> +		    cur_sp += extract_unsigned_integer (p + 2, 4, byte_order);
> +		  else
> +		    return;
> +		  break;

> +
> +  if (unwind_debug)
> +    fprintf_unfiltered
> +      (gdb_stdlog,
> +       "amd64_windows_find_unwind_data:  image_base=%s, unwind_data=%s\n",
> +       paddress (gdbarch, base), paddress (gdbarch, *unwind_info));

Another stale function name.

> +
> +  if (*unwind_info & 1)
> +    {
> +      /* Unofficially documented unwind info redirection, when UNWIND_INFO
> +	 address is odd.  */

What's "unofficially documented"?  Documented in some
blog?

> +      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);
> +      *unwind_info =
> +	extract_unsigned_integer (d.rva_EndAddress, 4, byte_order);
> +    }
> +  return 0;
> +}
> +
> +/* Fill THIS_CACHE using the native amd64-windows unwinding data
> +   for THIS_FRAME.  */
> +
> +static struct amd64_windows_frame_cache *
> +amd64_windows_frame_cache (struct frame_info *this_frame, void **this_cache)
> +{
> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  struct amd64_windows_frame_cache *cache;
> +  char buf[8];

gdb_byte.


> +static struct value *
> +amd64_windows_frame_prev_register (struct frame_info *this_frame,
> +				   void **this_cache, int regnum)
> +{
> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  struct amd64_windows_frame_cache *cache =
> +    amd64_windows_frame_cache (this_frame, this_cache);
> +  struct value *val;
> +  CORE_ADDR prev;
> +
> +  if (unwind_debug)
> +    fprintf_unfiltered (gdb_stdlog,
> +			"amd64_windows_frame_prev_register %s for sp=%s\n",
> +			gdbarch_register_name (gdbarch, regnum),
> +			paddress (gdbarch, cache->prev_sp));
> +
> +  if (regnum >= AMD64_XMM0_REGNUM && regnum <= AMD64_XMM0_REGNUM + 15)
> +      prev = cache->prev_xmm_addr[regnum - AMD64_XMM0_REGNUM];

Indentation looks odd.

> +  else if (regnum == AMD64_RSP_REGNUM)
> +    {
> +      prev = cache->prev_rsp_addr;
> +      if (prev == 0)
> +	return frame_unwind_got_constant (this_frame, regnum, cache->prev_sp);
> +    }
> +  else if (regnum >= AMD64_RAX_REGNUM && regnum <= AMD64_R15_REGNUM)
> +    prev = cache->prev_reg_addr[regnum - AMD64_RAX_REGNUM];
> +  else if (regnum == AMD64_RIP_REGNUM)
> +    prev = cache->prev_rip_addr;
> +  else
> +    prev = 0;
> +
> +  if (prev && unwind_debug)
> +    fprintf_unfiltered (gdb_stdlog, "  -> at %s\n", paddress (gdbarch, prev));
> +
> +  if (prev)

('prev' is not really a boolean.  I'd prefer 'prev != 0'
in these cases.)


> +/* Implement the "skip_prologue" gdbarch method.  */
> +
> +static CORE_ADDR
> +amd64_windows_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
> +{
> +  CORE_ADDR func_addr;
> +  CORE_ADDR unwind_info = 0;
> +  CORE_ADDR image_base, start_rva;
> +  struct external_pex64_unwind_info ex_ui;

ex_ui could move to the inner scope.

> +
> +  /* Use prologue size from unwind info.  */
> +  if (amd64_windows_find_unwind_info (gdbarch, pc, &unwind_info,
> +				      &image_base, &start_rva) == 0)
> +    {
> +      if (unwind_info == 0)
> +	{
> +	  /* Leaf function.  */
> +	  return pc;
> +	}
> +      else if (target_read_memory (image_base + unwind_info,
> +				   (gdb_byte *) &ex_ui, sizeof (ex_ui)) == 0
> +	       && PEX64_UWI_VERSION (ex_ui.Version_Flags) == 1)
> +	return max (pc, image_base + start_rva + ex_ui.SizeOfPrologue);
> +    }
> +
> +  /* See if we can determine the end of the prologue via the symbol
> +     table.  If so, then return either the PC, or the PC after
> +     the prologue, whichever is greater.  */
> +  if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
> +    {
> +      CORE_ADDR post_prologue_pc
> +	= skip_prologue_using_sal (gdbarch, func_addr);
> +
> +      if (post_prologue_pc != 0)
> +	return max (pc, post_prologue_pc);
> +    }
> +
> +  return pc;
> +}
> +
>  static void
>  amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  {
> @@ -225,6 +910,9 @@ amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  
>    set_gdbarch_auto_wide_charset (gdbarch, amd64_windows_auto_wide_charset);
>  
> +  frame_unwind_prepend_unwinder (gdbarch, &amd64_windows_frame_unwind);

I think a comment here referring to issues you
mentioned in the mail would be good.

> +  set_gdbarch_skip_prologue (gdbarch, amd64_windows_skip_prologue);
> +
>    set_solib_ops (gdbarch, &solib_target_so_ops);
>  }

-- 
Pedro Alves

Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Tristan Gingold]

Partial answer:

On Jan 9, 2013, at 4:52 PM, Pedro Alves wrote:

> On 01/09/2013 10:53 AM, Joel Brobecker wrote:
> 
>> This is the main part of the patch series, which actually
>> adds the unwinder.
>> 
>> One element worth mentioning, perhaps, is the fact that we prepend
>> the unwinder, and the sniffer is the default_frame_sniffer which
>> always returns 1 (absence of SEH info is normal and means it is
>> a leaf function).
> 
> Meaning you want to use this unwinder for leaf functions
> too, right?

Yes, and we need that. Unwinding lead functions is defined and handled
by this unwinder.

>> This effectively means that all other unwinders
>> effectively get shunted. And in particular, I do not think that
>> the DWARF unwinder will kick in even if DWARF unwind info is found.
>> 
>> The problem is that we want to be ahead of the default amd64-tdep
>> unwinders, which is kind of a last-resort unwinder doing a good
>> job under limited situations only. We'd like to be behind the DWARF
>> unwinder if we could, but I don't think there is a way to ensure
>> that yet.
>> 
>> In practice, this shouldn't be a problem, since this unwinder
>> has been very reliable for us so far.  But it does assume that
>> the compiler is recent enough to generate native SEH data which,
>> for GCC, means GCC 4.7 (I have been told). On the other hand,
>> it looks like the first GCC release to support x64-windows was
>> GCC 4.6.
>> 
>> I don't really see a real way of supporting both old and new versions
>> of GCC, unless we have a way of more finely ordering the unwinders.
> 
> What specific finer order where you considering would be needed to
> fix this?

Joel once proposed to activate this unwinder if the CU is compiled
by gcc 4.6 or older.

>> Worse case scenario, we stop supporting code generated by GCC 4.6.
> 
> Yuck.
> 
>> Or an alternative option is to provide a setting to disable this
>> unwinder.
> 
> That'd be my worse case scenario.  :-)
> 
> Maybe detect if the whole module (exe/dll) the PC points at
> contains any SEH (even if not for PC), and skip the SEH unwinder
> if not?  Is that possible?

Yes, but useless.  There are SEH info in crt0.

>> +struct amd64_windows_frame_cache
>> +{
>> +  /* ImageBase for the module.  */
>> +  CORE_ADDR image_base;
>> +
>> +  /* Function start rva.  */
>> +  CORE_ADDR start_rva;
>> +
>> +  /* Next instruction to be executed.  */
>> +  CORE_ADDR pc;
>> +
>> +  /* Current sp.  */
>> +  CORE_ADDR sp;
>> +
>> +  /* Address of saved integer and xmm registers.  */
>> +  CORE_ADDR prev_reg_addr[16];
>> +  CORE_ADDR prev_xmm_addr[16];
>> +
>> +  /* These two next fields are set only for machine info frames.  */
>> +
>> +  /* Likewise for RIP.  */
> 
> "next two fields", and then immediately "likewise"
> makes be believe there used to be two fields here that
> have since been removed?

You're correct.  That concerns only prev_rsp_addr.

>> +  CORE_ADDR prev_rip_addr;
>> +
>> +  /* Likewise for RSP.  */
>> +  CORE_ADDR prev_rsp_addr;
>> +
>> +  /* Address of the previous frame.  */
>> +  CORE_ADDR prev_sp;
>> +};
>> +
> 
> 
> 
>> +/* Try to recognize and decode an epilogue sequence.
>> +
>> +   Return -1 if we fail to read the instructions for any reason.
>> +   Return 1 if an epilogue sequence was recognized, 0 otherwise.  */
>> +
>> +static int
>> +amd64_windows_frame_decode_epilogue (struct frame_info *this_frame,
>> +				     struct amd64_windows_frame_cache *cache)
>> +{
>> +  /* Not in a prologue, so maybe in an epilogue.  For the rules, cf
> 
> OOW, what does "cf" stand for?

Confere.

>> +     http://msdn.microsoft.com/en-us/library/tawsa7cb.aspx
>> +     Furthermore, according to RtlVirtualUnwind, the complete list of
>> +     epilog marker is:
>> +     - ret                      [c3]
>> +     - ret n                    [c2 imm16]
>> +     - rep ret                  [f3 c3]
>> +     - jmp imm8 | imm32         [eb rel8] or [e9 rel32]
>> +     - jmp qword ptr imm32                 - not handled
>> +     - rex jmp reg              [4X ff eY]
>> +     I would add:
>> +     -  pop reg                 [41 58-5f] or [58-5f]
> 
> If you add "pop", and you'd add "add" and "lea" as well,
> right?  It's not super clear what "marker" means, but all
> the instructions listed by RtlVirtualUnwind's docs are
> control flow instructions.  And then, in the url you point
> above, we see:
> 
> "These are the only legal forms for an epilog. It must consist of either
> an add RSP,constant or lea RSP,constant[FPReg], followed by a series
> of zero or more 8-byte register pops and a return or a jmp. (Only
> a subset of jmp statements are allowable in the epilog."
> 
> So from both docs it seems to me that "marker" is always the
> last instruction of the epilogue, and that "pop" is not called
> a marker.

This is in fact an optimization. If we found a pop, followed by
an epilog marker, there is not need to decode unwind info.

> Furthermore,
> 
>> +
>> +     We don't care about the instruction deallocating the frame:
>> +     if it hasn't been executed, we can safely decode the insns;
>> +     if it has been executed, the following epilog decoding will
>> +     work.  */
>> +  CORE_ADDR pc = cache->pc;
>> +  CORE_ADDR cur_sp = cache->sp;
>> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> +
>> +  while (1)
>> +    {
>> +      gdb_byte op;
>> +      gdb_byte rex;
>> +
>> +      if (target_read_memory (pc, &op, 1) != 0)
>> +	return -1;
>> +
>> +      if (op == 0xc3)
>> +	{
>> +	  /* Ret.  */
>> +	  cache->prev_rip_addr = cur_sp;
>> +	  cache->prev_sp = cur_sp + 8;
>> +	  return 1;
>> +	}
>> +      else if (op == 0xeb)
>> +	{
>> +	  /* jmp rel8  */
>> +	  gdb_byte rel8;
>> +
>> +	  if (target_read_memory (pc + 1, &rel8, 1) != 0)
>> +	    return -1;
>> +	  pc = pc + 2 + (signed char) rel8;
> 
> this implementation follows jumps, and keeps looping
> at the jump destination.

Right.

>  I see no hint that such thing
> as an epilogue with a jump is a valid epilogue, only that
> a jmp is only valid as replacement for ret.
> Can you show an example where following the jmp until
> you see a ret is necessary?

Interesting remark. First attempt to answer is the case of a
jump to an epilogue.
But I may miss your point.


>> +/* Decode and execute unwind insns at UNWIND_INFO.  */
>> +
>> +static void
>> +amd64_windows_frame_decode_insns (struct frame_info *this_frame,
>> +				  struct amd64_windows_frame_cache *cache,
>> +				  CORE_ADDR unwind_info)
>> +{
> ...
> 
>> +      if (unwind_debug)
>> +	fprintf_unfiltered
>> +	  (gdb_stdlog,
>> +	   "amd64_windows_frame_play_insn: "
> 
> Is this supposed to be a function name?  If so, it's already
> out of sync.

Correct.

>> +	   "%s: ver: %02x, plgsz: %02x, cnt: %02x, frame: %02x\n",
>> +	   paddress (gdbarch, unwind_info),
>> +	   ex_ui.Version_Flags, ex_ui.SizeOfPrologue,
>> +	   ex_ui.CountOfCodes, ex_ui.FrameRegisterOffset);
> 
> 
> 
>> +	      switch (PEX64_UNWCODE_CODE (p[1]))
>> +		{
>> +		case UWOP_PUSH_NONVOL:
>> +		  /* Push pre-decrements RSP.  */
>> +		  reg = amd64_windows_w2gdb_regnum[PEX64_UNWCODE_INFO (p[1])];
>> +		  cache->prev_reg_addr[reg] = cur_sp;
>> +		  cur_sp += 8;
>> +		  break;
>> +		case UWOP_ALLOC_LARGE:
>> +		  if (PEX64_UNWCODE_INFO (p[1]) == 0)
>> +		    cur_sp +=
> 
> Operator goes on next line (multiple instances in the patch,
> including with '=').
> 
>> +		      8 * extract_unsigned_integer (p + 2, 2, byte_order);
>> +		  else if (PEX64_UNWCODE_INFO (p[1]) == 1)
>> +		    cur_sp += extract_unsigned_integer (p + 2, 4, byte_order);
>> +		  else
>> +		    return;
>> +		  break;
> 
>> +
>> +  if (unwind_debug)
>> +    fprintf_unfiltered
>> +      (gdb_stdlog,
>> +       "amd64_windows_find_unwind_data:  image_base=%s, unwind_data=%s\n",
>> +       paddress (gdbarch, base), paddress (gdbarch, *unwind_info));
> 
> Another stale function name.
> 
>> +
>> +  if (*unwind_info & 1)
>> +    {
>> +      /* Unofficially documented unwind info redirection, when UNWIND_INFO
>> +	 address is odd.  */
> 
> What's "unofficially documented"?  Documented in some
> blog?

Yes, or some papers not coming from MS.

> 
>> +      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);
>> +      *unwind_info =
>> +	extract_unsigned_integer (d.rva_EndAddress, 4, byte_order);
>> +    }
>> +  return 0;
>> +}
>> +
>> +/* Fill THIS_CACHE using the native amd64-windows unwinding data
>> +   for THIS_FRAME.  */
>> +
>> +static struct amd64_windows_frame_cache *
>> +amd64_windows_frame_cache (struct frame_info *this_frame, void **this_cache)
>> +{
>> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> +  struct amd64_windows_frame_cache *cache;
>> +  char buf[8];
> 
> gdb_byte.
> 
> 
>> +static struct value *
>> +amd64_windows_frame_prev_register (struct frame_info *this_frame,
>> +				   void **this_cache, int regnum)
>> +{
>> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> +  struct amd64_windows_frame_cache *cache =
>> +    amd64_windows_frame_cache (this_frame, this_cache);
>> +  struct value *val;
>> +  CORE_ADDR prev;
>> +
>> +  if (unwind_debug)
>> +    fprintf_unfiltered (gdb_stdlog,
>> +			"amd64_windows_frame_prev_register %s for sp=%s\n",
>> +			gdbarch_register_name (gdbarch, regnum),
>> +			paddress (gdbarch, cache->prev_sp));
>> +
>> +  if (regnum >= AMD64_XMM0_REGNUM && regnum <= AMD64_XMM0_REGNUM + 15)
>> +      prev = cache->prev_xmm_addr[regnum - AMD64_XMM0_REGNUM];
> 
> Indentation looks odd.
> 
>> +  else if (regnum == AMD64_RSP_REGNUM)
>> +    {
>> +      prev = cache->prev_rsp_addr;
>> +      if (prev == 0)
>> +	return frame_unwind_got_constant (this_frame, regnum, cache->prev_sp);
>> +    }
>> +  else if (regnum >= AMD64_RAX_REGNUM && regnum <= AMD64_R15_REGNUM)
>> +    prev = cache->prev_reg_addr[regnum - AMD64_RAX_REGNUM];
>> +  else if (regnum == AMD64_RIP_REGNUM)
>> +    prev = cache->prev_rip_addr;
>> +  else
>> +    prev = 0;
>> +
>> +  if (prev && unwind_debug)
>> +    fprintf_unfiltered (gdb_stdlog, "  -> at %s\n", paddress (gdbarch, prev));
>> +
>> +  if (prev)
> 
> ('prev' is not really a boolean.  I'd prefer 'prev != 0'
> in these cases.)
> 
> 
>> +/* Implement the "skip_prologue" gdbarch method.  */
>> +
>> +static CORE_ADDR
>> +amd64_windows_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
>> +{
>> +  CORE_ADDR func_addr;
>> +  CORE_ADDR unwind_info = 0;
>> +  CORE_ADDR image_base, start_rva;
>> +  struct external_pex64_unwind_info ex_ui;
> 
> ex_ui could move to the inner scope.
> 
>> +
>> +  /* Use prologue size from unwind info.  */
>> +  if (amd64_windows_find_unwind_info (gdbarch, pc, &unwind_info,
>> +				      &image_base, &start_rva) == 0)
>> +    {
>> +      if (unwind_info == 0)
>> +	{
>> +	  /* Leaf function.  */
>> +	  return pc;
>> +	}
>> +      else if (target_read_memory (image_base + unwind_info,
>> +				   (gdb_byte *) &ex_ui, sizeof (ex_ui)) == 0
>> +	       && PEX64_UWI_VERSION (ex_ui.Version_Flags) == 1)
>> +	return max (pc, image_base + start_rva + ex_ui.SizeOfPrologue);
>> +    }
>> +
>> +  /* See if we can determine the end of the prologue via the symbol
>> +     table.  If so, then return either the PC, or the PC after
>> +     the prologue, whichever is greater.  */
>> +  if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
>> +    {
>> +      CORE_ADDR post_prologue_pc
>> +	= skip_prologue_using_sal (gdbarch, func_addr);
>> +
>> +      if (post_prologue_pc != 0)
>> +	return max (pc, post_prologue_pc);
>> +    }
>> +
>> +  return pc;
>> +}
>> +
>> static void
>> amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> {
>> @@ -225,6 +910,9 @@ amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> 
>>   set_gdbarch_auto_wide_charset (gdbarch, amd64_windows_auto_wide_charset);
>> 
>> +  frame_unwind_prepend_unwinder (gdbarch, &amd64_windows_frame_unwind);
> 
> I think a comment here referring to issues you
> mentioned in the mail would be good.
> 
>> +  set_gdbarch_skip_prologue (gdbarch, amd64_windows_skip_prologue);
>> +
>>   set_solib_ops (gdbarch, &solib_target_so_ops);
>> }
> 
> -- 
> Pedro Alves

Tristan.

Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Pedro Alves]

On 01/09/2013 04:28 PM, Tristan Gingold wrote:

>>> I don't really see a real way of supporting both old and new versions
>>> of GCC, unless we have a way of more finely ordering the unwinders.
>>
>> What specific finer order where you considering would be needed to
>> fix this?
> 
> Joel once proposed to activate this unwinder if the CU is compiled
> by gcc 4.6 or older.

I don't think you need to have a way of more finely ordering
the unwinders for that.  AFAICS, we can make the sniffer
return false in that case.  I had understood him
as meaning something about making the whole prepend/append
mechanisms more finer grained somehow.

Checking the CU/producer is kind of the obvious choice.
It requires debug info though.

>> Maybe detect if the whole module (exe/dll) the PC points at
>> contains any SEH (even if not for PC), and skip the SEH unwinder
>> if not?  Is that possible?
> 
> Yes, but useless.  There are SEH info in crt0.

You mean, the parts that are in mingw, right?  I'd assume
any bits in gcc to only contain SEH in >=4.7.  Yeah,
seems like that wouldn't work for static binaries.

>>> +     http://msdn.microsoft.com/en-us/library/tawsa7cb.aspx
>>> +     Furthermore, according to RtlVirtualUnwind, the complete list of
>>> +     epilog marker is:
>>> +     - ret                      [c3]
>>> +     - ret n                    [c2 imm16]
>>> +     - rep ret                  [f3 c3]
>>> +     - jmp imm8 | imm32         [eb rel8] or [e9 rel32]
>>> +     - jmp qword ptr imm32                 - not handled
>>> +     - rex jmp reg              [4X ff eY]
>>> +     I would add:
>>> +     -  pop reg                 [41 58-5f] or [58-5f]
>>
>> If you add "pop", and you'd add "add" and "lea" as well,
>> right?  It's not super clear what "marker" means, but all
>> the instructions listed by RtlVirtualUnwind's docs are
>> control flow instructions.  And then, in the url you point
>> above, we see:
>>
>> "These are the only legal forms for an epilog. It must consist of either
>> an add RSP,constant or lea RSP,constant[FPReg], followed by a series
>> of zero or more 8-byte register pops and a return or a jmp. (Only
>> a subset of jmp statements are allowable in the epilog."
>>
>> So from both docs it seems to me that "marker" is always the
>> last instruction of the epilogue, and that "pop" is not called
>> a marker.
> 
> This is in fact an optimization. If we found a pop, followed by
> an epilog marker, there is not need to decode unwind info.

I don't understand.  pops will always be followed by a marker.
How can that be an optimization?

> 
>> Furthermore,
>>
>>> +
>>> +     We don't care about the instruction deallocating the frame:
>>> +     if it hasn't been executed, we can safely decode the insns;
>>> +     if it has been executed, the following epilog decoding will
>>> +     work.  */
>>> +  CORE_ADDR pc = cache->pc;
>>> +  CORE_ADDR cur_sp = cache->sp;
>>> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
>>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>> +
>>> +  while (1)
>>> +    {
>>> +      gdb_byte op;
>>> +      gdb_byte rex;
>>> +
>>> +      if (target_read_memory (pc, &op, 1) != 0)
>>> +	return -1;
>>> +
>>> +      if (op == 0xc3)
>>> +	{
>>> +	  /* Ret.  */
>>> +	  cache->prev_rip_addr = cur_sp;
>>> +	  cache->prev_sp = cur_sp + 8;
>>> +	  return 1;
>>> +	}
>>> +      else if (op == 0xeb)
>>> +	{
>>> +	  /* jmp rel8  */
>>> +	  gdb_byte rel8;
>>> +
>>> +	  if (target_read_memory (pc + 1, &rel8, 1) != 0)
>>> +	    return -1;
>>> +	  pc = pc + 2 + (signed char) rel8;
>>
>> this implementation follows jumps, and keeps looping
>> at the jump destination.
> 
> Right.
> 
>>  I see no hint that such thing
>> as an epilogue with a jump is a valid epilogue, only that
>> a jmp is only valid as replacement for ret.
>> Can you show an example where following the jmp until
>> you see a ret is necessary?
> 
> Interesting remark. First attempt to answer is the case of a
> jump to an epilogue.

The jump to an epilogue would not be part of the epilogue.

> But I may miss your point.

My point is that the docs say the epilogue has this rigid
format that always ends in a marker, and that a marker is
a ret or a jmp (therefore calling "pop" a marker as in the
"I would add" comment seems to me misleading).  The code
continues following the jmp, so it makes me believe the code
is erroneously decoding something after the jmp that is
not an epilogue (the caller or perhaps a tailcall).

-- 
Pedro Alves

Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I don't think you need to have a way of more finely ordering
Pedro> the unwinders for that.  AFAICS, we can make the sniffer
Pedro> return false in that case.  I had understood him
Pedro> as meaning something about making the whole prepend/append
Pedro> mechanisms more finer grained somehow.

FWIW I think Joel explained it in the original post.

My understanding based on that is that the absence of SEH is normal, but
they'd still like to use this unwinder for such frames, because
amd64-tdep.c provides a catch-all unwinder (I guess amd64_frame_unwind)
that is not always good enough.

It seems to me that there are various possibilities for fixing the
problem though.  For example, the new unwinder could be split into two
parts: one which checks for SEH and one which does not; then arrange
somehow for the checking variant to come before the DWARF unwinders, and
arrange for the non-checking one to come later; and perhaps change
amd64_init_abi so that the Windows code can request that the
unwinder-of-last-resort not be installed.

I don't know the mechanics of arranging the ordering with the DWARF
unwinders.  I couldn't actually figure out how these are installed for
x86-64.

Tom

Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Pedro Alves]

On 01/09/2013 05:20 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I don't think you need to have a way of more finely ordering
> Pedro> the unwinders for that.  AFAICS, we can make the sniffer
> Pedro> return false in that case.  I had understood him
> Pedro> as meaning something about making the whole prepend/append
> Pedro> mechanisms more finer grained somehow.
> 
> FWIW I think Joel explained it in the original post.

Thanks.  I hadn't caught the desire to put bits in before the
dwarf unwinder, and other bits after.  I re-read the
original post, and now I read it as always wanting this
unwinder after the DWARF unwinder.  (note: "behind" is
ambiguous to me).  That looks doable with the current
architecture, without splitting the new unwinder, by
appending the non-checking unwinder in amd64_windows_init_abi
before calling amd64_init_abi, and have its sniffer always
claim the frame (which it would anyway), so the fallback
heuristic's sniffer never gets a chance to run.  But I'd
guess before or after dwarf doesn't really matter.

> My understanding based on that is that the absence of SEH is normal, but
> they'd still like to use this unwinder for such frames, because
> amd64-tdep.c provides a catch-all unwinder (I guess amd64_frame_unwind)
> that is not always good enough.

Right.  The catch-all unwinders are heuristic, and naturally can't
always work 100% correctly, worse on non-x64 targets,
where we don't have as stiff prologue format requirements.

I got confused with the minimal leaf function handling in the
patch, but this, coupled with __fastcall makes it clearer:

<http://www.sciencezero.org/index.php?title=How_to_write_x64_assembly_functions_in_Visual_C%2B%2B#Leaf_or_frame_function>

"
 leaf functions have limitations:

    Can not call out to other functions
    Can not change any non-volatile registers
    Can not change the stack pointer
"

(It goes without saying, but FAOD, I'd prefer that
explanations to my doubts ended up as comments in the code.)

> I don't know the mechanics of arranging the ordering with the DWARF
> unwinders.  I couldn't actually figure out how these are installed for
> x86-64.

Yeah, it's not obvious.  Put a break on dwarf2_append_unwinders.
The x86-64 gdbarch initialization starts in i386-tdep.c:i386_gdbarch_init,
shared with 32-bit.  There's no amd64_gdbarch_init.  That is what
appends the dwarf unwinders, with the dwarf2_append_unwinders call.
At the bottom, this/a gdbarch init function initializes the osabi (e.g.,
amd64_linux_init_abi), and this is what calls the generic
x86-64 amd64_init_abi early on, and installs the prologue-based
fallback unwinders.

-- 
Pedro Alves

Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Tristan Gingold]

On Jan 9, 2013, at 6:10 PM, Pedro Alves wrote:

> On 01/09/2013 04:28 PM, Tristan Gingold wrote:
> 
>>>> I don't really see a real way of supporting both old and new versions
>>>> of GCC, unless we have a way of more finely ordering the unwinders.
>>> 
>>> What specific finer order where you considering would be needed to
>>> fix this?
>> 
>> Joel once proposed to activate this unwinder if the CU is compiled
>> by gcc 4.6 or older.
> 
> I don't think you need to have a way of more finely ordering
> the unwinders for that.  AFAICS, we can make the sniffer
> return false in that case.  I had understood him
> as meaning something about making the whole prepend/append
> mechanisms more finer grained somehow.
> 
> Checking the CU/producer is kind of the obvious choice.
> It requires debug info though.

Yes, but without debug info, unwinding doesn't work much better.
AFAIK, zero-cost exception was not supported on Windows64.

>>> Maybe detect if the whole module (exe/dll) the PC points at
>>> contains any SEH (even if not for PC), and skip the SEH unwinder
>>> if not?  Is that possible?
>> 
>> Yes, but useless.  There are SEH info in crt0.
> 
> You mean, the parts that are in mingw, right?  I'd assume
> any bits in gcc to only contain SEH in >=4.7.

No, there is some SEH entries manually created.

>  Yeah,
> seems like that wouldn't work for static binaries.
> 
>>>> +     http://msdn.microsoft.com/en-us/library/tawsa7cb.aspx
>>>> +     Furthermore, according to RtlVirtualUnwind, the complete list of
>>>> +     epilog marker is:
>>>> +     - ret                      [c3]
>>>> +     - ret n                    [c2 imm16]
>>>> +     - rep ret                  [f3 c3]
>>>> +     - jmp imm8 | imm32         [eb rel8] or [e9 rel32]
>>>> +     - jmp qword ptr imm32                 - not handled
>>>> +     - rex jmp reg              [4X ff eY]
>>>> +     I would add:
>>>> +     -  pop reg                 [41 58-5f] or [58-5f]
>>> 
>>> If you add "pop", and you'd add "add" and "lea" as well,
>>> right?  It's not super clear what "marker" means, but all
>>> the instructions listed by RtlVirtualUnwind's docs are
>>> control flow instructions.  And then, in the url you point
>>> above, we see:
>>> 
>>> "These are the only legal forms for an epilog. It must consist of either
>>> an add RSP,constant or lea RSP,constant[FPReg], followed by a series
>>> of zero or more 8-byte register pops and a return or a jmp. (Only
>>> a subset of jmp statements are allowable in the epilog."
>>> 
>>> So from both docs it seems to me that "marker" is always the
>>> last instruction of the epilogue, and that "pop" is not called
>>> a marker.
>> 
>> This is in fact an optimization. If we found a pop, followed by
>> an epilog marker, there is not need to decode unwind info.
> 
> I don't understand.  pops will always be followed by a marker.
> How can that be an optimization?

If pop weren't in this list, then if pc points to a pop the unwinder
will consider that the pc is in the body and need to decode unwind
infos.  Now, if pop is in the list, the unwinder will continue to decode
until a ret and if a ret is found, it will consider that the pc is in
the epilogue, avoiding decoding unwind infos.

>>> Furthermore,
>>> 
>>>> +
>>>> +     We don't care about the instruction deallocating the frame:
>>>> +     if it hasn't been executed, we can safely decode the insns;
>>>> +     if it has been executed, the following epilog decoding will
>>>> +     work.  */
>>>> +  CORE_ADDR pc = cache->pc;
>>>> +  CORE_ADDR cur_sp = cache->sp;
>>>> +  struct gdbarch *gdbarch = get_frame_arch (this_frame);
>>>> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>>> +
>>>> +  while (1)
>>>> +    {
>>>> +      gdb_byte op;
>>>> +      gdb_byte rex;
>>>> +
>>>> +      if (target_read_memory (pc, &op, 1) != 0)
>>>> +	return -1;
>>>> +
>>>> +      if (op == 0xc3)
>>>> +	{
>>>> +	  /* Ret.  */
>>>> +	  cache->prev_rip_addr = cur_sp;
>>>> +	  cache->prev_sp = cur_sp + 8;
>>>> +	  return 1;
>>>> +	}
>>>> +      else if (op == 0xeb)
>>>> +	{
>>>> +	  /* jmp rel8  */
>>>> +	  gdb_byte rel8;
>>>> +
>>>> +	  if (target_read_memory (pc + 1, &rel8, 1) != 0)
>>>> +	    return -1;
>>>> +	  pc = pc + 2 + (signed char) rel8;
>>> 
>>> this implementation follows jumps, and keeps looping
>>> at the jump destination.
>> 
>> Right.
>> 
>>> I see no hint that such thing
>>> as an epilogue with a jump is a valid epilogue, only that
>>> a jmp is only valid as replacement for ret.
>>> Can you show an example where following the jmp until
>>> you see a ret is necessary?
>> 
>> Interesting remark. First attempt to answer is the case of a
>> jump to an epilogue.
> 
> The jump to an epilogue would not be part of the epilogue.

Doesn't really matter. The point is that if the current instruction
is a jump to the epilogue, there is no need to decode unwind infs.

>> But I may miss your point.
> 
> My point is that the docs say the epilogue has this rigid
> format that always ends in a marker, and that a marker is
> a ret or a jmp (therefore calling "pop" a marker as in the
> "I would add" comment seems to me misleading).  The code
> continues following the jmp, so it makes me believe the code
> is erroneously decoding something after the jmp that is
> not an epilogue (the caller or perhaps a tailcall).

No, it doesn't say that it ends in a marker.  The epilogue
ends by a ret.  But the above instructions are considered to
be part of the epilogue.

Tristan.

Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Pedro Alves]

On 01/09/2013 08:07 PM, Tristan Gingold wrote:

>>> This is in fact an optimization. If we found a pop, followed by
>>> an epilog marker, there is not need to decode unwind info.
>>
>> I don't understand.  pops will always be followed by a marker.
>> How can that be an optimization?
> 
> If pop weren't in this list, then if pc points to a pop the unwinder
> will consider that the pc is in the body and need to decode unwind
> infos.  Now, if pop is in the list, the unwinder will continue to decode
> until a ret and if a ret is found, it will consider that the pc is in
> the epilogue, avoiding decoding unwind infos.

(and again for quotability:)

> If pop weren't in this list,

I think you meant, that if pops were not handled in the
while loop.  While I'm saying that the comment implies
that pop is an epilog marker, and claiming it's not,
therefore the comment is wrong and misleading.  It should
be clarified/extended.

But let me see if I understood, by trying to rewrite
and extend your reply into something that I'd find clearer:

 We want to detect if the PC points to an epilogue (entry or midway).
 If so, the following epilogue detection+decoding below is
 sufficient.  Otherwise, the unwinder will consider that the PC
 is in the body of the function and will need to decode unwind info.
 According to MSDN, an epilogue "must consist of either an
 add RSP,constant or lea RSP,constant[FPReg], followed by a
 series of zero or more 8-byte register pops and a return
 or a jmp".

It confuses me to call this an optimization though,
because MSDN says:

"This reduces the amount of unwind data required, because no extra
 data is needed to describe each epilog. Instead, the unwind code
 can determine that an epilog is being executed by scanning
 forward through a code stream to identify an epilog."

That reads to me that prologue decoding is not really
something we can forgo, but something that is _required_,
because the unwind data describing the epilog will simply
not be there.


>> The jump to an epilogue would not be part of the epilogue.
> 
> Doesn't really matter. The point is that if the current instruction
> is a jump to the epilogue, there is no need to decode unwind infs.

Okay, that one makes sense to me.  Please expand the comments
in that direction.

> 
>>> But I may miss your point.
>>
>> My point is that the docs say the epilogue has this rigid
>> format that always ends in a marker, and that a marker is
>> a ret or a jmp (therefore calling "pop" a marker as in the
>> "I would add" comment seems to me misleading).  The code
>> continues following the jmp, so it makes me believe the code
>> is erroneously decoding something after the jmp that is
>> not an epilogue (the caller or perhaps a tailcall).
> 
> No, it doesn't say that it ends in a marker.  The epilogue
> ends by a ret.

Not by my reading:

http://msdn.microsoft.com/en-us/library/tawsa7cb.aspx

"It must consist of either an add RSP,constant or
 lea RSP,constant[FPReg], followed by a series of zero or
 more 8-byte register pops and a return or a jmp"
                               ^^^^^^^^^^^^^^^^^

IOW, I read that as (in pseudo-bnf):

 add|lea
 (pop)*
 ret|jmp

This http://blogs.msdn.com/b/freik/archive/2006/01/04/509372.aspx
confirms my reading.

Hence all this confusion and the asking for an example
(a disassembly) where jmp appears in the middle
of an epilogue, and my fear that this prologue detection
might be following jmps when it should not, and
therefore not detecting prologue ends correctly.

"All function epilogues must look like this:

(optional) lea rsp, [frame ptr + frame size] or add rsp, frame size
pop reg (zero or more)
ret (or jmp)

(...)
No other instructions may occur betwen the first lea/add and the
final jmp or ret.
(...)
One other note: if the final jmp isn't an ip-relative jmp,
but an indirect jmp, it must be preceded by the REX prefix,
to indicate to the OS unwind routines that the jump is headed
outside of the function, otherwise, the OS assumes it's a jump
to a different location inside the same function."


So it looks to me that even if you follow
jumps to the epilogue as an optimization, the current
"rex jmp reg" handling is wrong - it should not follow the PC,
but instead by handling like ret and return 1.

Reading the patch in more detail, I now see that all the other
jmps handling in the patch's epilogue decoding are relative
jumps, and those are _not_ considered epilog markers (so it's
your optimization applying), per RtlVirtualUnwind's docs.  But,
as can be seen in the comment in the patch, immediate jmps
_are_ epilogue markers, so I do believe they should be handled,
and terminate the epilog decoding, just like ret.

-- 
Pedro Alves

Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Tristan Gingold]

On Jan 10, 2013, at 5:24 PM, Pedro Alves wrote:

> On 01/09/2013 08:07 PM, Tristan Gingold wrote:
> 
>>>> This is in fact an optimization. If we found a pop, followed by
>>>> an epilog marker, there is not need to decode unwind info.
>>> 
>>> I don't understand.  pops will always be followed by a marker.
>>> How can that be an optimization?
>> 
>> If pop weren't in this list, then if pc points to a pop the unwinder
>> will consider that the pc is in the body and need to decode unwind
>> infos.  Now, if pop is in the list, the unwinder will continue to decode
>> until a ret and if a ret is found, it will consider that the pc is in
>> the epilogue, avoiding decoding unwind infos.
> 
> (and again for quotability:)
> 
>> If pop weren't in this list,
> 
> I think you meant, that if pops were not handled in the
> while loop.  While I'm saying that the comment implies
> that pop is an epilog marker, and claiming it's not,
> therefore the comment is wrong and misleading.  It should
> be clarified/extended.

Ok.

> But let me see if I understood, by trying to rewrite
> and extend your reply into something that I'd find clearer:
> 
> We want to detect if the PC points to an epilogue (entry or midway).
> If so, the following epilogue detection+decoding below is
> sufficient.  Otherwise, the unwinder will consider that the PC
> is in the body of the function and will need to decode unwind info.
> According to MSDN, an epilogue "must consist of either an
> add RSP,constant or lea RSP,constant[FPReg], followed by a
> series of zero or more 8-byte register pops and a return
> or a jmp".
> 
> It confuses me to call this an optimization though,
> because MSDN says:
> 
> "This reduces the amount of unwind data required, because no extra
> data is needed to describe each epilog. Instead, the unwind code
> can determine that an epilog is being executed by scanning
> forward through a code stream to identify an epilog."
> 
> That reads to me that prologue decoding is not really
> something we can forgo, but something that is _required_,
> because the unwind data describing the epilog will simply
> not be there.

Yes, I now think that this is correct. The unwind data are only for the
prolog, the body don't need any additional data, while the epilog must
be specially handled.

>>> The jump to an epilogue would not be part of the epilogue.
>> 
>> Doesn't really matter. The point is that if the current instruction
>> is a jump to the epilogue, there is no need to decode unwind infs.
> 
> Okay, that one makes sense to me.  Please expand the comments
> in that direction.
> 
>> 
>>>> But I may miss your point.
>>> 
>>> My point is that the docs say the epilogue has this rigid
>>> format that always ends in a marker, and that a marker is
>>> a ret or a jmp (therefore calling "pop" a marker as in the
>>> "I would add" comment seems to me misleading).  The code
>>> continues following the jmp, so it makes me believe the code
>>> is erroneously decoding something after the jmp that is
>>> not an epilogue (the caller or perhaps a tailcall).
>> 
>> No, it doesn't say that it ends in a marker.  The epilogue
>> ends by a ret.
> 
> Not by my reading:
> 
> http://msdn.microsoft.com/en-us/library/tawsa7cb.aspx
> 
> "It must consist of either an add RSP,constant or
> lea RSP,constant[FPReg], followed by a series of zero or
> more 8-byte register pops and a return or a jmp"
>                               ^^^^^^^^^^^^^^^^^
> 
> IOW, I read that as (in pseudo-bnf):
> 
> add|lea
> (pop)*
> ret|jmp
> 
> This http://blogs.msdn.com/b/freik/archive/2006/01/04/509372.aspx
> confirms my reading.
> 
> Hence all this confusion and the asking for an example
> (a disassembly) where jmp appears in the middle
> of an epilogue, and my fear that this prologue detection
> might be following jmps when it should not, and
> therefore not detecting prologue ends correctly.
> 
> "All function epilogues must look like this:
> 
> (optional) lea rsp, [frame ptr + frame size] or add rsp, frame size
> pop reg (zero or more)
> ret (or jmp)
> 
> (...)
> No other instructions may occur betwen the first lea/add and the
> final jmp or ret.
> (...)
> One other note: if the final jmp isn't an ip-relative jmp,
> but an indirect jmp, it must be preceded by the REX prefix,
> to indicate to the OS unwind routines that the jump is headed
> outside of the function, otherwise, the OS assumes it's a jump
> to a different location inside the same function."

Ah, it looks like I missed this note!  That really make sense.

> So it looks to me that even if you follow
> jumps to the epilogue as an optimization, the current
> "rex jmp reg" handling is wrong - it should not follow the PC,
> but instead by handling like ret and return 1.

That correct.

> Reading the patch in more detail, I now see that all the other
> jmps handling in the patch's epilogue decoding are relative
> jumps, and those are _not_ considered epilog markers (so it's
> your optimization applying), per RtlVirtualUnwind's docs.  But,
> as can be seen in the comment in the patch, immediate jmps
> _are_ epilogue markers, so I do believe they should be handled,
> and terminate the epilog decoding, just like ret.

Yes, I think that's right.

Thanks for digging deeply!

Tristan.

[RFA] Windows x64 SEH unwinder (v2)

[Tristan Gingold]

Hello,

this is the second version of the patch we submitted in January.
I have rewritten amd64_windows_frame_decode_epilogue according to the
very serious review and comments from Pedro.

Tristan.

2013-07-08  Tristan Gingold  <gingold@adacore.com>

	* NEWS: Add entry mentioning support for native Windows x64
	SEH data. 

	* amd64-windows-tdep.c: #include "objfiles.h", "frame-unwind.h",
	"coff/internal.h", "coff/i386.h", "coff/pe.h" and "libcoff.h".
	(struct amd64_windows_frame_cache): New struct.
	(amd64_windows_w2gdb_regnum): New global.
	(pc_in_range, amd64_windows_frame_decode_epilogue)
	(amd64_windows_frame_decode_insns, amd64_windows_find_unwind_info)
	(amd64_windows_frame_cache, amd64_windows_frame_prev_register)
	(amd64_windows_frame_this_id): New functions.
	(amd64_windows_frame_unwind): New static global.
	(amd64_windows_skip_prologue): New function.
	(amd64_windows_init_abi): Call frame_unwind_prepend_unwinder
	with amd64_windows_frame_unwind. Call set_gdbarch_skip_prologue
	with amd64_windows_skip_prologue.

Index: amd64-windows-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/amd64-windows-tdep.c,v
retrieving revision 1.16
diff -u -r1.16 amd64-windows-tdep.c
--- amd64-windows-tdep.c	8 Apr 2013 19:59:08 -0000	1.16
+++ amd64-windows-tdep.c	8 Jul 2013 10:49:37 -0000
@@ -25,6 +25,12 @@
 #include "regcache.h"
 #include "windows-tdep.h"
 #include "frame.h"
+#include "objfiles.h"
+#include "frame-unwind.h"
+#include "coff/internal.h"
+#include "coff/i386.h"
+#include "coff/pe.h"
+#include "libcoff.h"
 
 /* The registers used to pass integer arguments during a function call.  */
 static int amd64_windows_dummy_call_integer_regs[] =
@@ -155,6 +161,752 @@
   return pc;
 }
 
+struct amd64_windows_frame_cache
+{
+  /* ImageBase for the module.  */
+  CORE_ADDR image_base;
+
+  /* Function start and end rva.  */
+  CORE_ADDR start_rva;
+  CORE_ADDR end_rva;
+
+  /* Next instruction to be executed.  */
+  CORE_ADDR pc;
+
+  /* Current sp.  */
+  CORE_ADDR sp;
+
+  /* Address of saved integer and xmm registers.  */
+  CORE_ADDR prev_reg_addr[16];
+  CORE_ADDR prev_xmm_addr[16];
+
+  /* These two next fields are set only for machine info frames.  */
+
+  /* Likewise for RIP.  */
+  CORE_ADDR prev_rip_addr;
+
+  /* Likewise for RSP.  */
+  CORE_ADDR prev_rsp_addr;
+
+  /* Address of the previous frame.  */
+  CORE_ADDR prev_sp;
+};
+
+/* Convert a Windows register number to gdb.  */
+static const enum amd64_regnum amd64_windows_w2gdb_regnum[] =
+{
+  AMD64_RAX_REGNUM,
+  AMD64_RCX_REGNUM,
+  AMD64_RDX_REGNUM,
+  AMD64_RBX_REGNUM,
+  AMD64_RSP_REGNUM,
+  AMD64_RBP_REGNUM,
+  AMD64_RSI_REGNUM,
+  AMD64_RDI_REGNUM,
+  AMD64_R8_REGNUM,
+  AMD64_R9_REGNUM,
+  AMD64_R10_REGNUM,
+  AMD64_R11_REGNUM,
+  AMD64_R12_REGNUM,
+  AMD64_R13_REGNUM,
+  AMD64_R14_REGNUM,
+  AMD64_R15_REGNUM
+};
+
+/* Return TRUE iff PC is the the range of the function corresponding to
+   CACHE.  */
+
+static int
+pc_in_range (CORE_ADDR pc, const struct amd64_windows_frame_cache *cache)
+{
+  return (pc >= cache->image_base + cache->start_rva
+	  && pc < cache->image_base + cache->end_rva);
+}
+
+/* Try to recognize and decode an epilogue sequence.
+
+   Return -1 if we fail to read the instructions for any reason.
+   Return 1 if an epilogue sequence was recognized, 0 otherwise.  */
+
+static int
+amd64_windows_frame_decode_epilogue (struct frame_info *this_frame,
+				     struct amd64_windows_frame_cache *cache)
+{
+  /* According to MSDN an epilogue "must consist of either an add RSP,constant
+     or lea RSP,constant[FPReg], followed by a series of zero or more 8-byte
+     register pops and a return or a jmp".
+
+     Furthermore, according to RtlVirtualUnwind, the complete list of
+     epilog marker is:
+     - ret                      [c3]
+     - ret n                    [c2 imm16]
+     - rep ret                  [f3 c3]
+     - jmp imm8 | imm32         [eb rel8] or [e9 rel32]
+     - jmp qword ptr imm32                 - not handled
+     - rex.w jmp reg            [4X ff eY]
+  */
+
+  CORE_ADDR pc = cache->pc;
+  CORE_ADDR cur_sp = cache->sp;
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  gdb_byte op;
+  gdb_byte rex;
+
+  /* We don't care about the instruction deallocating the frame:
+     if it hasn't been executed, the pc is still in the body,
+     if it has been executed, the following epilog decoding will work.  */
+
+  /* First decode:
+     -  pop reg                 [41 58-5f] or [58-5f].  */
+
+  while (1)
+    {
+      /* Read opcode. */
+      if (target_read_memory (pc, &op, 1) != 0)
+	return -1;
+
+      if (op >= 0x40 && op <= 0x4f)
+	{
+	  /* REX prefix.  */
+	  rex = op;
+
+	  /* Read opcode. */
+	  if (target_read_memory (pc + 1, &op, 1) != 0)
+	    return -1;
+	}
+      else
+	rex = 0;
+
+      if (op >= 0x58 && op <= 0x5f)
+	{
+	  /* pop reg  */
+	  gdb_byte reg = (op & 0x0f) | ((rex & 1) << 3);
+
+	  cache->prev_reg_addr[amd64_windows_w2gdb_regnum[reg]] = cur_sp;
+	  cur_sp += 8;
+	}
+      else
+	break;
+
+      /* Allow the user to break this loop.  This shouldn't happen as the
+	 number of consecutive pop should be small.  */
+      QUIT;
+    }
+
+  /* Then decode the marker.  */
+
+  /* Read opcode.  */
+  if (target_read_memory (pc, &op, 1) != 0)
+    return -1;
+
+  switch (op)
+    {
+    case 0xc3:
+      /* Ret.  */
+      cache->prev_rip_addr = cur_sp;
+      cache->prev_sp = cur_sp + 8;
+      return 1;
+
+    case 0xeb:
+      {
+	/* jmp rel8  */
+	gdb_byte rel8;
+	CORE_ADDR npc;
+
+	if (target_read_memory (pc + 1, &rel8, 1) != 0)
+	  return -1;
+	npc = pc + 2 + (signed char) rel8;
+
+	/* If the jump is within the function, then this is not a marker,
+	   otherwise this is a tail-call.  */
+	return !pc_in_range (npc, cache);
+      }
+
+    case 0xec:
+      {
+	/* jmp rel32  */
+	gdb_byte rel32[4];
+	CORE_ADDR npc;
+
+	if (target_read_memory (pc + 1, rel32, 4) != 0)
+	  return -1;
+	npc = pc + 5 + extract_signed_integer (rel32, 4, byte_order);
+
+	/* If the jump is within the function, then this is not a marker,
+	   otherwise this is a tail-call.  */
+	return !pc_in_range (npc, cache);
+      }
+
+    case 0xc2:
+      {
+	/* ret n  */
+	gdb_byte imm16[2];
+
+	if (target_read_memory (pc + 1, imm16, 2) != 0)
+	  return -1;
+	cache->prev_rip_addr = cur_sp;
+	cache->prev_sp = cur_sp
+	  + extract_unsigned_integer (imm16, 4, byte_order);
+	return 1;
+      }
+
+    case 0xf3:
+      {
+	/* rep; ret  */
+	gdb_byte op1;
+
+	if (target_read_memory (pc + 2, &op1, 1) != 0)
+	  return -1;
+	if (op1 != 0xc3)
+	  return 0;
+
+	cache->prev_rip_addr = cur_sp;
+	cache->prev_sp = cur_sp + 8;
+	return 1;
+      }
+
+    case 0x40:
+    case 0x41:
+    case 0x42:
+    case 0x43:
+    case 0x44:
+    case 0x45:
+    case 0x46:
+    case 0x47:
+    case 0x48:
+    case 0x49:
+    case 0x4a:
+    case 0x4b:
+    case 0x4c:
+    case 0x4d:
+    case 0x4e:
+    case 0x4f:
+      /* Got a REX prefix, read next byte.  */
+      rex = op;
+      if (target_read_memory (pc + 1, &op, 1) != 0)
+	return -1;
+
+      if (op == 0xff)
+	{
+	  /* rex jmp reg  */
+	  gdb_byte op1;
+	  unsigned int reg;
+	  gdb_byte buf[8];
+
+	  if (target_read_memory (pc + 2, &op1, 1) != 0)
+	    return -1;
+	  return (op1 & 0xf8) == 0xe0;
+	}
+      else
+	return 0;
+
+    default:
+      /* Not REX, so unknown.  */
+      return 0;
+    }
+}
+
+/* Decode and execute unwind insns at UNWIND_INFO.  */
+
+static void
+amd64_windows_frame_decode_insns (struct frame_info *this_frame,
+				  struct amd64_windows_frame_cache *cache,
+				  CORE_ADDR unwind_info)
+{
+  CORE_ADDR save_addr = 0;
+  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;
+
+  for (j = 0; ; j++)
+    {
+      struct external_pex64_unwind_info ex_ui;
+      /* There are at most 256 16-bit unwind insns.  */
+      gdb_byte insns[2 * 256];
+      gdb_byte *p;
+      gdb_byte *end_insns;
+      unsigned char codes_count;
+      unsigned char frame_reg;
+      unsigned char frame_off;
+
+      /* Read and decode header.  */
+      if (target_read_memory (cache->image_base + unwind_info,
+			      (gdb_byte *) &ex_ui, sizeof (ex_ui)) != 0)
+	return;
+
+      if (frame_debug)
+	fprintf_unfiltered
+	  (gdb_stdlog,
+	   "amd64_windows_frame_decodes_insn: "
+	   "%s: ver: %02x, plgsz: %02x, cnt: %02x, frame: %02x\n",
+	   paddress (gdbarch, unwind_info),
+	   ex_ui.Version_Flags, ex_ui.SizeOfPrologue,
+	   ex_ui.CountOfCodes, ex_ui.FrameRegisterOffset);
+
+      /* Check version.  */
+      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) != 1)
+	return;
+
+      if (j == 0
+	  && (cache->pc >=
+	      cache->image_base + cache->start_rva + 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
+	     sufficient.  Otherwise, the unwinder will consider that the PC
+	     is in the body of the function and will need to decode unwind
+	     info.  */
+	  if (amd64_windows_frame_decode_epilogue (this_frame, cache) == 1)
+	    return;
+
+	  /* Not in an epilog.  Clear possible side effects.  */
+	  memset (cache->prev_reg_addr, 0, sizeof (cache->prev_reg_addr));
+	}
+
+      codes_count = ex_ui.CountOfCodes;
+      frame_reg = PEX64_UWI_FRAMEREG (ex_ui.FrameRegisterOffset);
+
+      if (frame_reg != 0)
+	{
+	  /* According to msdn:
+	     If an FP reg is used, then any unwind code taking an offset must
+	     only be used after the FP reg is established in the prolog.  */
+	  gdb_byte buf[8];
+	  int frreg = amd64_windows_w2gdb_regnum[frame_reg];
+
+	  get_frame_register (this_frame, frreg, buf);
+	  save_addr = extract_unsigned_integer (buf, 8, byte_order);
+
+	  if (frame_debug)
+	    fprintf_unfiltered (gdb_stdlog, "   frame_reg=%s, val=%s\n",
+				gdbarch_register_name (gdbarch, frreg),
+				paddress (gdbarch, save_addr));
+	}
+
+      /* Read opcodes.  */
+      if (codes_count != 0
+	  && target_read_memory (cache->image_base + unwind_info
+				 + sizeof (ex_ui),
+				 insns, codes_count * 2) != 0)
+	return;
+
+      end_insns = &insns[codes_count * 2];
+      for (p = insns; p < end_insns; p += 2)
+	{
+	  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])
+	    {
+	      /* If there is no frame registers defined, the current value of
+		 rsp is used instead.  */
+	      if (frame_reg == 0)
+		save_addr = cur_sp;
+
+	      switch (PEX64_UNWCODE_CODE (p[1]))
+		{
+		case UWOP_PUSH_NONVOL:
+		  /* Push pre-decrements RSP.  */
+		  reg = amd64_windows_w2gdb_regnum[PEX64_UNWCODE_INFO (p[1])];
+		  cache->prev_reg_addr[reg] = cur_sp;
+		  cur_sp += 8;
+		  break;
+		case UWOP_ALLOC_LARGE:
+		  if (PEX64_UNWCODE_INFO (p[1]) == 0)
+		    cur_sp +=
+		      8 * extract_unsigned_integer (p + 2, 2, byte_order);
+		  else if (PEX64_UNWCODE_INFO (p[1]) == 1)
+		    cur_sp += extract_unsigned_integer (p + 2, 4, byte_order);
+		  else
+		    return;
+		  break;
+		case UWOP_ALLOC_SMALL:
+		  cur_sp += 8 + 8 * PEX64_UNWCODE_INFO (p[1]);
+		  break;
+		case UWOP_SET_FPREG:
+		  cur_sp = save_addr
+		    - PEX64_UWI_FRAMEOFF (ex_ui.FrameRegisterOffset) * 16;
+		  break;
+		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);
+		  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);
+		  break;
+		case UWOP_SAVE_XMM128:
+		  cache->prev_xmm_addr[PEX64_UNWCODE_INFO (p[1])] =
+		    save_addr
+		    - 16 * extract_unsigned_integer (p + 2, 2, byte_order);
+		  break;
+		case UWOP_SAVE_XMM128_FAR:
+		  cache->prev_xmm_addr[PEX64_UNWCODE_INFO (p[1])] =
+		    save_addr
+		    - 16 * extract_unsigned_integer (p + 2, 4, byte_order);
+		  break;
+		case UWOP_PUSH_MACHFRAME:
+		  if (PEX64_UNWCODE_INFO (p[1]) == 0)
+		    {
+		      cache->prev_rip_addr = cur_sp + 0;
+		      cache->prev_rsp_addr = cur_sp + 24;
+		      cur_sp += 40;
+		    }
+		  else if (PEX64_UNWCODE_INFO (p[1]) == 1)
+		    {
+		      cache->prev_rip_addr = cur_sp + 8;
+		      cache->prev_rsp_addr = cur_sp + 32;
+		      cur_sp += 48;
+		    }
+		  else
+		    return;
+		  break;
+		default:
+		  return;
+		}
+	    }
+
+	  /* Adjust with the length of the opcode.  */
+	  switch (PEX64_UNWCODE_CODE (p[1]))
+	    {
+	    case UWOP_PUSH_NONVOL:
+	    case UWOP_ALLOC_SMALL:
+	    case UWOP_SET_FPREG:
+	    case UWOP_PUSH_MACHFRAME:
+	      break;
+	    case UWOP_ALLOC_LARGE:
+	      if (PEX64_UNWCODE_INFO (p[1]) == 0)
+		p += 2;
+	      else if (PEX64_UNWCODE_INFO (p[1]) == 1)
+		p += 4;
+	      else
+		return;
+	      break;
+	    case UWOP_SAVE_NONVOL:
+	    case UWOP_SAVE_XMM128:
+	      p += 2;
+	      break;
+	    case UWOP_SAVE_NONVOL_FAR:
+	    case UWOP_SAVE_XMM128_FAR:
+	      p += 4;
+	      break;
+	    default:
+	      return;
+	    }
+	}
+      if (PEX64_UWI_FLAGS (ex_ui.Version_Flags) != UNW_FLAG_CHAININFO)
+	break;
+      else
+	{
+	  /* Read the chained unwind info.  */
+	  struct external_pex64_runtime_function d;
+	  CORE_ADDR chain_vma;
+
+	  chain_vma = cache->image_base + unwind_info
+	    + sizeof (ex_ui) + ((codes_count + 1) & ~1) * 2 + 8;
+
+	  if (target_read_memory (chain_vma, (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);
+	}
+
+      /* Allow the user to break this loop.  */
+      QUIT;
+    }
+  /* PC is saved by the call.  */
+  if (cache->prev_rip_addr == 0)
+    cache->prev_rip_addr = cur_sp;
+  cache->prev_sp = cur_sp + 8;
+
+  if (frame_debug)
+    fprintf_unfiltered (gdb_stdlog, "   prev_sp: %s, prev_pc @%s\n",
+			paddress (gdbarch, cache->prev_sp),
+			paddress (gdbarch, cache->prev_rip_addr));
+}
+
+/* Find SEH unwind info for PC, returning 0 on success.
+
+   UNWIND_INFO is set to the rva of unwind info address, IMAGE_BASE
+   to the base address of the corresponding image, and START_RVA
+   to the rva of the function containing PC.  */
+
+static int
+amd64_windows_find_unwind_info (struct gdbarch *gdbarch, CORE_ADDR pc,
+				CORE_ADDR *unwind_info,
+				CORE_ADDR *image_base,
+				CORE_ADDR *start_rva,
+				CORE_ADDR *end_rva)
+{
+  struct obj_section *sec;
+  pe_data_type *pe;
+  IMAGE_DATA_DIRECTORY *dir;
+  struct objfile *objfile;
+  unsigned long lo, hi;
+  CORE_ADDR base;
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+  /* Get the corresponding exception directory.  */
+  sec = find_pc_section (pc);
+  if (sec == NULL)
+    return -1;
+  objfile = sec->objfile;
+  pe = pe_data (sec->objfile->obfd);
+  dir = &pe->pe_opthdr.DataDirectory[PE_EXCEPTION_TABLE];
+
+  base = pe->pe_opthdr.ImageBase
+    + ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+  *image_base = base;
+
+  /* Find the entry.
+
+     Note: This does not handle dynamically added entries (for JIT
+     engines).  For this, we would need to ask the kernel directly,
+     which means getting some info from the native layer.  For the
+     rest of the code, however, it's probably faster to search
+     the entry ourselves.  */
+  lo = 0;
+  hi = dir->Size / sizeof (struct external_pex64_runtime_function);
+  *unwind_info = 0;
+  while (lo <= hi)
+    {
+      unsigned long mid = lo + (hi - lo) / 2;
+      struct external_pex64_runtime_function d;
+      CORE_ADDR sa, ea;
+
+      if (target_read_memory (base + dir->VirtualAddress + mid * sizeof (d),
+			      (gdb_byte *) &d, sizeof (d)) != 0)
+	return -1;
+
+      sa = extract_unsigned_integer (d.rva_BeginAddress, 4, byte_order);
+      ea = extract_unsigned_integer (d.rva_EndAddress, 4, byte_order);
+      if (pc < base + sa)
+	hi = mid - 1;
+      else if (pc >= base + ea)
+	lo = mid + 1;
+      else if (pc >= base + sa && pc < base + ea)
+	{
+	  /* Got it.  */
+	  *start_rva = sa;
+	  *end_rva = ea;
+	  *unwind_info =
+	    extract_unsigned_integer (d.rva_UnwindData, 4, byte_order);
+	  break;
+	}
+      else
+	break;
+    }
+
+  if (frame_debug)
+    fprintf_unfiltered
+      (gdb_stdlog,
+       "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;
+}
+
+/* Fill THIS_CACHE using the native amd64-windows unwinding data
+   for THIS_FRAME.  */
+
+static struct amd64_windows_frame_cache *
+amd64_windows_frame_cache (struct frame_info *this_frame, void **this_cache)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct amd64_windows_frame_cache *cache;
+  gdb_byte buf[8];
+  struct obj_section *sec;
+  pe_data_type *pe;
+  IMAGE_DATA_DIRECTORY *dir;
+  CORE_ADDR image_base;
+  CORE_ADDR pc;
+  struct objfile *objfile;
+  unsigned long lo, hi;
+  CORE_ADDR unwind_info = 0;
+
+  if (*this_cache)
+    return *this_cache;
+
+  cache = FRAME_OBSTACK_ZALLOC (struct amd64_windows_frame_cache);
+  *this_cache = cache;
+
+  /* Get current PC and SP.  */
+  pc = get_frame_pc (this_frame);
+  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
+  cache->sp = extract_unsigned_integer (buf, 8, byte_order);
+  cache->pc = pc;
+
+  if (amd64_windows_find_unwind_info (gdbarch, pc, &unwind_info,
+				      &cache->image_base,
+				      &cache->start_rva,
+				      &cache->end_rva))
+    return cache;
+
+  if (unwind_info == 0)
+    {
+      /* Assume a leaf function.  */
+      cache->prev_sp = cache->sp + 8;
+      cache->prev_rip_addr = cache->sp;
+    }
+  else
+    {
+      /* Decode unwind insns to compute saved addresses.  */
+      amd64_windows_frame_decode_insns (this_frame, cache, unwind_info);
+    }
+  return cache;
+}
+
+/* Implement the "prev_register" method of struct frame_unwind
+   using the standard Windows x64 SEH info.  */
+
+static struct value *
+amd64_windows_frame_prev_register (struct frame_info *this_frame,
+				   void **this_cache, int regnum)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct amd64_windows_frame_cache *cache =
+    amd64_windows_frame_cache (this_frame, this_cache);
+  struct value *val;
+  CORE_ADDR prev;
+
+  if (frame_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"amd64_windows_frame_prev_register %s for sp=%s\n",
+			gdbarch_register_name (gdbarch, regnum),
+			paddress (gdbarch, cache->prev_sp));
+
+  if (regnum >= AMD64_XMM0_REGNUM && regnum <= AMD64_XMM0_REGNUM + 15)
+      prev = cache->prev_xmm_addr[regnum - AMD64_XMM0_REGNUM];
+  else if (regnum == AMD64_RSP_REGNUM)
+    {
+      prev = cache->prev_rsp_addr;
+      if (prev == 0)
+	return frame_unwind_got_constant (this_frame, regnum, cache->prev_sp);
+    }
+  else if (regnum >= AMD64_RAX_REGNUM && regnum <= AMD64_R15_REGNUM)
+    prev = cache->prev_reg_addr[regnum - AMD64_RAX_REGNUM];
+  else if (regnum == AMD64_RIP_REGNUM)
+    prev = cache->prev_rip_addr;
+  else
+    prev = 0;
+
+  if (prev && frame_debug)
+    fprintf_unfiltered (gdb_stdlog, "  -> at %s\n", paddress (gdbarch, prev));
+
+  if (prev)
+    {
+      /* Register was saved.  */
+      return frame_unwind_got_memory (this_frame, regnum, prev);
+    }
+  else
+    {
+      /* Register is either volatile or not modified.  */
+      return frame_unwind_got_register (this_frame, regnum, regnum);
+    }
+}
+
+/* Implement the "this_id" method of struct frame_unwind using
+   the standard Windows x64 SEH info.  */
+
+static void
+amd64_windows_frame_this_id (struct frame_info *this_frame, void **this_cache,
+		   struct frame_id *this_id)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  struct amd64_windows_frame_cache *cache =
+    amd64_windows_frame_cache (this_frame, this_cache);
+
+  *this_id = frame_id_build (cache->prev_sp,
+			     cache->image_base + cache->start_rva);
+}
+
+/* Windows x64 SEH unwinder.  */
+
+static const struct frame_unwind amd64_windows_frame_unwind =
+{
+  NORMAL_FRAME,
+  default_frame_unwind_stop_reason,
+  &amd64_windows_frame_this_id,
+  &amd64_windows_frame_prev_register,
+  NULL,
+  default_frame_sniffer
+};
+
+/* Implement the "skip_prologue" gdbarch method.  */
+
+static CORE_ADDR
+amd64_windows_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  CORE_ADDR func_addr;
+  CORE_ADDR unwind_info = 0;
+  CORE_ADDR image_base, start_rva, end_rva;
+  struct external_pex64_unwind_info ex_ui;
+
+  /* Use prologue size from unwind info.  */
+  if (amd64_windows_find_unwind_info (gdbarch, pc, &unwind_info,
+				      &image_base, &start_rva, &end_rva) == 0)
+    {
+      if (unwind_info == 0)
+	{
+	  /* Leaf function.  */
+	  return pc;
+	}
+      else if (target_read_memory (image_base + unwind_info,
+				   (gdb_byte *) &ex_ui, sizeof (ex_ui)) == 0
+	       && PEX64_UWI_VERSION (ex_ui.Version_Flags) == 1)
+	return max (pc, image_base + start_rva + ex_ui.SizeOfPrologue);
+    }
+
+  /* See if we can determine the end of the prologue via the symbol
+     table.  If so, then return either the PC, or the PC after
+     the prologue, whichever is greater.  */
+  if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
+    {
+      CORE_ADDR post_prologue_pc
+	= skip_prologue_using_sal (gdbarch, func_addr);
+
+      if (post_prologue_pc != 0)
+	return max (pc, post_prologue_pc);
+    }
+
+  return pc;
+}
+
 /* Check Win64 DLL jmp trampolines and find jump destination.  */
 
 static CORE_ADDR
@@ -225,6 +977,9 @@
   set_gdbarch_iterate_over_objfiles_in_search_order
     (gdbarch, windows_iterate_over_objfiles_in_search_order);
 
+  frame_unwind_prepend_unwinder (gdbarch, &amd64_windows_frame_unwind);
+  set_gdbarch_skip_prologue (gdbarch, amd64_windows_skip_prologue);
+
   set_gdbarch_auto_wide_charset (gdbarch, amd64_windows_auto_wide_charset);
 
   set_solib_ops (gdbarch, &solib_target_so_ops);
@@ -239,4 +994,3 @@
   gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x86_64, GDB_OSABI_CYGWIN,
                           amd64_windows_init_abi);
 }
-

Re: [RFA] Windows x64 SEH unwinder (v2)

[Pedro Alves]

On 07/08/2013 11:55 AM, Tristan Gingold wrote:
> Hello,
> 
> this is the second version of the patch we submitted in January.
> I have rewritten amd64_windows_frame_decode_epilogue according to the
> very serious review and comments from Pedro.

Thanks!

(That was here, for reference:
 http://sourceware.org/ml/gdb-patches/2013-01/msg00165.html
 I had to go back and read it again, I had already swapped out
 all my SEH knowledge :-) )

What's the plan for debugging binaries with dwarf instead of SEH?
That'd be binaries built with gcc 4.6, IIUC.
There was the fallback idea of providing a knob to disable the
unwinder.  Is that no longer necessary?  Did you guys manage to
confirm what happens with those binaries?  Or will we take the wait
until someone complains the missing support approach?  If the latter,
than I think NEWS should mention we no longer support such binaries.
Now that I mention that, I notice the NEWS hunk is mentioned in the
ChangeLog, but it's actually missing from the patch.  :-)

Otherwise I have no further comments.  It looks good to me.

-- 
Pedro Alves

Re: [RFA] Windows x64 SEH unwinder (v2)

[Tristan Gingold]

On Jul 26, 2013, at 5:22 PM, Pedro Alves <palves@redhat.com> wrote:

> On 07/08/2013 11:55 AM, Tristan Gingold wrote:
>> Hello,
>> 
>> this is the second version of the patch we submitted in January.
>> I have rewritten amd64_windows_frame_decode_epilogue according to the
>> very serious review and comments from Pedro.
> 
> Thanks!
> 
> (That was here, for reference:
> http://sourceware.org/ml/gdb-patches/2013-01/msg00165.html
> I had to go back and read it again, I had already swapped out
> all my SEH knowledge :-) )

:-)

> What's the plan for debugging binaries with dwarf instead of SEH?
> That'd be binaries built with gcc 4.6, IIUC.
> There was the fallback idea of providing a knob to disable the
> unwinder.  Is that no longer necessary?  Did you guys manage to
> confirm what happens with those binaries?  Or will we take the wait
> until someone complains the missing support approach?  If the latter,
> than I think NEWS should mention we no longer support such binaries.
> Now that I mention that, I notice the NEWS hunk is mentioned in the
> ChangeLog, but it's actually missing from the patch.  :-)

I'd simply vote for not supporting binaries built with old versions of gcc.
If a user complain, we can either add a command to disable the SEH unwinder,
or (and I prefer that option) simply say: sorry, but your binary is not
compliant with the x64 ABI.

> Otherwise I have no further comments.  It looks good to me.

Ok, will commit it then (with the NEWS hunk).

Tristan.

Re: [RFA] Windows x64 SEH unwinder (v2)

[Pedro Alves]

On 08/19/2013 02:59 PM, Tristan Gingold wrote:
> 
> On Jul 26, 2013, at 5:22 PM, Pedro Alves <palves@redhat.com> wrote:
>> What's the plan for debugging binaries with dwarf instead of SEH?
>> That'd be binaries built with gcc 4.6, IIUC.
>> There was the fallback idea of providing a knob to disable the
>> unwinder.  Is that no longer necessary?  Did you guys manage to
>> confirm what happens with those binaries?  Or will we take the wait
>> until someone complains the missing support approach?  If the latter,
>> than I think NEWS should mention we no longer support such binaries.
>> Now that I mention that, I notice the NEWS hunk is mentioned in the
>> ChangeLog, but it's actually missing from the patch.  :-)
> 
> I'd simply vote for not supporting binaries built with old versions of gcc.
> If a user complain, we can either add a command to disable the SEH unwinder,
> or (and I prefer that option) simply say: sorry, but your binary is not
> compliant with the x64 ABI.

That's fine with me, but I do think that if going that direction,
then it'd be nicer to users to mention it in NEWS now, so they
may know upfront they'll need to upgrade their compiler (and stick
with older gdbs for older binaries).

Thanks,
-- 
Pedro Alves

[PATCH v3] Windows x64 SEH unwinder

[Tristan Gingold]

Hello,

after discussion with Roland Schwingel, I have found that the patch
doesn't handle well dwarf3 DW_OP_call_frame_cfa, because the SEH
unwinder is before the dwarf2 one.

So I propose this new patch.  The only change is the position of the
SEH unwinder: it is appended after the dwarf2 one.
As a consequence, old binaries should work too.

I have also added the NEWS chunk.

Ok for the trunk ?

Tristan.

2013-07-08  Tristan Gingold  <gingold@adacore.com>

	* NEWS: Add entry mentioning support for native Windows x64
	SEH data. 

	* amd64-windows-tdep.c: #include "objfiles.h", "frame-unwind.h",
	"coff/internal.h", "coff/i386.h", "coff/pe.h" and "libcoff.h".
	(struct amd64_windows_frame_cache): New struct.
	(amd64_windows_w2gdb_regnum): New global.
	(pc_in_range, amd64_windows_frame_decode_epilogue)
	(amd64_windows_frame_decode_insns, amd64_windows_find_unwind_info)
	(amd64_windows_frame_cache, amd64_windows_frame_prev_register)
	(amd64_windows_frame_this_id): New functions.
	(amd64_windows_frame_unwind): New static global.
	(amd64_windows_skip_prologue): New function.
	(amd64_windows_init_abi): Call frame_unwind_prepend_unwinder
	with amd64_windows_frame_unwind. Call set_gdbarch_skip_prologue
	with amd64_windows_skip_prologue.

diff --git a/gdb/NEWS b/gdb/NEWS
index 6ee82f7..d9588ff 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -134,6 +134,8 @@ qXfer:libraries-svr4:read's annex
 * New 'z' formatter for printing and examining memory, this displays the
   value as hexadecimal zero padded on the left to the size of the type.
 
+* GDB can now use Windows x64 unwinding data.
+
 *** Changes in GDB 7.6
 
 * Target record has been renamed to record-full.
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index a0fd074..4e750a1 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -25,6 +25,12 @@
 #include "regcache.h"
 #include "windows-tdep.h"
 #include "frame.h"
+#include "objfiles.h"
+#include "frame-unwind.h"
+#include "coff/internal.h"
+#include "coff/i386.h"
+#include "coff/pe.h"
+#include "libcoff.h"
 
 /* The registers used to pass integer arguments during a function call.  */
 static int amd64_windows_dummy_call_integer_regs[] =
@@ -155,6 +161,752 @@ amd64_skip_main_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
   return pc;
 }
 
+struct amd64_windows_frame_cache
+{
+  /* ImageBase for the module.  */
+  CORE_ADDR image_base;
+
+  /* Function start and end rva.  */
+  CORE_ADDR start_rva;
+  CORE_ADDR end_rva;
+
+  /* Next instruction to be executed.  */
+  CORE_ADDR pc;
+
+  /* Current sp.  */
+  CORE_ADDR sp;
+
+  /* Address of saved integer and xmm registers.  */
+  CORE_ADDR prev_reg_addr[16];
+  CORE_ADDR prev_xmm_addr[16];
+
+  /* These two next fields are set only for machine info frames.  */
+
+  /* Likewise for RIP.  */
+  CORE_ADDR prev_rip_addr;
+
+  /* Likewise for RSP.  */
+  CORE_ADDR prev_rsp_addr;
+
+  /* Address of the previous frame.  */
+  CORE_ADDR prev_sp;
+};
+
+/* Convert a Windows register number to gdb.  */
+static const enum amd64_regnum amd64_windows_w2gdb_regnum[] =
+{
+  AMD64_RAX_REGNUM,
+  AMD64_RCX_REGNUM,
+  AMD64_RDX_REGNUM,
+  AMD64_RBX_REGNUM,
+  AMD64_RSP_REGNUM,
+  AMD64_RBP_REGNUM,
+  AMD64_RSI_REGNUM,
+  AMD64_RDI_REGNUM,
+  AMD64_R8_REGNUM,
+  AMD64_R9_REGNUM,
+  AMD64_R10_REGNUM,
+  AMD64_R11_REGNUM,
+  AMD64_R12_REGNUM,
+  AMD64_R13_REGNUM,
+  AMD64_R14_REGNUM,
+  AMD64_R15_REGNUM
+};
+
+/* Return TRUE iff PC is the the range of the function corresponding to
+   CACHE.  */
+
+static int
+pc_in_range (CORE_ADDR pc, const struct amd64_windows_frame_cache *cache)
+{
+  return (pc >= cache->image_base + cache->start_rva
+	  && pc < cache->image_base + cache->end_rva);
+}
+
+/* Try to recognize and decode an epilogue sequence.
+
+   Return -1 if we fail to read the instructions for any reason.
+   Return 1 if an epilogue sequence was recognized, 0 otherwise.  */
+
+static int
+amd64_windows_frame_decode_epilogue (struct frame_info *this_frame,
+				     struct amd64_windows_frame_cache *cache)
+{
+  /* According to MSDN an epilogue "must consist of either an add RSP,constant
+     or lea RSP,constant[FPReg], followed by a series of zero or more 8-byte
+     register pops and a return or a jmp".
+
+     Furthermore, according to RtlVirtualUnwind, the complete list of
+     epilog marker is:
+     - ret                      [c3]
+     - ret n                    [c2 imm16]
+     - rep ret                  [f3 c3]
+     - jmp imm8 | imm32         [eb rel8] or [e9 rel32]
+     - jmp qword ptr imm32                 - not handled
+     - rex.w jmp reg            [4X ff eY]
+  */
+
+  CORE_ADDR pc = cache->pc;
+  CORE_ADDR cur_sp = cache->sp;
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  gdb_byte op;
+  gdb_byte rex;
+
+  /* We don't care about the instruction deallocating the frame:
+     if it hasn't been executed, the pc is still in the body,
+     if it has been executed, the following epilog decoding will work.  */
+
+  /* First decode:
+     -  pop reg                 [41 58-5f] or [58-5f].  */
+
+  while (1)
+    {
+      /* Read opcode. */
+      if (target_read_memory (pc, &op, 1) != 0)
+	return -1;
+
+      if (op >= 0x40 && op <= 0x4f)
+	{
+	  /* REX prefix.  */
+	  rex = op;
+
+	  /* Read opcode. */
+	  if (target_read_memory (pc + 1, &op, 1) != 0)
+	    return -1;
+	}
+      else
+	rex = 0;
+
+      if (op >= 0x58 && op <= 0x5f)
+	{
+	  /* pop reg  */
+	  gdb_byte reg = (op & 0x0f) | ((rex & 1) << 3);
+
+	  cache->prev_reg_addr[amd64_windows_w2gdb_regnum[reg]] = cur_sp;
+	  cur_sp += 8;
+	}
+      else
+	break;
+
+      /* Allow the user to break this loop.  This shouldn't happen as the
+	 number of consecutive pop should be small.  */
+      QUIT;
+    }
+
+  /* Then decode the marker.  */
+
+  /* Read opcode.  */
+  if (target_read_memory (pc, &op, 1) != 0)
+    return -1;
+
+  switch (op)
+    {
+    case 0xc3:
+      /* Ret.  */
+      cache->prev_rip_addr = cur_sp;
+      cache->prev_sp = cur_sp + 8;
+      return 1;
+
+    case 0xeb:
+      {
+	/* jmp rel8  */
+	gdb_byte rel8;
+	CORE_ADDR npc;
+
+	if (target_read_memory (pc + 1, &rel8, 1) != 0)
+	  return -1;
+	npc = pc + 2 + (signed char) rel8;
+
+	/* If the jump is within the function, then this is not a marker,
+	   otherwise this is a tail-call.  */
+	return !pc_in_range (npc, cache);
+      }
+
+    case 0xec:
+      {
+	/* jmp rel32  */
+	gdb_byte rel32[4];
+	CORE_ADDR npc;
+
+	if (target_read_memory (pc + 1, rel32, 4) != 0)
+	  return -1;
+	npc = pc + 5 + extract_signed_integer (rel32, 4, byte_order);
+
+	/* If the jump is within the function, then this is not a marker,
+	   otherwise this is a tail-call.  */
+	return !pc_in_range (npc, cache);
+      }
+
+    case 0xc2:
+      {
+	/* ret n  */
+	gdb_byte imm16[2];
+
+	if (target_read_memory (pc + 1, imm16, 2) != 0)
+	  return -1;
+	cache->prev_rip_addr = cur_sp;
+	cache->prev_sp = cur_sp
+	  + extract_unsigned_integer (imm16, 4, byte_order);
+	return 1;
+      }
+
+    case 0xf3:
+      {
+	/* rep; ret  */
+	gdb_byte op1;
+
+	if (target_read_memory (pc + 2, &op1, 1) != 0)
+	  return -1;
+	if (op1 != 0xc3)
+	  return 0;
+
+	cache->prev_rip_addr = cur_sp;
+	cache->prev_sp = cur_sp + 8;
+	return 1;
+      }
+
+    case 0x40:
+    case 0x41:
+    case 0x42:
+    case 0x43:
+    case 0x44:
+    case 0x45:
+    case 0x46:
+    case 0x47:
+    case 0x48:
+    case 0x49:
+    case 0x4a:
+    case 0x4b:
+    case 0x4c:
+    case 0x4d:
+    case 0x4e:
+    case 0x4f:
+      /* Got a REX prefix, read next byte.  */
+      rex = op;
+      if (target_read_memory (pc + 1, &op, 1) != 0)
+	return -1;
+
+      if (op == 0xff)
+	{
+	  /* rex jmp reg  */
+	  gdb_byte op1;
+	  unsigned int reg;
+	  gdb_byte buf[8];
+
+	  if (target_read_memory (pc + 2, &op1, 1) != 0)
+	    return -1;
+	  return (op1 & 0xf8) == 0xe0;
+	}
+      else
+	return 0;
+
+    default:
+      /* Not REX, so unknown.  */
+      return 0;
+    }
+}
+
+/* Decode and execute unwind insns at UNWIND_INFO.  */
+
+static void
+amd64_windows_frame_decode_insns (struct frame_info *this_frame,
+				  struct amd64_windows_frame_cache *cache,
+				  CORE_ADDR unwind_info)
+{
+  CORE_ADDR save_addr = 0;
+  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;
+
+  for (j = 0; ; j++)
+    {
+      struct external_pex64_unwind_info ex_ui;
+      /* There are at most 256 16-bit unwind insns.  */
+      gdb_byte insns[2 * 256];
+      gdb_byte *p;
+      gdb_byte *end_insns;
+      unsigned char codes_count;
+      unsigned char frame_reg;
+      unsigned char frame_off;
+
+      /* Read and decode header.  */
+      if (target_read_memory (cache->image_base + unwind_info,
+			      (gdb_byte *) &ex_ui, sizeof (ex_ui)) != 0)
+	return;
+
+      if (frame_debug)
+	fprintf_unfiltered
+	  (gdb_stdlog,
+	   "amd64_windows_frame_decodes_insn: "
+	   "%s: ver: %02x, plgsz: %02x, cnt: %02x, frame: %02x\n",
+	   paddress (gdbarch, unwind_info),
+	   ex_ui.Version_Flags, ex_ui.SizeOfPrologue,
+	   ex_ui.CountOfCodes, ex_ui.FrameRegisterOffset);
+
+      /* Check version.  */
+      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) != 1)
+	return;
+
+      if (j == 0
+	  && (cache->pc >=
+	      cache->image_base + cache->start_rva + 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
+	     sufficient.  Otherwise, the unwinder will consider that the PC
+	     is in the body of the function and will need to decode unwind
+	     info.  */
+	  if (amd64_windows_frame_decode_epilogue (this_frame, cache) == 1)
+	    return;
+
+	  /* Not in an epilog.  Clear possible side effects.  */
+	  memset (cache->prev_reg_addr, 0, sizeof (cache->prev_reg_addr));
+	}
+
+      codes_count = ex_ui.CountOfCodes;
+      frame_reg = PEX64_UWI_FRAMEREG (ex_ui.FrameRegisterOffset);
+
+      if (frame_reg != 0)
+	{
+	  /* According to msdn:
+	     If an FP reg is used, then any unwind code taking an offset must
+	     only be used after the FP reg is established in the prolog.  */
+	  gdb_byte buf[8];
+	  int frreg = amd64_windows_w2gdb_regnum[frame_reg];
+
+	  get_frame_register (this_frame, frreg, buf);
+	  save_addr = extract_unsigned_integer (buf, 8, byte_order);
+
+	  if (frame_debug)
+	    fprintf_unfiltered (gdb_stdlog, "   frame_reg=%s, val=%s\n",
+				gdbarch_register_name (gdbarch, frreg),
+				paddress (gdbarch, save_addr));
+	}
+
+      /* Read opcodes.  */
+      if (codes_count != 0
+	  && target_read_memory (cache->image_base + unwind_info
+				 + sizeof (ex_ui),
+				 insns, codes_count * 2) != 0)
+	return;
+
+      end_insns = &insns[codes_count * 2];
+      for (p = insns; p < end_insns; p += 2)
+	{
+	  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])
+	    {
+	      /* If there is no frame registers defined, the current value of
+		 rsp is used instead.  */
+	      if (frame_reg == 0)
+		save_addr = cur_sp;
+
+	      switch (PEX64_UNWCODE_CODE (p[1]))
+		{
+		case UWOP_PUSH_NONVOL:
+		  /* Push pre-decrements RSP.  */
+		  reg = amd64_windows_w2gdb_regnum[PEX64_UNWCODE_INFO (p[1])];
+		  cache->prev_reg_addr[reg] = cur_sp;
+		  cur_sp += 8;
+		  break;
+		case UWOP_ALLOC_LARGE:
+		  if (PEX64_UNWCODE_INFO (p[1]) == 0)
+		    cur_sp +=
+		      8 * extract_unsigned_integer (p + 2, 2, byte_order);
+		  else if (PEX64_UNWCODE_INFO (p[1]) == 1)
+		    cur_sp += extract_unsigned_integer (p + 2, 4, byte_order);
+		  else
+		    return;
+		  break;
+		case UWOP_ALLOC_SMALL:
+		  cur_sp += 8 + 8 * PEX64_UNWCODE_INFO (p[1]);
+		  break;
+		case UWOP_SET_FPREG:
+		  cur_sp = save_addr
+		    - PEX64_UWI_FRAMEOFF (ex_ui.FrameRegisterOffset) * 16;
+		  break;
+		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);
+		  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);
+		  break;
+		case UWOP_SAVE_XMM128:
+		  cache->prev_xmm_addr[PEX64_UNWCODE_INFO (p[1])] =
+		    save_addr
+		    - 16 * extract_unsigned_integer (p + 2, 2, byte_order);
+		  break;
+		case UWOP_SAVE_XMM128_FAR:
+		  cache->prev_xmm_addr[PEX64_UNWCODE_INFO (p[1])] =
+		    save_addr
+		    - 16 * extract_unsigned_integer (p + 2, 4, byte_order);
+		  break;
+		case UWOP_PUSH_MACHFRAME:
+		  if (PEX64_UNWCODE_INFO (p[1]) == 0)
+		    {
+		      cache->prev_rip_addr = cur_sp + 0;
+		      cache->prev_rsp_addr = cur_sp + 24;
+		      cur_sp += 40;
+		    }
+		  else if (PEX64_UNWCODE_INFO (p[1]) == 1)
+		    {
+		      cache->prev_rip_addr = cur_sp + 8;
+		      cache->prev_rsp_addr = cur_sp + 32;
+		      cur_sp += 48;
+		    }
+		  else
+		    return;
+		  break;
+		default:
+		  return;
+		}
+	    }
+
+	  /* Adjust with the length of the opcode.  */
+	  switch (PEX64_UNWCODE_CODE (p[1]))
+	    {
+	    case UWOP_PUSH_NONVOL:
+	    case UWOP_ALLOC_SMALL:
+	    case UWOP_SET_FPREG:
+	    case UWOP_PUSH_MACHFRAME:
+	      break;
+	    case UWOP_ALLOC_LARGE:
+	      if (PEX64_UNWCODE_INFO (p[1]) == 0)
+		p += 2;
+	      else if (PEX64_UNWCODE_INFO (p[1]) == 1)
+		p += 4;
+	      else
+		return;
+	      break;
+	    case UWOP_SAVE_NONVOL:
+	    case UWOP_SAVE_XMM128:
+	      p += 2;
+	      break;
+	    case UWOP_SAVE_NONVOL_FAR:
+	    case UWOP_SAVE_XMM128_FAR:
+	      p += 4;
+	      break;
+	    default:
+	      return;
+	    }
+	}
+      if (PEX64_UWI_FLAGS (ex_ui.Version_Flags) != UNW_FLAG_CHAININFO)
+	break;
+      else
+	{
+	  /* Read the chained unwind info.  */
+	  struct external_pex64_runtime_function d;
+	  CORE_ADDR chain_vma;
+
+	  chain_vma = cache->image_base + unwind_info
+	    + sizeof (ex_ui) + ((codes_count + 1) & ~1) * 2 + 8;
+
+	  if (target_read_memory (chain_vma, (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);
+	}
+
+      /* Allow the user to break this loop.  */
+      QUIT;
+    }
+  /* PC is saved by the call.  */
+  if (cache->prev_rip_addr == 0)
+    cache->prev_rip_addr = cur_sp;
+  cache->prev_sp = cur_sp + 8;
+
+  if (frame_debug)
+    fprintf_unfiltered (gdb_stdlog, "   prev_sp: %s, prev_pc @%s\n",
+			paddress (gdbarch, cache->prev_sp),
+			paddress (gdbarch, cache->prev_rip_addr));
+}
+
+/* Find SEH unwind info for PC, returning 0 on success.
+
+   UNWIND_INFO is set to the rva of unwind info address, IMAGE_BASE
+   to the base address of the corresponding image, and START_RVA
+   to the rva of the function containing PC.  */
+
+static int
+amd64_windows_find_unwind_info (struct gdbarch *gdbarch, CORE_ADDR pc,
+				CORE_ADDR *unwind_info,
+				CORE_ADDR *image_base,
+				CORE_ADDR *start_rva,
+				CORE_ADDR *end_rva)
+{
+  struct obj_section *sec;
+  pe_data_type *pe;
+  IMAGE_DATA_DIRECTORY *dir;
+  struct objfile *objfile;
+  unsigned long lo, hi;
+  CORE_ADDR base;
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+
+  /* Get the corresponding exception directory.  */
+  sec = find_pc_section (pc);
+  if (sec == NULL)
+    return -1;
+  objfile = sec->objfile;
+  pe = pe_data (sec->objfile->obfd);
+  dir = &pe->pe_opthdr.DataDirectory[PE_EXCEPTION_TABLE];
+
+  base = pe->pe_opthdr.ImageBase
+    + ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+  *image_base = base;
+
+  /* Find the entry.
+
+     Note: This does not handle dynamically added entries (for JIT
+     engines).  For this, we would need to ask the kernel directly,
+     which means getting some info from the native layer.  For the
+     rest of the code, however, it's probably faster to search
+     the entry ourselves.  */
+  lo = 0;
+  hi = dir->Size / sizeof (struct external_pex64_runtime_function);
+  *unwind_info = 0;
+  while (lo <= hi)
+    {
+      unsigned long mid = lo + (hi - lo) / 2;
+      struct external_pex64_runtime_function d;
+      CORE_ADDR sa, ea;
+
+      if (target_read_memory (base + dir->VirtualAddress + mid * sizeof (d),
+			      (gdb_byte *) &d, sizeof (d)) != 0)
+	return -1;
+
+      sa = extract_unsigned_integer (d.rva_BeginAddress, 4, byte_order);
+      ea = extract_unsigned_integer (d.rva_EndAddress, 4, byte_order);
+      if (pc < base + sa)
+	hi = mid - 1;
+      else if (pc >= base + ea)
+	lo = mid + 1;
+      else if (pc >= base + sa && pc < base + ea)
+	{
+	  /* Got it.  */
+	  *start_rva = sa;
+	  *end_rva = ea;
+	  *unwind_info =
+	    extract_unsigned_integer (d.rva_UnwindData, 4, byte_order);
+	  break;
+	}
+      else
+	break;
+    }
+
+  if (frame_debug)
+    fprintf_unfiltered
+      (gdb_stdlog,
+       "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;
+}
+
+/* Fill THIS_CACHE using the native amd64-windows unwinding data
+   for THIS_FRAME.  */
+
+static struct amd64_windows_frame_cache *
+amd64_windows_frame_cache (struct frame_info *this_frame, void **this_cache)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct amd64_windows_frame_cache *cache;
+  gdb_byte buf[8];
+  struct obj_section *sec;
+  pe_data_type *pe;
+  IMAGE_DATA_DIRECTORY *dir;
+  CORE_ADDR image_base;
+  CORE_ADDR pc;
+  struct objfile *objfile;
+  unsigned long lo, hi;
+  CORE_ADDR unwind_info = 0;
+
+  if (*this_cache)
+    return *this_cache;
+
+  cache = FRAME_OBSTACK_ZALLOC (struct amd64_windows_frame_cache);
+  *this_cache = cache;
+
+  /* Get current PC and SP.  */
+  pc = get_frame_pc (this_frame);
+  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
+  cache->sp = extract_unsigned_integer (buf, 8, byte_order);
+  cache->pc = pc;
+
+  if (amd64_windows_find_unwind_info (gdbarch, pc, &unwind_info,
+				      &cache->image_base,
+				      &cache->start_rva,
+				      &cache->end_rva))
+    return cache;
+
+  if (unwind_info == 0)
+    {
+      /* Assume a leaf function.  */
+      cache->prev_sp = cache->sp + 8;
+      cache->prev_rip_addr = cache->sp;
+    }
+  else
+    {
+      /* Decode unwind insns to compute saved addresses.  */
+      amd64_windows_frame_decode_insns (this_frame, cache, unwind_info);
+    }
+  return cache;
+}
+
+/* Implement the "prev_register" method of struct frame_unwind
+   using the standard Windows x64 SEH info.  */
+
+static struct value *
+amd64_windows_frame_prev_register (struct frame_info *this_frame,
+				   void **this_cache, int regnum)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  struct amd64_windows_frame_cache *cache =
+    amd64_windows_frame_cache (this_frame, this_cache);
+  struct value *val;
+  CORE_ADDR prev;
+
+  if (frame_debug)
+    fprintf_unfiltered (gdb_stdlog,
+			"amd64_windows_frame_prev_register %s for sp=%s\n",
+			gdbarch_register_name (gdbarch, regnum),
+			paddress (gdbarch, cache->prev_sp));
+
+  if (regnum >= AMD64_XMM0_REGNUM && regnum <= AMD64_XMM0_REGNUM + 15)
+      prev = cache->prev_xmm_addr[regnum - AMD64_XMM0_REGNUM];
+  else if (regnum == AMD64_RSP_REGNUM)
+    {
+      prev = cache->prev_rsp_addr;
+      if (prev == 0)
+	return frame_unwind_got_constant (this_frame, regnum, cache->prev_sp);
+    }
+  else if (regnum >= AMD64_RAX_REGNUM && regnum <= AMD64_R15_REGNUM)
+    prev = cache->prev_reg_addr[regnum - AMD64_RAX_REGNUM];
+  else if (regnum == AMD64_RIP_REGNUM)
+    prev = cache->prev_rip_addr;
+  else
+    prev = 0;
+
+  if (prev && frame_debug)
+    fprintf_unfiltered (gdb_stdlog, "  -> at %s\n", paddress (gdbarch, prev));
+
+  if (prev)
+    {
+      /* Register was saved.  */
+      return frame_unwind_got_memory (this_frame, regnum, prev);
+    }
+  else
+    {
+      /* Register is either volatile or not modified.  */
+      return frame_unwind_got_register (this_frame, regnum, regnum);
+    }
+}
+
+/* Implement the "this_id" method of struct frame_unwind using
+   the standard Windows x64 SEH info.  */
+
+static void
+amd64_windows_frame_this_id (struct frame_info *this_frame, void **this_cache,
+		   struct frame_id *this_id)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  struct amd64_windows_frame_cache *cache =
+    amd64_windows_frame_cache (this_frame, this_cache);
+
+  *this_id = frame_id_build (cache->prev_sp,
+			     cache->image_base + cache->start_rva);
+}
+
+/* Windows x64 SEH unwinder.  */
+
+static const struct frame_unwind amd64_windows_frame_unwind =
+{
+  NORMAL_FRAME,
+  default_frame_unwind_stop_reason,
+  &amd64_windows_frame_this_id,
+  &amd64_windows_frame_prev_register,
+  NULL,
+  default_frame_sniffer
+};
+
+/* Implement the "skip_prologue" gdbarch method.  */
+
+static CORE_ADDR
+amd64_windows_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  CORE_ADDR func_addr;
+  CORE_ADDR unwind_info = 0;
+  CORE_ADDR image_base, start_rva, end_rva;
+  struct external_pex64_unwind_info ex_ui;
+
+  /* Use prologue size from unwind info.  */
+  if (amd64_windows_find_unwind_info (gdbarch, pc, &unwind_info,
+				      &image_base, &start_rva, &end_rva) == 0)
+    {
+      if (unwind_info == 0)
+	{
+	  /* Leaf function.  */
+	  return pc;
+	}
+      else if (target_read_memory (image_base + unwind_info,
+				   (gdb_byte *) &ex_ui, sizeof (ex_ui)) == 0
+	       && PEX64_UWI_VERSION (ex_ui.Version_Flags) == 1)
+	return max (pc, image_base + start_rva + ex_ui.SizeOfPrologue);
+    }
+
+  /* See if we can determine the end of the prologue via the symbol
+     table.  If so, then return either the PC, or the PC after
+     the prologue, whichever is greater.  */
+  if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
+    {
+      CORE_ADDR post_prologue_pc
+	= skip_prologue_using_sal (gdbarch, func_addr);
+
+      if (post_prologue_pc != 0)
+	return max (pc, post_prologue_pc);
+    }
+
+  return pc;
+}
+
 /* Check Win64 DLL jmp trampolines and find jump destination.  */
 
 static CORE_ADDR
@@ -205,6 +957,19 @@ amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
+  /* The dwarf2 unwinder (appended very early by i386_gdbarch_init) is
+     preferred over the SEH one.  The reasons are:
+     - binaries without SEH but with dwarf2 debug info are correcly handled
+       (although they aren't ABI compliant, gcc before 4.7 didn't emit SEH
+       info).
+     - dwarf3 DW_OP_call_frame_cfa is correctly handled (it can only be
+       handled if the dwarf2 unwinder is used).
+
+    The call to amd64_init_abi appends default unwinders, that aren't
+    compatible with the SEH one.
+  */
+  frame_unwind_append_unwinder (gdbarch, &amd64_windows_frame_unwind);
+
   amd64_init_abi (info, gdbarch);
 
   /* On Windows, "long"s are only 32bit.  */
@@ -225,6 +990,8 @@ amd64_windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_iterate_over_objfiles_in_search_order
     (gdbarch, windows_iterate_over_objfiles_in_search_order);
 
+  set_gdbarch_skip_prologue (gdbarch, amd64_windows_skip_prologue);
+
   set_gdbarch_auto_wide_charset (gdbarch, amd64_windows_auto_wide_charset);
 
   set_solib_ops (gdbarch, &solib_target_so_ops);
@@ -239,4 +1006,3 @@ _initialize_amd64_windows_tdep (void)
   gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x86_64, GDB_OSABI_CYGWIN,
                           amd64_windows_init_abi);
 }
-

Re: [PATCH v3] Windows x64 SEH unwinder

[Eli Zaretskii]

> From: Tristan Gingold <gingold@adacore.com>
> Date: Thu, 22 Aug 2013 11:33:10 +0200
> Cc: Joel Brobecker <brobecker@adacore.com>, Roland Schwingel <roland.schwingel@onevision.de>, Pedro Alves <palves@redhat.com>
> 
> after discussion with Roland Schwingel, I have found that the patch
> doesn't handle well dwarf3 DW_OP_call_frame_cfa, because the SEH
> unwinder is before the dwarf2 one.
> 
> So I propose this new patch.  The only change is the position of the
> SEH unwinder: it is appended after the dwarf2 one.
> As a consequence, old binaries should work too.
> 
> I have also added the NEWS chunk.

The NEWS part is OK.

Thanks.

Re: [PATCH v3] Windows x64 SEH unwinder

[Pedro Alves]

On 08/22/2013 10:33 AM, Tristan Gingold wrote:
> 
> after discussion with Roland Schwingel, I have found that the patch
> doesn't handle well dwarf3 DW_OP_call_frame_cfa, because the SEH
> unwinder is before the dwarf2 one.

Can you clarify this a little better for the archives?

So that mean that for binaries built before that gcc fix,
the SEH unwinder won't kick in at all, right?  Then,
how come this fixes Roland's age old issue, and improves
unwinding for him?

In the previous versions, there was talk about needing
finer ordering of the unwinders in order to support both
old and new binaries.  What changed?  Why is this okay
now?

-- 
Pedro Alves

Re: [PATCH v3] Windows x64 SEH unwinder

[Tristan Gingold]

On Aug 22, 2013, at 5:26 PM, Pedro Alves <palves@redhat.com> wrote:

> On 08/22/2013 10:33 AM, Tristan Gingold wrote:
>> 
>> after discussion with Roland Schwingel, I have found that the patch
>> doesn't handle well dwarf3 DW_OP_call_frame_cfa, because the SEH
>> unwinder is before the dwarf2 one.
> 
> Can you clarify this a little better for the archives?

Sure.

> So that mean that for binaries built before that gcc fix,
> the SEH unwinder won't kick in at all, right?

No.
If dwarf2 info are presents, they will be used to unwind the
frames.  If they aren't, unwinding will probably fail.

>  Then,
> how come this fixes Roland's age old issue, and improves
> unwinding for him?

For the part compiled with gcc, the patch shouldn't change
anything.  But for function compiled by MS compilers (those
in dll), gdb will now unwind their frame by using the SEH
unwinder,

> In the previous versions, there was talk about needing
> finer ordering of the unwinders in order to support both
> old and new binaries.  What changed?  Why is this okay
> now?

Unwinding was possible when compiled with -g.  It is still
possible.
Version 1 and 2 of the patch failed, because the dwarf
unwinder was never used (always masked by the SEH unwinder).

It is still possible that gdb with support for SEH unwinder
fails where gdb without for binaries produced by old gcc for
the cases where the prologue analyzer using heuristic was
correct.  But debugging with this unwinder isn't reliable.

Hopes it clarifies :-)

Tristan.

Re: [PATCH v3] Windows x64 SEH unwinder

[Pedro Alves]

On 08/22/2013 04:41 PM, Tristan Gingold wrote:
> 
> On Aug 22, 2013, at 5:26 PM, Pedro Alves <palves@redhat.com> wrote:
> 
>> On 08/22/2013 10:33 AM, Tristan Gingold wrote:
>>>
>>> after discussion with Roland Schwingel, I have found that the patch
>>> doesn't handle well dwarf3 DW_OP_call_frame_cfa, because the SEH
>>> unwinder is before the dwarf2 one.
>>
>> Can you clarify this a little better for the archives?
> 
> Sure.
> 
>> So that mean that for binaries built before that gcc fix,
>> the SEH unwinder won't kick in at all, right?
> 
> No.
> If dwarf2 info are presents, they will be used to unwind the
> frames.  If they aren't, unwinding will probably fail.

Okay, I ISTR now that the SEH unwinder needs to always kicks
in, as leaf frames are identified by absence of SEH...  So on
old binaries without SEH and without dwarf, the SEH unwinder
will kick in, but the SEH unwinder will probably think all
functions are leaf, and that naturally most probably fails.

Good, now we have somewhere archived to point people at
once someone complains.  :-)

> 
>>  Then,
>> how come this fixes Roland's age old issue, and improves
>> unwinding for him?
> 
> For the part compiled with gcc, the patch shouldn't change
> anything.  But for function compiled by MS compilers (those
> in dll), gdb will now unwind their frame by using the SEH
> unwinder,

Got it.

> 
>> In the previous versions, there was talk about needing
>> finer ordering of the unwinders in order to support both
>> old and new binaries.  What changed?  Why is this okay
>> now?
> 
> Unwinding was possible when compiled with -g.  It is still
> possible.
> Version 1 and 2 of the patch failed, because the dwarf
> unwinder was never used (always masked by the SEH unwinder).

I understand that, but what I'm asking is about this discussion
in v2:

http://sourceware.org/ml/gdb-patches/2013-01/msg00185.html

 On Jan 9, 2013, at 6:10 PM, Pedro Alves wrote:
 > On 01/09/2013 04:28 PM, Tristan Gingold wrote:
 >
 >>>> I don't really see a real way of supporting both old and new versions
 >>>> of GCC, unless we have a way of more finely ordering the unwinders.
 >>>
 >>> What specific finer order where you considering would be needed to
 >>> fix this?
 >>
 > > Joel once proposed to activate this unwinder if the CU is compiled
 >> by gcc 4.6 or older.
 >
 > I don't think you need to have a way of more finely ordering
 > the unwinders for that.  AFAICS, we can make the sniffer
 > return false in that case.  I had understood him
 > as meaning something about making the whole prepend/append
 > mechanisms more finer grained somehow.

So coming from that angle, and seeing that v3 just uses the
usual prepend/append mechanisms, I naturally get curious on
whether we're missing something now.

So IIUC, this new ordering means that even for objects
compiled with newer gcc's that emit SEH, as long as there's
dwarf debug info, then GDB won't use the SEH to unwind.
I'm guessing that using SEH if available would
be better over dwarf2 (though I don't know for sure).
And, this version is then a compromise.  Right?

Just trying to understand, and record all this info
somewhere, not pushing to have it fixed.

> It is still possible that gdb with support for SEH unwinder
> fails where gdb without for binaries produced by old gcc for
> the cases where the prologue analyzer using heuristic was
> correct.  But debugging with this unwinder isn't reliable.

> 
> Hopes it clarifies :-)

Thanks, it does, somewhat.  :-)

-- 
Pedro Alves

Re: [PATCH v3] Windows x64 SEH unwinder

[Tristan Gingold]

On Aug 22, 2013, at 6:15 PM, Pedro Alves <palves@redhat.com> wrote:

> On 08/22/2013 04:41 PM, Tristan Gingold wrote:
>> 
>> On Aug 22, 2013, at 5:26 PM, Pedro Alves <palves@redhat.com> wrote:
>> 
>>> On 08/22/2013 10:33 AM, Tristan Gingold wrote:
>>>> 
>>>> after discussion with Roland Schwingel, I have found that the patch
>>>> doesn't handle well dwarf3 DW_OP_call_frame_cfa, because the SEH
>>>> unwinder is before the dwarf2 one.
>>> 
>>> Can you clarify this a little better for the archives?
>> 
>> Sure.
>> 
>>> So that mean that for binaries built before that gcc fix,
>>> the SEH unwinder won't kick in at all, right?
>> 
>> No.
>> If dwarf2 info are presents, they will be used to unwind the
>> frames.  If they aren't, unwinding will probably fail.
> 
> Okay, I ISTR now that the SEH unwinder needs to always kicks
> in, as leaf frames are identified by absence of SEH...  So on
> old binaries without SEH and without dwarf, the SEH unwinder
> will kick in, but the SEH unwinder will probably think all
> functions are leaf, and that naturally most probably fails.

Correct.  But it should be roughly as good as without the SEH
unwinder.

> Good, now we have somewhere archived to point people at
> once someone complains.  :-)
> 
>> 
>>> Then,
>>> how come this fixes Roland's age old issue, and improves
>>> unwinding for him?
>> 
>> For the part compiled with gcc, the patch shouldn't change
>> anything.  But for function compiled by MS compilers (those
>> in dll), gdb will now unwind their frame by using the SEH
>> unwinder,
> 
> Got it.
> 
>> 
>>> In the previous versions, there was talk about needing
>>> finer ordering of the unwinders in order to support both
>>> old and new binaries.  What changed?  Why is this okay
>>> now?
>> 
>> Unwinding was possible when compiled with -g.  It is still
>> possible.
>> Version 1 and 2 of the patch failed, because the dwarf
>> unwinder was never used (always masked by the SEH unwinder).
> 
> I understand that, but what I'm asking is about this discussion
> in v2:
> 
> http://sourceware.org/ml/gdb-patches/2013-01/msg00185.html
> 
> On Jan 9, 2013, at 6:10 PM, Pedro Alves wrote:
>> On 01/09/2013 04:28 PM, Tristan Gingold wrote:
>> 
>>>>> I don't really see a real way of supporting both old and new versions
>>>>> of GCC, unless we have a way of more finely ordering the unwinders.
>>>> 
>>>> What specific finer order where you considering would be needed to
>>>> fix this?
>>> 
>>> Joel once proposed to activate this unwinder if the CU is compiled
>>> by gcc 4.6 or older.
>> 
>> I don't think you need to have a way of more finely ordering
>> the unwinders for that.  AFAICS, we can make the sniffer
>> return false in that case.  I had understood him
>> as meaning something about making the whole prepend/append
>> mechanisms more finer grained somehow.
> 
> So coming from that angle, and seeing that v3 just uses the
> usual prepend/append mechanisms, I naturally get curious on
> whether we're missing something now.
> 
> So IIUC, this new ordering means that even for objects
> compiled with newer gcc's that emit SEH, as long as there's
> dwarf debug info, then GDB won't use the SEH to unwind.

Correct.

> I'm guessing that using SEH if available would
> be better over dwarf2 (though I don't know for sure).

Why ?  I think that both unwinders should be correct. One might
be faster than the other, but I don't know which.

> And, this version is then a compromise.  Right?

The only compromise is when there are no unwind infos.  Previously
a default unwinder based on heuristic was used (doesn't work well,
particularly for MS dll).  With the patch, they are considered as
leaf functions by the SEH unwinder.

> Just trying to understand, and record all this info
> somewhere, not pushing to have it fixed.

Sure. No problem with that.

>> It is still possible that gdb with support for SEH unwinder
>> fails where gdb without for binaries produced by old gcc for
>> the cases where the prologue analyzer using heuristic was
>> correct.  But debugging with this unwinder isn't reliable.
> 
>> 
>> Hopes it clarifies :-)
> 
> Thanks, it does, somewhat.  :-)

Tristan.

Re: [PATCH v3] Windows x64 SEH unwinder

[Pedro Alves]

Hi there,

On 08/23/2013 07:53 AM, Tristan Gingold wrote:
> 
> On Aug 22, 2013, at 6:15 PM, Pedro Alves <palves@redhat.com> wrote:
>> I'm guessing that using SEH if available would
>> be better over dwarf2 (though I don't know for sure).
> 
> Why ?  

No particular reason -- I guess I got confused by earlier
discussions in v1/v2, and I understood it as such.

> I think that both unwinders should be correct. One might
> be faster than the other, but I don't know which.

Okay.

> 
>> And, this version is then a compromise.  Right?
> 
> The only compromise is when there are no unwind infos.  Previously
> a default unwinder based on heuristic was used (doesn't work well,
> particularly for MS dll).  With the patch, they are considered as
> leaf functions by the SEH unwinder.

Okay.

FAOD, the patch is fine with me.

-- 
Pedro Alves

Re: [PATCH v3] Windows x64 SEH unwinder

[Tristan Gingold]

On Aug 27, 2013, at 7:45 PM, Pedro Alves <palves@redhat.com> wrote:
>> 
>>> And, this version is then a compromise.  Right?
>> 
>> The only compromise is when there are no unwind infos.  Previously
>> a default unwinder based on heuristic was used (doesn't work well,
>> particularly for MS dll).  With the patch, they are considered as
>> leaf functions by the SEH unwinder.
> 
> Okay.
> 
> FAOD, the patch is fine with me.

Now committed.

Thanks,
Tristan.

Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Eli Zaretskii]

> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Joel Brobecker <brobecker@adacore.com>
> Date: Wed,  9 Jan 2013 14:53:01 +0400
> 
> This is the main part of the patch series, which actually
> adds the unwinder.

Thanks!

> One element worth mentioning, perhaps, is the fact that we prepend
> the unwinder, and the sniffer is the default_frame_sniffer which
> always returns 1 (absence of SEH info is normal and means it is
> a leaf function).  This effectively means that all other unwinders
> effectively get shunted. And in particular, I do not think that
> the DWARF unwinder will kick in even if DWARF unwind info is found.
> 
> The problem is that we want to be ahead of the default amd64-tdep
> unwinders, which is kind of a last-resort unwinder doing a good
> job under limited situations only. We'd like to be behind the DWARF
> unwinder if we could, but I don't think there is a way to ensure
> that yet.
> 
> In practice, this shouldn't be a problem, since this unwinder
> has been very reliable for us so far.  But it does assume that
> the compiler is recent enough to generate native SEH data which,
> for GCC, means GCC 4.7 (I have been told). On the other hand,
> it looks like the first GCC release to support x64-windows was
> GCC 4.6.
> 
> I don't really see a real way of supporting both old and new versions
> of GCC, unless we have a way of more finely ordering the unwinders.
> Worse case scenario, we stop supporting code generated by GCC 4.6.
> Or an alternative option is to provide a setting to disable this
> unwinder.

I'm confused: is this change only for 64-bit Windows programs?  If
not, what GCC versions will this support for debugging 32-bit
executables?

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 3451505..d981ac5 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -134,6 +134,8 @@ show print type typedefs
>    feature to be enabled.  For more information, see:
>        http://fedoraproject.org/wiki/Features/MiniDebugInfo
>  
> +* GDB can now use Windows x64 unwinding data.
> +

This is OK to go in, but it seems to imply that only 64-bit
executables are affected.  What would it take to do the same on 32-bit
Windows?

Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Tristan Gingold]

On Jan 9, 2013, at 5:05 PM, Eli Zaretskii wrote:

>> From: Joel Brobecker <brobecker@adacore.com>
>> Cc: Joel Brobecker <brobecker@adacore.com>
>> Date: Wed,  9 Jan 2013 14:53:01 +0400
>> 
>> This is the main part of the patch series, which actually
>> adds the unwinder.
> 
> Thanks!
> 
>> One element worth mentioning, perhaps, is the fact that we prepend
>> the unwinder, and the sniffer is the default_frame_sniffer which
>> always returns 1 (absence of SEH info is normal and means it is
>> a leaf function).  This effectively means that all other unwinders
>> effectively get shunted. And in particular, I do not think that
>> the DWARF unwinder will kick in even if DWARF unwind info is found.
>> 
>> The problem is that we want to be ahead of the default amd64-tdep
>> unwinders, which is kind of a last-resort unwinder doing a good
>> job under limited situations only. We'd like to be behind the DWARF
>> unwinder if we could, but I don't think there is a way to ensure
>> that yet.
>> 
>> In practice, this shouldn't be a problem, since this unwinder
>> has been very reliable for us so far.  But it does assume that
>> the compiler is recent enough to generate native SEH data which,
>> for GCC, means GCC 4.7 (I have been told). On the other hand,
>> it looks like the first GCC release to support x64-windows was
>> GCC 4.6.
>> 
>> I don't really see a real way of supporting both old and new versions
>> of GCC, unless we have a way of more finely ordering the unwinders.
>> Worse case scenario, we stop supporting code generated by GCC 4.6.
>> Or an alternative option is to provide a setting to disable this
>> unwinder.
> 
> I'm confused: is this change only for 64-bit Windows programs?

Yes.

>  If
> not, what GCC versions will this support for debugging 32-bit
> executables?
> 
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 3451505..d981ac5 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -134,6 +134,8 @@ show print type typedefs
>>   feature to be enabled.  For more information, see:
>>       http://fedoraproject.org/wiki/Features/MiniDebugInfo
>> 
>> +* GDB can now use Windows x64 unwinding data.
>> +
> 
> This is OK to go in, but it seems to imply that only 64-bit
> executables are affected.  What would it take to do the same on 32-bit
> Windows?

There is no such thing as unwind info for SEH on windows 32-bit.

Tristan.

[RFA/commit+NEWS 1/2] Add command set/show debug unwind.

[Joel Brobecker]

Hello,

This patch adds a new debug setting to be used by frame unwinders.
It should be relatively straightforward.

gdb/ChangeLog:

        * frame-unwind.h (unwind_debug): Declare.
        * frame-unwind.c: #include "command.h" and "gdbcmd.h".
        (unwind_debug): New global.
        (show_unwind_debug): New function.
        (_initialize_frame_unwind): Add "set/show debug unwind"
        commands.

        * NEWS: Add entry for new "set/show debug unwind" commands.

gdb/doc/ChangeLog:

        * gdb.texinfo (Debugging Output): Document the new
        "set/show debug unwind" commands.

Will commit in a couple of days, if no objection.

---
 gdb/NEWS            |    4 ++++
 gdb/doc/gdb.texinfo |    5 +++++
 gdb/frame-unwind.c  |   26 ++++++++++++++++++++++++++
 gdb/frame-unwind.h  |    4 ++++
 4 files changed, 39 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 36bbd12..3451505 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -84,6 +84,10 @@ set debug notification
 show debug notification
   Control display of debugging info for async remote notification.
 
+set debug unwind
+show debug unwind
+  Control display of debugging info for some unwinders.
+
 * Removed commands
 
   ** For the Renesas Super-H architecture, the "regs" command has been removed
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f973263..f6fb342 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22046,6 +22046,11 @@ message.
 @item show debug timestamp
 Displays the current state of displaying timestamps with @value{GDBN}
 debugging info.
+@item set debug unwind
+@cindex unwinding debugging info
+Turns on or off display debugging info from some unwinders.
+@item show debug unwind
+Display the current state of displaying unwind debugging info.
 @item set debugvarobj
 @cindex variable object debugging info
 Turns on or off display of @value{GDBN} variable object debugging
diff --git a/gdb/frame-unwind.c b/gdb/frame-unwind.c
index b66febf..7a9f6a2 100644
--- a/gdb/frame-unwind.c
+++ b/gdb/frame-unwind.c
@@ -27,6 +27,8 @@
 #include "exceptions.h"
 #include "gdb_assert.h"
 #include "gdb_obstack.h"
+#include "command.h"
+#include "gdbcmd.h"
 
 static struct gdbarch_data *frame_unwind_data;
 
@@ -237,6 +239,20 @@ frame_unwind_got_address (struct frame_info *frame, int regnum,
   return reg_val;
 }
 
+/* Flag to control frame-unwind debugging.  */
+
+int unwind_debug;
+
+/* Implement the "show debug unwind" command.  */
+
+static void
+show_unwind_debug (struct ui_file *file, int from_tty,
+		   struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("unwind debugging is %s.\n"), value);
+}
+
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_frame_unwind;
 
@@ -244,4 +260,14 @@ void
 _initialize_frame_unwind (void)
 {
   frame_unwind_data = gdbarch_data_register_pre_init (frame_unwind_init);
+
+  add_setshow_zinteger_cmd ("unwind", class_maintenance, &unwind_debug,
+			    _("Set unwind debugging."),
+			    _("Show unwind debugging."),
+			    _("When non-zero, frame specific internal "
+			      "debugging is enabled."),
+			    NULL,
+			    show_unwind_debug,
+			    &setdebuglist, &showdebuglist);
+
 }
diff --git a/gdb/frame-unwind.h b/gdb/frame-unwind.h
index 91ccf8c..0471730 100644
--- a/gdb/frame-unwind.h
+++ b/gdb/frame-unwind.h
@@ -30,6 +30,10 @@ struct value;
 
 #include "frame.h"		/* For enum frame_type.  */
 
+/* Flag to control frame-unwind debugging.  */
+
+extern int unwind_debug;
+
 /* The following unwind functions assume a chain of frames forming the
    sequence: (outer) prev <-> this <-> next (inner).  All the
    functions are called with this frame's `struct frame_info' and
-- 
1.7.10.4

Re: [RFA/commit+NEWS 1/2] Add command set/show debug unwind.

[Jan Kratochvil]

Hi Joel,

On Wed, 09 Jan 2013 11:53:00 +0100, Joel Brobecker wrote:
> This patch adds a new debug setting to be used by frame unwinders.
> It should be relatively straightforward.

is the existing "set debug frame 1" output too disturbing?
I was using that one already during unwinders debugging.

No problem with it, just if it was not somehow forgotten.


Regards,
Jan

Re: [RFA/commit+NEWS 1/2] Add command set/show debug unwind.

[Joel Brobecker]

> > This patch adds a new debug setting to be used by frame unwinders.
> > It should be relatively straightforward.
> 
> is the existing "set debug frame 1" output too disturbing?
> I was using that one already during unwinders debugging.

Actually, why not? Tristan and I discussed it, and we don't
necessarily see a problem with it. I find it a little verbose,
so I'd like to give it a try, and see how it works out. But,
a priori, I think using "set debug frame" would be a good
suggestion.

To answer Tom's question about using zinteger, I think it comes
from what we have been typically using at the time when the code
was written. We had the zinteger vs boolean discussion only
recently... If we prefer staying with the new setting, we will
change it to a boolean.

Thanks for the suggestion!

-- 
Joel

Re: [RFA/commit+NEWS 1/2] Add command set/show debug unwind.

[Tom Tromey]

Joel> This patch adds a new debug setting to be used by frame unwinders.
Joel> It should be relatively straightforward.

Joel> +  add_setshow_zinteger_cmd ("unwind", class_maintenance, &unwind_debug,

In addition to what Jan said, I'm curious why zinteger and not boolean.

Tom

Re: [RFA/commit+NEWS 1/2] Add command set/show debug unwind.

[Eli Zaretskii]

> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Joel Brobecker <brobecker@adacore.com>
> Date: Wed,  9 Jan 2013 14:53:00 +0400
> 
>         * NEWS: Add entry for new "set/show debug unwind" commands.
> 
> gdb/doc/ChangeLog:
> 
>         * gdb.texinfo (Debugging Output): Document the new
>         "set/show debug unwind" commands.

The patches to NEWS and gdb.texinfo are approved.

Thanks.

Re: Add Windows x64 SEH unwinder (take 2)

[Pedro Alves]

What changed compared to take 1?

-- 
Pedro Alves

Re: Add Windows x64 SEH unwinder (take 2)

[Joel Brobecker]

> What changed compared to take 1?

Very little - a couple of bug fixes, I believe. I also reworded
a bit some of the comments, expanded some of the function descriptions,
reformatted a few lines...

-- 
Joel