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

Reply via email to