diff mbox series

[6/9,v2] libdw: Make libdw_find_split_unit thread-safe

Message ID 20240717223409.1788719-7-amerey@redhat.com
State Superseded
Headers
Series Fix thread-safety for elfutils |

Commit Message

Aaron Merey July 17, 2024, 10:34 p.m. UTC 
  From: Heather McIntyre <hsm2@rice.edu>

	* (__libdw_find_split_unit): Add lock for cu->split.

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:
Locking applied to __libdw_find_split_unit instead of try_split_file.

 libdw/libdw_find_split_unit.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Mark Wielaard July 18, 2024, 9:19 p.m. UTC | #1 
Hi,

On Wed, Jul 17, 2024 at 06:34:05PM -0400, Aaron Merey wrote:
> From: Heather McIntyre <hsm2@rice.edu>
> 
> 	* (__libdw_find_split_unit): Add lock for cu->split.
> 
> 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:
> Locking applied to __libdw_find_split_unit instead of try_split_file.
> 
>  libdw/libdw_find_split_unit.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Would be nice to have the split_lock definition from patch "lib: Add
eu_tsearch, eu_tfind, eu_tdelete and eu_tdestroy " here to make this
patch self-contained.

> diff --git a/libdw/libdw_find_split_unit.c b/libdw/libdw_find_split_unit.c
> index 67d31a9c..eb3d88d7 100644
> --- a/libdw/libdw_find_split_unit.c
> +++ b/libdw/libdw_find_split_unit.c
> @@ -150,9 +150,14 @@ Dwarf_CU *
>  internal_function
>  __libdw_find_split_unit (Dwarf_CU *cu)
>  {
> +  rwlock_wrlock(cu->split_lock);
> +
>    /* Only try once.  */
>    if (cu->split != (Dwarf_CU *) -1)
> -    return cu->split;
> +    {
> +      rwlock_unlock(cu->split_lock);
> +      return cu->split;
> +    }
>  
>    /* 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 +212,6 @@ __libdw_find_split_unit (Dwarf_CU *cu)
>    if (cu->split == (Dwarf_CU *) -1)
>      cu->split = NULL;
>  
> +  rwlock_unlock(cu->split_lock);
>    return cu->split;
>  }

Again, just as a style issue, because it looks like the unlocked read
is fine in this case, but I really like the Dwarf_CU *result; result =
cu->split; unlock; return result; idiom here.

Cheers,

Mark
diff mbox series

Patch

diff --git a/libdw/libdw_find_split_unit.c b/libdw/libdw_find_split_unit.c
index 67d31a9c..eb3d88d7 100644
--- a/libdw/libdw_find_split_unit.c
+++ b/libdw/libdw_find_split_unit.c
@@ -150,9 +150,14 @@  Dwarf_CU *
 internal_function
 __libdw_find_split_unit (Dwarf_CU *cu)
 {
+  rwlock_wrlock(cu->split_lock);
+
   /* Only try once.  */
   if (cu->split != (Dwarf_CU *) -1)
-    return cu->split;
+    {
+      rwlock_unlock(cu->split_lock);
+      return cu->split;
+    }
 
   /* 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 +212,6 @@  __libdw_find_split_unit (Dwarf_CU *cu)
   if (cu->split == (Dwarf_CU *) -1)
     cu->split = NULL;
 
+  rwlock_unlock(cu->split_lock);
   return cu->split;
 }