[09/15] dwarf_onesrcline.c: Add locking

Message ID 20250120032041.280173-9-amerey@redhat.com
State Dropped
Delegated to: Mark Wielaard
Headers
Series [01/15] Add eu_search_tree_init |

Commit Message

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

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

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

Comments

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

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

Is this really necessary? When the user calls dwarf_onesrcline
wouldn't they already have a fully setup Dwarf_Lines? Can it still
change concurrently?

> Signed-off-by: Aaron Merey <amerey@redhat.com>
> 
> ---
>  libdw/dwarf_onesrcline.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libdw/dwarf_onesrcline.c b/libdw/dwarf_onesrcline.c
> index 5d3c3ded..1a16269a 100644
> --- a/libdw/dwarf_onesrcline.c
> +++ b/libdw/dwarf_onesrcline.c
> @@ -40,11 +40,16 @@ dwarf_onesrcline (Dwarf_Lines *lines, size_t idx)
>    if (lines == NULL)
>      return NULL;
>  
> +  mutex_lock (lines->dbg->dwarf_lock);
> +
>    if (idx >= lines->nlines)
>      {
>        __libdw_seterrno (DWARF_E_INVALID_LINE_IDX);
>        return NULL;
>      }
>  
> -  return &lines->info[idx];
> +  Dwarf_Line *result = &lines->info[idx];
> +  mutex_unlock (lines->dbg->dwarf_lock);
> +
> +  return result;
>  }
> -- 
> 2.47.1
>
  
Aaron Merey Jan. 24, 2025, 2:19 a.m. UTC | #2
Hi Mark,

On Wed, Jan 22, 2025 at 7:15 PM Mark Wielaard <mark@klomp.org> wrote:
>
> On Sun, Jan 19, 2025 at 10:20:35PM -0500, Aaron Merey wrote:
> >       * libdw/dwarf_onesrcline.c (dwarf_onesrcline): Use dwarf_lock.
>
> Is this really necessary? When the user calls dwarf_onesrcline
> wouldn't they already have a fully setup Dwarf_Lines? Can it still
> change concurrently?

Hmm as with dwarf_lineaddr, I had previously seen helgrind flag
dwarf_onesrcline during testing but I am unable to reproduce an
error with these locks commented out.  I'm not sure why this is.

I'll drop this patch until I can prove there is a need for it.

Aaron
  

Patch

diff --git a/libdw/dwarf_onesrcline.c b/libdw/dwarf_onesrcline.c
index 5d3c3ded..1a16269a 100644
--- a/libdw/dwarf_onesrcline.c
+++ b/libdw/dwarf_onesrcline.c
@@ -40,11 +40,16 @@  dwarf_onesrcline (Dwarf_Lines *lines, size_t idx)
   if (lines == NULL)
     return NULL;
 
+  mutex_lock (lines->dbg->dwarf_lock);
+
   if (idx >= lines->nlines)
     {
       __libdw_seterrno (DWARF_E_INVALID_LINE_IDX);
       return NULL;
     }
 
-  return &lines->info[idx];
+  Dwarf_Line *result = &lines->info[idx];
+  mutex_unlock (lines->dbg->dwarf_lock);
+
+  return result;
 }