[07/10,v3] libdw: Make libdw_find_split_unit thread-safe
Commit Message
From: Heather McIntyre <hsm2@rice.edu>
* libdw/libdwP.h (struct Dwarf_CU): Add split_lock.
* libdw/libdw_find_split_unit.c (__libdw_find_split_unit):
Add locking.
Signed-off-by: Heather S. McIntyre <hsm2@rice.edu>
Signed-off-by: Aaron Merey <amerey@redhat.com>
Signed-off-by: Mark Wielaard <mark@klomp.org>
---
v3 changes:
Move Dwarf split_lock code from patch 3/9 v2 to this patch.
Assign return value before releasing lock.
libdw/dwarf_end.c | 1 +
libdw/libdwP.h | 3 +++
libdw/libdw_find_split_unit.c | 15 +++++++++++++--
libdw/libdw_findcu.c | 1 +
4 files changed, 18 insertions(+), 2 deletions(-)
Comments
Hi,
On Fri, Aug 02, 2024 at 07:38:06PM -0400, Aaron Merey wrote:
> From: Heather McIntyre <hsm2@rice.edu>
>
> * libdw/libdwP.h (struct Dwarf_CU): Add split_lock.
> * libdw/libdw_find_split_unit.c (__libdw_find_split_unit):
> Add locking.
>
> Signed-off-by: Heather S. McIntyre <hsm2@rice.edu>
> Signed-off-by: Aaron Merey <amerey@redhat.com>
> Signed-off-by: Mark Wielaard <mark@klomp.org>
>
> ---
> v3 changes:
> Move Dwarf split_lock code from patch 3/9 v2 to this patch.
> Assign return value before releasing lock.
Looks good this way.
Thanks,
Mark
> libdw/dwarf_end.c | 1 +
> libdw/libdwP.h | 3 +++
> libdw/libdw_find_split_unit.c | 15 +++++++++++++--
> libdw/libdw_findcu.c | 1 +
> 4 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
> index 487d6ce7..ee3ed74e 100644
> --- a/libdw/dwarf_end.c
> +++ b/libdw/dwarf_end.c
> @@ -69,6 +69,7 @@ cu_free (void *arg)
> {
> Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
> rwlock_fini (p->abbrev_lock);
> + rwlock_fini (p->split_lock);
>
> /* Free split dwarf one way (from skeleton to split). */
> if (p->unit_type == DW_UT_skeleton
> diff --git a/libdw/libdwP.h b/libdw/libdwP.h
> index 48c34871..8f942433 100644
> --- a/libdw/libdwP.h
> +++ b/libdw/libdwP.h
> @@ -454,6 +454,9 @@ struct Dwarf_CU
> /* Synchronize Dwarf_Die abbrev access. */
> rwlock_define(, abbrev_lock);
>
> + /* Synchronize split Dwarf access. */
> + rwlock_define(, split_lock);
> +
> /* Memory boundaries of this CU. */
> void *startp;
> void *endp;
> diff --git a/libdw/libdw_find_split_unit.c b/libdw/libdw_find_split_unit.c
> index 4005793b..f4f34c5d 100644
> --- a/libdw/libdw_find_split_unit.c
> +++ b/libdw/libdw_find_split_unit.c
> @@ -150,9 +150,18 @@ Dwarf_CU *
> internal_function
> __libdw_find_split_unit (Dwarf_CU *cu)
> {
> + /* Set result before releasing the lock. */
> + Dwarf_CU *result;
> +
> + rwlock_wrlock(cu->split_lock);
> +
> /* Only try once. */
> if (cu->split != (Dwarf_CU *) -1)
> - return cu->split;
> + {
> + result = cu->split;
> + rwlock_unlock(cu->split_lock);
> + return result;
> + }
>
> /* We need a skeleton unit with a comp_dir and [GNU_]dwo_name attributes.
> The split unit will be the first in the dwo file and should have the
> @@ -207,5 +216,7 @@ __libdw_find_split_unit (Dwarf_CU *cu)
> if (cu->split == (Dwarf_CU *) -1)
> cu->split = NULL;
>
> - return cu->split;
> + result = cu->split;
> + rwlock_unlock(cu->split_lock);
> + return result;
> }
> diff --git a/libdw/libdw_findcu.c b/libdw/libdw_findcu.c
> index 0f46d777..c74e895e 100644
> --- a/libdw/libdw_findcu.c
> +++ b/libdw/libdw_findcu.c
> @@ -178,6 +178,7 @@ __libdw_intern_next_unit (Dwarf *dbg, bool debug_types)
> newp->endp = data->d_buf + newp->end;
> eu_search_tree_init (&newp->locs_tree);
> rwlock_init (newp->abbrev_lock);
> + rwlock_init (newp->split_lock);
>
> /* v4 debug type units have version == 4 and unit_type == DW_UT_type. */
> if (debug_types)
> --
> 2.45.2
>
Hi Mark,
On Fri, Aug 16, 2024 at 5:14 PM Mark Wielaard <mark@klomp.org> wrote:
>
> On Fri, Aug 02, 2024 at 07:38:06PM -0400, Aaron Merey wrote:
> > From: Heather McIntyre <hsm2@rice.edu>
> >
> > * libdw/libdwP.h (struct Dwarf_CU): Add split_lock.
> > * libdw/libdw_find_split_unit.c (__libdw_find_split_unit):
> > Add locking.
> >
> > Signed-off-by: Heather S. McIntyre <hsm2@rice.edu>
> > Signed-off-by: Aaron Merey <amerey@redhat.com>
> > Signed-off-by: Mark Wielaard <mark@klomp.org>
> >
> > ---
> > v3 changes:
> > Move Dwarf split_lock code from patch 3/9 v2 to this patch.
> > Assign return value before releasing lock.
>
> Looks good this way.
Thanks, pushed as commit 2630bce7476.
Aaron
@@ -69,6 +69,7 @@ cu_free (void *arg)
{
Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
rwlock_fini (p->abbrev_lock);
+ rwlock_fini (p->split_lock);
/* Free split dwarf one way (from skeleton to split). */
if (p->unit_type == DW_UT_skeleton
@@ -454,6 +454,9 @@ struct Dwarf_CU
/* Synchronize Dwarf_Die abbrev access. */
rwlock_define(, abbrev_lock);
+ /* Synchronize split Dwarf access. */
+ rwlock_define(, split_lock);
+
/* Memory boundaries of this CU. */
void *startp;
void *endp;
@@ -150,9 +150,18 @@ Dwarf_CU *
internal_function
__libdw_find_split_unit (Dwarf_CU *cu)
{
+ /* Set result before releasing the lock. */
+ Dwarf_CU *result;
+
+ rwlock_wrlock(cu->split_lock);
+
/* Only try once. */
if (cu->split != (Dwarf_CU *) -1)
- return cu->split;
+ {
+ result = cu->split;
+ rwlock_unlock(cu->split_lock);
+ return result;
+ }
/* We need a skeleton unit with a comp_dir and [GNU_]dwo_name attributes.
The split unit will be the first in the dwo file and should have the
@@ -207,5 +216,7 @@ __libdw_find_split_unit (Dwarf_CU *cu)
if (cu->split == (Dwarf_CU *) -1)
cu->split = NULL;
- return cu->split;
+ result = cu->split;
+ rwlock_unlock(cu->split_lock);
+ return result;
}
@@ -178,6 +178,7 @@ __libdw_intern_next_unit (Dwarf *dbg, bool debug_types)
newp->endp = data->d_buf + newp->end;
eu_search_tree_init (&newp->locs_tree);
rwlock_init (newp->abbrev_lock);
+ rwlock_init (newp->split_lock);
/* v4 debug type units have version == 4 and unit_type == DW_UT_type. */
if (debug_types)