Hi Heather,
On Tue, 2023-10-10 at 15:42 +0200, Mark Wielaard wrote:
> From: Heather McIntyre <h...@rice.edu>
>
> * libdw/dwarf_hasattr.c (dwarf_hasattr): Use die_abbrev_lock
> around __libdw_dieabbrev call.
>
> Signed-off-by: Heather S. McIntyre <h...@rice.edu>
> Signed-off-by: Mark Wielaard <m...@klomp.org>
> ---
> libdw/dwarf_hasattr.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/libdw/dwarf_hasattr.c b/libdw/dwarf_hasattr.c
> index eca08394..92f8de68 100644
> --- a/libdw/dwarf_hasattr.c
> +++ b/libdw/dwarf_hasattr.c
> @@ -34,6 +34,10 @@
> #include <dwarf.h>
> #include "libdwP.h"
>
> +/* dwarf_hasattr() calls __libdw_dieabbrev() in libdwP.h.
> + __libdw_dieabbrev() reads/writes "die->abbrev".
> + Mutual exclusion is enforced around the call to __libdw_dieabbrev to
> prevent a race. */
> +rwlock_define(static, die_abbrev_lock);
dwarf_child, dwarf_getattrs, dwarf_haschildren and dwarf_tag also use
__libdw_dieabbrev to get the Dwarf_Abbrev pointer for the given
Dwarf_DIE. Shouldn't they also use such locking? Or have the locking
inside __libdw_dieabbrev itself?
Also there are many Dwarf_Dies which all start out "lazy" without
abbrev set. So taking a global static lock, or even taking any pthread
lock at all might be a big overhead. Is there some way we can do this
with atomics instead?
>
> int
> dwarf_hasattr (Dwarf_Die *die, unsigned int search_name)
> @@ -41,8 +45,13 @@ dwarf_hasattr (Dwarf_Die *die, unsigned int search_name)
> if (die == NULL)
> return 0;
>
> + rwlock_wrlock(die_abbrev_lock);
> +
> /* Find the abbreviation entry. */
> Dwarf_Abbrev *abbrevp = __libdw_dieabbrev (die, NULL);
> +
> + rwlock_unlock(die_abbrev_lock);
> +
> if (unlikely (abbrevp == DWARF_END_ABBREV))
> {
> __libdw_seterrno (DWARF_E_INVALID_DWARF);
