[7/9,v2] libdw: Make libdw_findcu thread-safe
Commit Message
From: Heather McIntyre <hsm2@rice.edu>
* libdw/libdw_findcu.c (__libdw_findcu): Use eu_tfind
and dwarf_lock
(__libdw_intern_next_unit): Use per-Dwarf_CU locks.
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>
---
v2 changes:
Use per-Dwarf_CU lock instead of a global lock.
libdw/libdw_findcu.c | 47 ++++++++++++++++++++++++++++----------------
1 file changed, 30 insertions(+), 17 deletions(-)
Comments
Hi,
On Wed, 2024-07-17 at 18:34 -0400, Aaron Merey wrote:
> From: Heather McIntyre <hsm2@rice.edu>
>
> * libdw/libdw_findcu.c (__libdw_findcu): Use eu_tfind
> and dwarf_lock
> (__libdw_intern_next_unit): Use per-Dwarf_CU locks.
>
> 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>
>
> ---
> v2 changes:
> Use per-Dwarf_CU lock instead of a global lock.
Are you sure this description and the ChangeLog entry are correct?
This patch doesn't contain a change to use eu_tfind (there is already
one). And it seems to use a per-Dwarf lock, not a per-Dwarf_CU lock.
> libdw/libdw_findcu.c | 47 ++++++++++++++++++++++++++++----------------
> 1 file changed, 30 insertions(+), 17 deletions(-)
>
> diff --git a/libdw/libdw_findcu.c b/libdw/libdw_findcu.c
> index 72cf261c..8acff448 100644
> --- a/libdw/libdw_findcu.c
> +++ b/libdw/libdw_findcu.c
> @@ -177,6 +177,8 @@ __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);
> + rwlock_init (newp->split_lock);
>
> /* v4 debug type units have version == 4 and unit_type == DW_UT_type. */
> if (debug_types)
Neither of these locks are used in the rest of the patch.
> @@ -243,27 +245,38 @@ __libdw_findcu (Dwarf *dbg, Dwarf_Off start, bool v4_debug_types)
> /* Maybe we already know that CU. */
> struct Dwarf_CU fake = { .start = start, .end = 0 };
> struct Dwarf_CU **found = eu_tfind (&fake, tree, findcu_cb);
> + struct Dwarf_CU *result = NULL;
> if (found != NULL)
> return *found;
>
> - if (start < *next_offset)
> - {
> - __libdw_seterrno (DWARF_E_INVALID_DWARF);
> - return NULL;
> - }
> + rwlock_wrlock (dbg->dwarf_lock);
>
> - /* No. Then read more CUs. */
> - while (1)
> - {
> - struct Dwarf_CU *newp = __libdw_intern_next_unit (dbg, v4_debug_types);
> - if (newp == NULL)
> - return NULL;
> -
> - /* Is this the one we are looking for? */
> - if (start < *next_offset || start == newp->start)
> - return newp;
> - }
> - /* NOTREACHED */
> + if (start < *next_offset)
> + __libdw_seterrno (DWARF_E_INVALID_DWARF);
> + else
> + {
> + /* No. Then read more CUs. */
> + while (1)
> + {
> + struct Dwarf_CU *newp = __libdw_intern_next_unit (dbg,
> + v4_debug_types);
> + if (newp == NULL)
> + {
> + result = NULL;
> + break;
> + }
> +
> + /* Is this the one we are looking for? */
> + if (start < *next_offset || start == newp->start)
> + {
> + result = newp;
> + break;
> + }
> + }
> + }
> +
> + rwlock_unlock (dbg->dwarf_lock);
> + return result;
> }
>
> struct Dwarf_CU *
This uses the global Dwarf structure lock, not a per Dwarf_CU lock.
Cheers,
Mark
@@ -177,6 +177,8 @@ __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);
+ rwlock_init (newp->split_lock);
/* v4 debug type units have version == 4 and unit_type == DW_UT_type. */
if (debug_types)
@@ -243,27 +245,38 @@ __libdw_findcu (Dwarf *dbg, Dwarf_Off start, bool v4_debug_types)
/* Maybe we already know that CU. */
struct Dwarf_CU fake = { .start = start, .end = 0 };
struct Dwarf_CU **found = eu_tfind (&fake, tree, findcu_cb);
+ struct Dwarf_CU *result = NULL;
if (found != NULL)
return *found;
- if (start < *next_offset)
- {
- __libdw_seterrno (DWARF_E_INVALID_DWARF);
- return NULL;
- }
+ rwlock_wrlock (dbg->dwarf_lock);
- /* No. Then read more CUs. */
- while (1)
- {
- struct Dwarf_CU *newp = __libdw_intern_next_unit (dbg, v4_debug_types);
- if (newp == NULL)
- return NULL;
-
- /* Is this the one we are looking for? */
- if (start < *next_offset || start == newp->start)
- return newp;
- }
- /* NOTREACHED */
+ if (start < *next_offset)
+ __libdw_seterrno (DWARF_E_INVALID_DWARF);
+ else
+ {
+ /* No. Then read more CUs. */
+ while (1)
+ {
+ struct Dwarf_CU *newp = __libdw_intern_next_unit (dbg,
+ v4_debug_types);
+ if (newp == NULL)
+ {
+ result = NULL;
+ break;
+ }
+
+ /* Is this the one we are looking for? */
+ if (start < *next_offset || start == newp->start)
+ {
+ result = newp;
+ break;
+ }
+ }
+ }
+
+ rwlock_unlock (dbg->dwarf_lock);
+ return result;
}
struct Dwarf_CU *