On 12/11/2014 03:05 PM, Mark Wielaard wrote: > While rebasing my work on top of Josh attrabbrev reading speedups > I noticed a typo/thinko in my form_val_len bound checking patch. > > We should check against the die->cu->endp, not the abbrev endp.
In that case, I don't think you need endp as a parameter at all. Just use cu->endp as needed, and prevent this mistake from ever happening. Maybe you could also check valp >= cu->startp while you're at it, but that's probably an ok invariant to just assume. Also along these lines, the dbg parameter could just use cu->dbg as needed. Saving parameters probably won't do much for performance in this case, nor is changing to a pointer read likely to hurt. It does help code clarity though, IMO. > > Signed-off-by: Mark Wielaard <[email protected]> > --- > libdw/ChangeLog | 6 ++++++ > libdw/dwarf_child.c | 2 +- > libdw/dwarf_getattrs.c | 2 +- > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libdw/ChangeLog b/libdw/ChangeLog > index 69592a7..8b00970 100644 > --- a/libdw/ChangeLog > +++ b/libdw/ChangeLog > @@ -1,3 +1,9 @@ > +2014-12-11 Mark Wielaard <[email protected]> > + > + * dwarf_child.c (__libdw_find_attr): Call __libdw_form_val_len with > + die->cu->endp, not the abbrev endp. > + * dwarf_getattrs.c (dwarf_getattrs): Likewise. > + > 2014-12-10 Josh Stone <[email protected]> > > * libdwP.h (Dwarf_CU): Add startp and endp boundaries. > diff --git a/libdw/dwarf_child.c b/libdw/dwarf_child.c > index 2a5d379..46d010d 100644 > --- a/libdw/dwarf_child.c > +++ b/libdw/dwarf_child.c > @@ -95,7 +95,7 @@ __libdw_find_attr (Dwarf_Die *die, unsigned int search_name, > if (attr_form != 0) > { > size_t len = __libdw_form_val_len (dbg, die->cu, attr_form, readp, > - endp); > + die->cu->endp); > > if (unlikely (len == (size_t) -1l)) > { > diff --git a/libdw/dwarf_getattrs.c b/libdw/dwarf_getattrs.c > index 9ea70fc..f6453c7 100644 > --- a/libdw/dwarf_getattrs.c > +++ b/libdw/dwarf_getattrs.c > @@ -107,7 +107,7 @@ dwarf_getattrs (Dwarf_Die *die, int (*callback) > (Dwarf_Attribute *, void *), > if (attr.form != 0) > { > size_t len = __libdw_form_val_len (dbg, die->cu, attr.form, > - die_addr, endp); > + die_addr, die->cu->endp); > > if (unlikely (len == (size_t) -1l)) > /* Something wrong with the file. */ >
