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: Mon, 28 Oct 2013 16:53:09 +0100
- Subject: Re: [patch v7 3/5] x86* unwinder: libdwfl/
On Mon, 2013-10-28 at 16:16 +0100, Jan Kratochvil wrote:
> > > + if (op->atom == DW_OP_deref_size)
> > > + {
> > > + if (op->number > 8)
> > > + {
> > > + free (stack);
> > > + __libdwfl_seterrno (DWFL_E_INVALID_DWARF);
> > > + return false;
> > > + }
> >
> > This is fine, but to be pedantically correct (in an error case), you
> > could check against the actual address_size, which can be gotten from
> > the cfi->e_ident[EI_CLASS] == ELFCLASS32 ? 4 : 8.
>
> It is not even just the pedanticality, there was I think a bug for big endian
> 32-bit hosts.
Missed that. Good catch.
> > > 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 */
> >
> > I think an assert is in order here.
>
> What if you run on 32-bit host, foreign 32-bit process run-time patches its
> ELF header to be ELFCLASS64 and you run eu-stack on it? I think you would
> assert. This is dependency on external data so I believe there should not be
> an assert.
And again you are correct.
Thanks,
Mark