Hi Aaron, On Wed, Sep 30, 2026 at 03:06:46PM -0400, Aaron Merey wrote: > __libdw_getabbrev may reparse a cached abbrev when argument lengthp is > non-NULL. This reparsing is the same as for an uncached abbrev seen for > the first time. The fields of the abbrev are written to whether or > not they've already been set. > > This can create a data race for multithreaded use cases. Fix this by > not writing to the abbrev if it is already cached.
Aha, the comment does say "This must not be a problem, since the content better be the same", but without synchronization overwriting memory might show up differently in another thread. > Signed-off-by: Aaron Merey <[email protected]> > --- > libdw/dwarf_getabbrev.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/libdw/dwarf_getabbrev.c b/libdw/dwarf_getabbrev.c > index d9a6c022..26cf38e8 100644 > --- a/libdw/dwarf_getabbrev.c > +++ b/libdw/dwarf_getabbrev.c > @@ -107,18 +107,26 @@ __libdw_getabbrev (Dwarf *dbg, struct Dwarf_CU *cu, > Dwarf_Off offset, > goto out; > } > > - /* If there is already a value in the hash table we are going to > - overwrite its content. This must not be a problem, since the > - content better be the same. */ > - abb->code = code; Right, remove write of code and comment (because it is subtly wrong). > if (abbrevp >= end) > goto invalid; > - get_uleb128 (abb->tag, abbrevp, end); > + > + unsigned int tag; > + get_uleb128 (tag, abbrevp, end); > + OK, read tag in local var. > if (abbrevp + 1 >= end) > goto invalid; > - abb->has_children = *abbrevp++ == DW_CHILDREN_yes; > - abb->attrp = (unsigned char *) abbrevp; > - abb->offset = offset; > + > + bool has_children = *abbrevp++ == DW_CHILDREN_yes; OK, read has_children into local var, but don't write to abb. > + /* Set the entry's fields if it was just allocated. */ > + if (! foundit) > + { > + abb->code = code; > + abb->tag = tag; > + abb->has_children = has_children; > + abb->attrp = (unsigned char *) abbrevp; > + abb->offset = offset; > + } OK, only write abb fields if it is a newly allocated Dwarf_Abbrev. Looks correct. Thanks, Mark
