Hi Aaron, On Tue, Feb 10, 2026 at 05:24:39PM -0500, Aaron Merey wrote: > On Tue, Feb 10, 2026 at 8:42 AM Mark Wielaard <[email protected]> wrote: > > On Mon, 2026-02-09 at 08:16 -0500, Aaron Merey wrote: > > > - /* 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) > > The issue is this check `last_abbrev_offset != (size_t) -1l`. If > multiple threads have Dwarf_Abbrev_Hash_find return NULL, > last_abbrev_offset != -1 for only the first thread to grab the lock. > The other threads will grab the lock and see last_abbrev_offset == -1. > In this case dwarf_tag returns DWARF_END_ABBREV instead of the abbrev > cached by the first thread.
O right, yes. I missed that -1 is the special case. My theory only works when last_abbrev_offset != -1. So should it be? + /* Check once more in case entry was added before abbrev_lock + was aquired and another thread got to the last_abbrev. */ * if (cu->last_abbrev_offset == (size_t) -1l) + abb = Dwarf_Abbrev_Hash_find (&cu->abbrev_hash, code); * * while (cu->last_abbrev_offset != (size_t) -1l) Then the if (abb == NULL) check can also be dropped. So if we already reached the end (-1), check if it is in the hash table already, otherwise go into the while loop where __libdw_getabbrev will get the value from the hash table if it is already there or parse more abbrevs to find it. Cheers, Mark
