[RFC] PR gdb/17445 fix

["Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>]

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

  Bug report gdb/17445
shows that use of explicit xmm15 register in windows x86_64
code leads to wrong unwinding of stacktrace by GDB.
  This problem comes from the fact that
the return address column is set to 32 for x86_64 pe objects,
while it is 16 for other targets.
  Dwarf x86_64 register 16 is RIP,
while register 32 is XMM15.
  The reason for this was apparently that the value of
the return address column is also interpreted as the highest
index of the register that needs to be saved according to the ABI 
which is indeed different for Microsoft.
  Nevertheless, I found nothing inside DWARF4 documentation
that makes any relation between the resisters that should be saved
and the return address column.
  I came to the conclusion that this is a mis-interpretation
of the dwarf standard that is specific to GNU bfd-gas-gdb.
  The patch proposed below tries to fix the current PR
by removing the above assumptions and restoring RIP
as return address register for pe(i)-x86-64 targets.

 Comments most welcome,

Pierre Muller
Pascal language maintainer for GDB.

PS: One problem is that I was not able to correctly run
the testsuite before and after my patch, as
there are still cygwin/mingw specific issues with testsuite runs.


ChangeLog (needs to be split into binutils/gas/gdb directories)

2014-10-01  Pierre Muller  <muller@sourceware.org>

        PR gdb/17445
        * binutils/dwarf.c (display_debug_frames): Handle return address
column
        specifically, do not limit num_regs to the value of return address
        column.
        Modified handling of DW_CFA_restore{_extended} to check that
        a valid storage is present at restore point.

        * gas/config/tc-i386.c (x86_dwarf2_return_column): Remove special
        value for x86_64 pe coff return address column.
        This restores RIP register for return address column.

        * gdb/dwarf2-frame.c (dwarf2_frame_cache): Avoid double handling of
        return address column.


[-- Attachment #2: fix-PR-17445-v5.patch --]
[-- Type: application/octet-stream, Size: 6253 bytes --]

2014-10-01  Pierre Muller  <muller@sourceware.org>

	PR gdb/17445
	* binutils/dwarf.c (display_debug_frames): Handle return address column
	specifically, do not limit num_regs to the value of return address
	column.
	Modified handling of DW_CFA_restore{_extended} to check that
	a valid storage is present at restore point.

	* gas/config/tc-i386.c (x86_dwarf2_return_column): Remove special
	value for x86_64 pe coff return address column.
	This restores RIP register for return address column.

	* gdb/dwarf2-frame.c (dwarf2_frame_cache): Avoid double handling of
	return address column.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 3f4095a..8e903dc 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -5337,6 +5337,7 @@ display_debug_frames (struct dwarf_section *section,
   unsigned int length_return;
   int max_regs = 0;
   const char *bad_reg = _("bad register: ");
+  const char *bad_cfa = _("bad cfa: ");
   int saved_eh_addr_size = eh_addr_size;
 
   printf (_("Contents of the %s section:\n"), section->name);
@@ -5632,8 +5633,8 @@ display_debug_frames (struct dwarf_section *section,
 		    fc->col_type[opa] = DW_CFA_undefined;
 		  break;
 		case DW_CFA_restore:
-		  if (frame_need_space (fc, opa) >= 0)
-		    fc->col_type[opa] = DW_CFA_undefined;
+		  /* This reg must be valid before, so
+		     do not increase size of fc here.  */
 		  break;
 		case DW_CFA_set_loc:
 		  start += encoded_ptr_size;
@@ -5655,9 +5656,8 @@ display_debug_frames (struct dwarf_section *section,
 		  break;
 		case DW_CFA_restore_extended:
 		  reg = LEB ();
-		  frame_need_space (fc, reg);
-		  if (frame_need_space (fc, reg) >= 0)
-		    fc->col_type[reg] = DW_CFA_undefined;
+		  /* This reg must be valid before, so
+		     do not increase size of fc here.  */
 		  break;
 		case DW_CFA_undefined:
 		  reg = LEB ();
@@ -5735,8 +5735,10 @@ display_debug_frames (struct dwarf_section *section,
 	  long l;
 	  dwarf_vma ofs;
 	  dwarf_vma vma;
-	  const char *reg_prefix = "";
+	  const char *reg_name;
+	  const char *reg_prefix;
 
+	  reg_prefix = "";
 	  op = *start++;
 	  opa = op & 0x3f;
 	  if (op & 0xc0)
@@ -5761,10 +5763,16 @@ display_debug_frames (struct dwarf_section *section,
 	    case DW_CFA_offset:
 	      roffs = LEB ();
 	      if (opa >= (unsigned int) fc->ncols)
-		reg_prefix = bad_reg;
+		{
+		  reg_prefix = bad_reg;
+		}
+	      if (opa == (unsigned int) fc->ra)
+		reg_name = "RA ";
+	      else
+		reg_name = regname (opa, 0);
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
 		printf ("  DW_CFA_offset: %s%s at cfa%+ld\n",
-			reg_prefix, regname (opa, 0),
+			reg_prefix, reg_name,
 			roffs * fc->data_factor);
 	      if (*reg_prefix == '\0')
 		{
@@ -5774,19 +5782,29 @@ display_debug_frames (struct dwarf_section *section,
 	      break;
 
 	    case DW_CFA_restore:
-	      if (opa >= (unsigned int) cie->ncols
-		  || opa >= (unsigned int) fc->ncols)
+	      if (/* opa >= (unsigned int) cie->ncols
+		  || */ opa >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
+	      else if (fc->col_type[opa] == DW_CFA_undefined)
+		reg_prefix = bad_cfa;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
 		printf ("  DW_CFA_restore: %s%s\n",
 			reg_prefix, regname (opa, 0));
 	      if (*reg_prefix == '\0')
 		{
-		  fc->col_type[opa] = cie->col_type[opa];
-		  fc->col_offset[opa] = cie->col_offset[opa];
-		  if (do_debug_frames_interp
-		      && fc->col_type[opa] == DW_CFA_unreferenced)
-		    fc->col_type[opa] = DW_CFA_undefined;
+		  if (opa <= (unsigned int) cie->ncols)
+		    {
+		      fc->col_type[opa] = cie->col_type[opa];
+		      fc->col_offset[opa] = cie->col_offset[opa];
+		      if (do_debug_frames_interp
+			  && fc->col_type[opa] == DW_CFA_unreferenced)
+			fc->col_type[opa] = DW_CFA_undefined;
+		    }
+		  else
+		    {
+		      fc->col_type[opa] = DW_CFA_undefined;
+		      fc->col_offset[opa] = 0;
+		    }
 		}
 	      break;
 
@@ -5874,16 +5892,26 @@ display_debug_frames (struct dwarf_section *section,
 
 	    case DW_CFA_restore_extended:
 	      reg = LEB ();
-	      if (reg >= (unsigned int) cie->ncols
-		  || reg >= (unsigned int) fc->ncols)
+	      if (/*reg >= (unsigned int) cie->ncols
+		  || */reg >= (unsigned int) fc->ncols)
 		reg_prefix = bad_reg;
+	      else if (fc->col_type[reg] == DW_CFA_undefined)
+		reg_prefix = bad_cfa;
 	      if (! do_debug_frames_interp || *reg_prefix != '\0')
 		printf ("  DW_CFA_restore_extended: %s%s\n",
 			reg_prefix, regname (reg, 0));
 	      if (*reg_prefix == '\0')
 		{
-		  fc->col_type[reg] = cie->col_type[reg];
-		  fc->col_offset[reg] = cie->col_offset[reg];
+		  if (reg < (unsigned int) cie->ncols)
+		    {
+		      fc->col_type[reg] = cie->col_type[reg];
+		      fc->col_offset[reg] = cie->col_offset[reg];
+		    }
+		  else
+		    {
+		      fc->col_type[opa] = DW_CFA_undefined;
+		      fc->col_offset[opa] = 0;
+		    }
 		}
 	      break;
 
diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 68ca7e4..fde7eba 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -2632,7 +2632,9 @@ md_begin (void)
 
   if (flag_code == CODE_64BIT)
     {
-#if defined (OBJ_COFF) && defined (TE_PE)
+#if 0
+      // defined (OBJ_COFF) && defined (TE_PE)
+      // This was wrong
       x86_dwarf2_return_column = (OUTPUT_FLAVOR == bfd_target_coff_flavour
 				  ? 32 : 16);
 #else
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index f2330d9..5e17da6 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -1160,6 +1160,11 @@ dwarf2_frame_cache (struct frame_info *this_frame, void **this_cache)
 	/* Use the GDB register number as the destination index.  */
 	int regnum = gdbarch_dwarf2_reg_to_regnum (gdbarch, column);
 
+	/* Do not handle return address here, it is done in the next
+	   loop. This also avoids problem with pei-x86-64 target
+	   having retaddr_column set to 32 (which is xmm15). */
+	if (column == fs->retaddr_column)
+	  continue;
 	/* If there's no corresponding GDB register, ignore it.  */
 	if (regnum < 0 || regnum >= num_regs)
 	  continue;