This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [PATCH] libdw: Fix __libdw_form_val_len endp pointer.
- From: Josh Stone <jistone at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Thu, 11 Dec 2014 15:34:17 -0800
- Subject: Re: [PATCH] libdw: Fix __libdw_form_val_len endp pointer.
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 <mjw@redhat.com>
> ---
> 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 <mjw@redhat.com>
> +
> + * 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 <jistone@redhat.com>
>
> * 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. */
>