Re: [patch v7 3/5] x86* unwinder: libdwfl/

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Sat, 26 Oct 2013 23:47:04 +0200, Mark Wielaard wrote:
> On Fri, 2013-10-25 at 18:22 +0200, Jan Kratochvil wrote:
> > I have implemented all the missing ops except for:
> > +      case DW_OP_GNU_encoded_addr:
> > +       /* Missing support in the rest of elfutils.  */
> > +      case DW_OP_deref_size:
> > +       /* Missing appropriate callback.  */
> 
> Do we need to extend the memory_read callback to take a size argument?
> Or can we do something clever (depending on byte order) with the current
> memory_read of a full word and chop it down? The size will never be
> bigger than the number of bytes in an address. So it will never be
> bigger than 8.

OK, true, implemented DW_OP_deref_size.


> > BTW elfutils should really get DW_OP_GNU_encoded_addr implemented I guess as it
> > may appear in CFI.
> 
> It is indeed documented (at the end of)
> https://sourceware.org/binutils/docs-2.22/as/CFI-directives.html
> .cfi_val_encoded_addr will generate a DW_OP_GNU_encoded_addr according
> to http://gcc.gnu.org/ml/gcc-patches/2008-09/msg01713.html
> But I haven't actually seen it used. Would be nice to have an example
> usage.

It is true that gcc/dwarf2cfi.c does not use it, if I read it correctly.


> If users are really interested they could do some checks on the last
> frame address to see if it actually falls inside a known Module, has a
> Dwarf/CFI and whether that CFI has a range covering the address (if they
> really cared).

OK, so left as is.


> > +      case DW_OP_rot:
> > +	{
> > +	  Dwarf_Addr val3;
> > +	if (! pop (&val1) || ! pop (&val2) || ! pop (&val3)
> > +	    || ! push (val1) || ! push (val2) || ! push (val3))
> > +	  {
> > +	    free (stack);
> > +	    return false;
> > +	  }
> > +	}
> > +	break;
> 
> I think that should be: push (val1), push (val3), push (val2).

That's bad, I cannot even copy the GDB code.

So I have finally run the elfutils unwinder on Jakub's
gcc/testsuite/gcc.dg/cleanup-13.c and it really found DW_OP_rot is buggy.
Besides that it found also DW_OP_pick had a bug (fixed it below).
And then it works (patched it with the simple attached patch).

Just I do not know if cleanup-13.c is covered by GPL3 or if it is public
domain but I guess the former.  In such case I do not think it is compatible
with elfutils due to its LGPL option.


Thanks,
Jan

--- /home/jkratoch/t/cleanup-13.c-orig	2010-01-21 07:01:45.000000000 +0100
+++ /home/jkratoch/t/cleanup-13.c	2013-10-27 11:31:16.556145134 +0100
@@ -281,6 +281,7 @@ extern char verify_it[sizeof (cfi_arch_p
 		: : "i" (sizeof (cfi_arch_program)))
 #endif
 #endif
+#include <unistd.h>
 static _Unwind_Reason_Code
 force_unwind_stop (int version, _Unwind_Action actions,
 		   _Unwind_Exception_Class exc_class,
@@ -289,7 +290,10 @@ force_unwind_stop (int version, _Unwind_
 		   void *stop_parameter)
 {
   if (actions & _UA_END_OF_STACK)
+{
+pause();
     abort ();
+}
   return _URC_NO_REASON;
 }
 

diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
index eca7a04..a875e98 100644
--- a/libdwfl/frame_unwind.c
+++ b/libdwfl/frame_unwind.c
@@ -231,8 +231,13 @@ expr_eval (Dwfl_Frame *state, Dwarf_Frame *frame, const Dwarf_Op *ops,
 	  }
 	break;
       case DW_OP_pick:
-	if (! pop (&val1) || stack_used <= val1
-	    || ! push (stack[stack_used - 1 - val1]))
+	if (stack_used <= op->number)
+	  {
+	    free (stack);
+	    __libdwfl_seterrno (DWFL_E_INVALID_DWARF);
+	    return false;
+	  }
+	if (! push (stack[stack_used - 1 - op->number]))
 	  {
 	    free (stack);
 	    return false;
@@ -257,7 +262,7 @@ expr_eval (Dwfl_Frame *state, Dwarf_Frame *frame, const Dwarf_Op *ops,
 	{
 	  Dwarf_Addr val3;
 	if (! pop (&val1) || ! pop (&val2) || ! pop (&val3)
-	    || ! push (val1) || ! push (val2) || ! push (val3))
+	    || ! push (val1) || ! push (val3) || ! push (val2))
 	  {
 	    free (stack);
 	    return false;
@@ -265,6 +270,7 @@ expr_eval (Dwfl_Frame *state, Dwarf_Frame *frame, const Dwarf_Op *ops,
 	}
 	break;
       case DW_OP_deref:
+      case DW_OP_deref_size:
 	if (process->callbacks->memory_read == NULL)
 	  {
 	    free (stack);
@@ -273,17 +279,35 @@ expr_eval (Dwfl_Frame *state, Dwarf_Frame *frame, const Dwarf_Op *ops,
 	  }
 	if (! pop (&val1)
 	    || ! process->callbacks->memory_read (process->dwfl, val1, &val1,
-						  process->callbacks_arg)
-	    || ! push (val1))
+						  process->callbacks_arg))
+	  {
+	    free (stack);
+	    return false;
+	  }
+	if (op->atom == DW_OP_deref_size)
+	  {
+	    if (op->number > 8)
+	      {
+		free (stack);
+		__libdwfl_seterrno (DWFL_E_INVALID_DWARF);
+		return false;
+	      }
+#if BYTE_ORDER == BIG_ENDIAN
+	    if (op->number == 0)
+	      val1 = 0;
+	    else
+	      val1 >>= 64 - op->number * 8;
+#else
+	    if (op->number < 8)
+	      val1 &= (1 << (op->number * 8)) - 1;
+#endif
+	  }
+	if (! push (val1))
 	  {
 	    free (stack);
 	    return false;
 	  }
 	break;
-      case DW_OP_deref_size:
-	/* Missing appropriate callback.  */
-	__libdwfl_seterrno (DWFL_E_UNSUPPORTED_DWARF);
-	return false;
 #define UNOP(atom, expr)						\
       case atom:							\
 	if (! pop (&val1) || ! push (expr))				\
diff --git a/libdwfl/linux-pid-attach.c b/libdwfl/linux-pid-attach.c
index 0721c88..66a0814 100644
--- a/libdwfl/linux-pid-attach.c
+++ b/libdwfl/linux-pid-attach.c
@@ -122,9 +122,14 @@ pid_memory_read (Dwfl *dwfl, Dwarf_Addr addr, Dwarf_Word *result, void *arg)
   Dwfl_Process *process = dwfl->process;
   if (ebl_get_elfclass (process->ebl) == ELFCLASS64)
     {
+#if SIZEOF_LONG == 8
       errno = 0;
       *result = ptrace (PTRACE_PEEKDATA, tid, (void *) (uintptr_t) addr, NULL);
       return errno == 0;
+#else /* SIZEOF_LONG != 8 */
+      /* This should not happen.  */
+      return false;
+#endif /* SIZEOF_LONG != 8 */
     }
 #if SIZEOF_LONG == 8
   /* We do not care about reads unaliged to 4 bytes boundary.