This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [patch v7 3/5] x86* unwinder: libdwfl/
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Sat, 26 Oct 2013 23:47:04 +0200
- Subject: Re: [patch v7 3/5] x86* unwinder: libdwfl/
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.
> 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.
> > Sorry for a late suggestion, but I only thought of it now. Before I
> > didn't fully appreciate why you wanted to see the difference between
> > "real error" and "no information available".
> >
> > Since dwfl_thread_getframes returns an int, could we change it to:
>
> Sorry but I do not think it matters too much to differentiate
> CFI-interpretation-error vs. CFI-not-found-error. In most cases we end up on
> CFI-not-found-error because the inferior is not come from RH-distro, therefore
> it does not have -fasynchronous-unwind-tables, and it is stripped, therefore it
> does not have .debug_frame. And it is a bug, not end-of-unwind.
>
> For the cases where we end up with CFI-not-found-error where the backtrace
> should have been really finished we should fix the inferior CFI. We are on
> Free OS so we can do so.
>
> CFI-interpretation-error will not happen in practice I think to have to deal
> with it somehow.
>
> I understand your patch makes sense but I think it rather needlessly
> complicates the situation. Do you still want to get it applied?
No, I think you are right. It complicates things. And we should indeed
just fix the rest of the toolchain :)
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).
> + 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).
All else looks fine.
Thanks,
Mark