[04/10,v3] libdw: Add rwlocks for Dwarf and Dwarf_CU
Commit Message
From: Heather McIntyre <hsm2@rice.edu>
Add dwarf_lock for Dwarf as well as abbrev_lock for Dwarf_CU.
* libdw/dwarf_begin_elf.c (dwarf_begin_elf): Init dwarf_lock.
* libdw/dwarf_end.c (cu_free): Free abbrev_lock.
(dwarf_end): Free dwarf_lock.
* libdw/libdwP.h (struct Dwarf): Define dwarf_lock.
(struct Dwarf_CU): Define abbrev_lock.
* libdw/libdw_findcu.c (__libdw_intern_next_unit): Init
abbrev_lock.
Signed-off-by: Heather S. McIntyre <hsm2@rice.edu>
Signed-off-by: Aaron Merey <amerey@redhat.com>
---
v3 changes:
New patch added in v3 that contains rwlock changes previously in patch 3/9 v2.
libdw/dwarf_begin_elf.c | 1 +
libdw/dwarf_end.c | 2 ++
libdw/libdwP.h | 7 +++++++
libdw/libdw_findcu.c | 1 +
4 files changed, 11 insertions(+)
Comments
Hi Aaron,
On Fri, 2024-08-02 at 19:38 -0400, Aaron Merey wrote:
> From: Heather McIntyre <hsm2@rice.edu>
>
> Add dwarf_lock for Dwarf as well as abbrev_lock for Dwarf_CU.
>
> * libdw/dwarf_begin_elf.c (dwarf_begin_elf): Init dwarf_lock.
> * libdw/dwarf_end.c (cu_free): Free abbrev_lock.
> (dwarf_end): Free dwarf_lock.
> * libdw/libdwP.h (struct Dwarf): Define dwarf_lock.
> (struct Dwarf_CU): Define abbrev_lock.
> * libdw/libdw_findcu.c (__libdw_intern_next_unit): Init
> abbrev_lock.
>
> Signed-off-by: Heather S. McIntyre <hsm2@rice.edu>
> Signed-off-by: Aaron Merey <amerey@redhat.com>
>
> ---
> v3 changes:
> New patch added in v3 that contains rwlock changes previously in patch 3/9 v2.
The patch itself looks correct. But I think I would have introduced the
locks separately and/or in the first patch that used them (dwarf_getalt
for the dwarf_lock, dwarf_formref_die and __libdw_dieabbrev for
abbrev_lock).
It might also be a good idea to explicitly document which structure
fields are "protected" by the separate locks to make reasoning about
them a little easier.
Cheers,
Mark
Hi Mark,
On Fri, Aug 16, 2024 at 9:38 AM Mark Wielaard <mark@klomp.org> wrote:
>
> Hi Aaron,
>
> On Fri, 2024-08-02 at 19:38 -0400, Aaron Merey wrote:
> > From: Heather McIntyre <hsm2@rice.edu>
> >
> > Add dwarf_lock for Dwarf as well as abbrev_lock for Dwarf_CU.
> >
> > * libdw/dwarf_begin_elf.c (dwarf_begin_elf): Init dwarf_lock.
> > * libdw/dwarf_end.c (cu_free): Free abbrev_lock.
> > (dwarf_end): Free dwarf_lock.
> > * libdw/libdwP.h (struct Dwarf): Define dwarf_lock.
> > (struct Dwarf_CU): Define abbrev_lock.
> > * libdw/libdw_findcu.c (__libdw_intern_next_unit): Init
> > abbrev_lock.
> >
> > Signed-off-by: Heather S. McIntyre <hsm2@rice.edu>
> > Signed-off-by: Aaron Merey <amerey@redhat.com>
> >
> > ---
> > v3 changes:
> > New patch added in v3 that contains rwlock changes previously in patch 3/9 v2.
>
> The patch itself looks correct. But I think I would have introduced the
> locks separately and/or in the first patch that used them (dwarf_getalt
> for the dwarf_lock, dwarf_formref_die and __libdw_dieabbrev for
> abbrev_lock).
>
> It might also be a good idea to explicitly document which structure
> fields are "protected" by the separate locks to make reasoning about
> them a little easier.
Done. The dwarf_lock changes were squashed into commit 7c4fcff44ae and
the abbrev_lock changes were squashed into commit 28b74a1bf73.
Aaron
@@ -579,6 +579,7 @@ dwarf_begin_elf (Elf *elf, Dwarf_Cmd cmd, Elf_Scn *scngrp)
__libdw_seterrno (DWARF_E_NOMEM); /* no memory. */
return NULL;
}
+ rwlock_init (result->dwarf_lock);
result->mem_stacks = 0;
result->mem_tails = NULL;
@@ -68,6 +68,7 @@ cu_free (void *arg)
&& p != p->dbg->fake_addr_cu)
{
Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
+ rwlock_fini (p->abbrev_lock);
/* Free split dwarf one way (from skeleton to split). */
if (p->unit_type == DW_UT_skeleton
@@ -127,6 +128,7 @@ dwarf_end (Dwarf *dwarf)
if (dwarf->mem_tails != NULL)
free (dwarf->mem_tails);
pthread_rwlock_destroy (&dwarf->mem_rwl);
+ rwlock_fini (dwarf->dwarf_lock);
/* Free the pubnames helper structure. */
free (dwarf->pubnames_sets);
@@ -264,6 +264,10 @@ struct Dwarf
allocations for this Dwarf. */
pthread_rwlock_t mem_rwl;
+ /* The dwarf_lock is a read-write lock designed to ensure thread-safe access
+ and modification of Dwarf objects. */
+ rwlock_define(, dwarf_lock);
+
/* Internal memory handling. This is basically a simplified thread-local
reimplementation of obstacks. Unfortunately the standard obstack
implementation is not usable in libraries. */
@@ -447,6 +451,9 @@ struct Dwarf_CU
Don't access directly, call __libdw_cu_locs_base. */
Dwarf_Off locs_base;
+ /* Synchronize Dwarf_Die abbrev access. */
+ rwlock_define(, abbrev_lock);
+
/* Memory boundaries of this CU. */
void *startp;
void *endp;
@@ -177,6 +177,7 @@ __libdw_intern_next_unit (Dwarf *dbg, bool debug_types)
newp->startp = data->d_buf + newp->start;
newp->endp = data->d_buf + newp->end;
eu_search_tree_init (&newp->locs_tree);
+ rwlock_init (newp->abbrev_lock);
/* v4 debug type units have version == 4 and unit_type == DW_UT_type. */
if (debug_types)