Hi Aaron, On Mon, 2026-02-09 at 08:16 -0500, Aaron Merey wrote: > Acquire abbrev_lock before reading cu->last_abbrev_offset in dwarf_tag.
This seems correct. Getting and setting the cu->last_abbrev_offset value should only be done under a lock. > Also double check for abbrev hash table entry once lock is held to > avoid multiple threads attempting to populate the abbrev hash table. But I think this isn't necessary because __libdw_getabbrev already does that. > Signed-off-by: Aaron Merey <[email protected]> > --- > libdw/dwarf_tag.c | 48 ++++++++++++++++++++++++++++------------------- > 1 file changed, 29 insertions(+), 19 deletions(-) > > diff --git a/libdw/dwarf_tag.c b/libdw/dwarf_tag.c > index 7daa4813..69e3bdc2 100644 > --- a/libdw/dwarf_tag.c > +++ b/libdw/dwarf_tag.c > @@ -47,30 +47,40 @@ __libdw_findabbrev (struct Dwarf_CU *cu, unsigned int > code) > /* See whether the entry is already in the hash table. */ > abb = Dwarf_Abbrev_Hash_find (&cu->abbrev_hash, code); > if (abb == NULL) > - while (cu->last_abbrev_offset != (size_t) -1l) > - { > - size_t length; > + { > + mutex_lock (cu->abbrev_lock); This is good. > > - /* Find the next entry. It gets automatically added to the > - hash table. */ > - mutex_lock (cu->abbrev_lock); > - abb = __libdw_getabbrev (cu->dbg, cu, cu->last_abbrev_offset, &length); > + /* Check once more in case entry was added before abbrev_lock > + was aquired. */ > + abb = Dwarf_Abbrev_Hash_find (&cu->abbrev_hash, code); > > - if (abb == NULL || abb == DWARF_END_ABBREV) > + if (abb == NULL) But this part seems unnecessary because... > + while (cu->last_abbrev_offset != (size_t) -1l) > { > - /* Make sure we do not try to search for it again. */ > - cu->last_abbrev_offset = (size_t) -1l; > - mutex_unlock (cu->abbrev_lock); > - return DWARF_END_ABBREV; > + size_t length; > + > + /* Find the next entry. It gets automatically added to the > + hash table. */ > + abb = __libdw_getabbrev (cu->dbg, cu, cu->last_abbrev_offset, > + &length); This will work just fine if the abbrev is already in the hash table. > + > + if (abb == NULL || abb == DWARF_END_ABBREV) > + { > + /* Make sure we do not try to search for it again. */ > + cu->last_abbrev_offset = (size_t) -1l; > + mutex_unlock (cu->abbrev_lock); > + return DWARF_END_ABBREV; > + } > + > + cu->last_abbrev_offset += length; > + > + /* Is this the code we are looking for? */ > + if (abb->code == code) > + break; > } > > - cu->last_abbrev_offset += length; > - mutex_unlock (cu->abbrev_lock); > - > - /* Is this the code we are looking for? */ > - if (abb->code == code) > - break; > - } > + mutex_unlock (cu->abbrev_lock); > + } All good. > > /* This is our second (or third, etc.) call to __libdw_findabbrev > and the code is invalid. */ Cheers, Mark
