[08/15] dwarf_offdie.c: Add locking

Message ID 20250120032041.280173-8-amerey@redhat.com
State Superseded
Headers
Series [01/15] Add eu_search_tree_init |

Commit Message

Aaron Merey Jan. 20, 2025, 3:20 a.m. UTC
  * libdw/dwarf_offdie.c (__libdw_offdie): Use dwarf_lock.

Signed-off-by: Aaron Merey <amerey@redhat.com>

---
 libdw/dwarf_offdie.c | 3 +++
 1 file changed, 3 insertions(+)
  

Comments

Mark Wielaard Jan. 23, 2025, 12:12 a.m. UTC | #1
Hi Aaron,

On Sun, Jan 19, 2025 at 10:20:34PM -0500, Aaron Merey wrote:
> 	* libdw/dwarf_offdie.c (__libdw_offdie): Use dwarf_lock.

What exactly is "protected" here?  If it is just the __libdw_findcu
call, then it seems the lock is too wide/early. It could just be taken
just before that call, the code before it doesn't seem to need to be
under this lock.

> Signed-off-by: Aaron Merey <amerey@redhat.com>
> 
> ---
>  libdw/dwarf_offdie.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libdw/dwarf_offdie.c b/libdw/dwarf_offdie.c
> index 883720de..74eac420 100644
> --- a/libdw/dwarf_offdie.c
> +++ b/libdw/dwarf_offdie.c
> @@ -43,11 +43,13 @@ __libdw_offdie (Dwarf *dbg, Dwarf_Off offset, Dwarf_Die *result,
>    if (dbg == NULL)
>      return NULL;
>  
> +  mutex_lock (dbg->dwarf_lock);
>    Elf_Data *const data = dbg->sectiondata[debug_types ? IDX_debug_types
>  					  : IDX_debug_info];
>    if (data == NULL || offset >= data->d_size)
>      {
>        __libdw_seterrno (DWARF_E_INVALID_DWARF);
> +      mutex_unlock (dbg->dwarf_lock);
>        return NULL;
>      }
>  
> @@ -66,6 +68,7 @@ __libdw_offdie (Dwarf *dbg, Dwarf_Off offset, Dwarf_Die *result,
>        result = NULL;
>      }
>  
> +  mutex_unlock (dbg->dwarf_lock);
>    return result;
>  }
>  
> -- 
> 2.47.1
>
  
Aaron Merey Jan. 24, 2025, 2:18 a.m. UTC | #2
Hi Mark,

On Wed, Jan 22, 2025 at 7:12 PM Mark Wielaard <mark@klomp.org> wrote:
>
> On Sun, Jan 19, 2025 at 10:20:34PM -0500, Aaron Merey wrote:
> >       * libdw/dwarf_offdie.c (__libdw_offdie): Use dwarf_lock.
>
> What exactly is "protected" here?  If it is just the __libdw_findcu
> call, then it seems the lock is too wide/early. It could just be taken
> just before that call, the code before it doesn't seem to need to be
> under this lock.

The tests pass when taking the lock just before __libdw_findcu. Will
update this patch.

Aaron
  

Patch

diff --git a/libdw/dwarf_offdie.c b/libdw/dwarf_offdie.c
index 883720de..74eac420 100644
--- a/libdw/dwarf_offdie.c
+++ b/libdw/dwarf_offdie.c
@@ -43,11 +43,13 @@  __libdw_offdie (Dwarf *dbg, Dwarf_Off offset, Dwarf_Die *result,
   if (dbg == NULL)
     return NULL;
 
+  mutex_lock (dbg->dwarf_lock);
   Elf_Data *const data = dbg->sectiondata[debug_types ? IDX_debug_types
 					  : IDX_debug_info];
   if (data == NULL || offset >= data->d_size)
     {
       __libdw_seterrno (DWARF_E_INVALID_DWARF);
+      mutex_unlock (dbg->dwarf_lock);
       return NULL;
     }
 
@@ -66,6 +68,7 @@  __libdw_offdie (Dwarf *dbg, Dwarf_Off offset, Dwarf_Die *result,
       result = NULL;
     }
 
+  mutex_unlock (dbg->dwarf_lock);
   return result;
 }