* libdw/dwarf_getsrcfiles.c (dwarf_getsrcfiles): Use dwarf_lock.
* libdw/dwarf_getsrclines.c (dwarf_getsrclines): Ditto.
* libdw/dwarf_macro_getsrclines.c (dwarf_macro_getsrclines):
Ditto.
Signed-off-by: Aaron Merey <ame...@redhat.com>
---
v3 changes:
This patch replaces v2 02/10 and v2 03/10.
The locking to dwarf_filesrc in 02/10 has been removed. The removal of
dwarf_filesrc locking triggered an error from helgrind that is now fixed
by the dwarf_macro_getsrclines locking added in this patch.
On Wed, Feb 12, 2025 at 8:17 AM Mark Wielaard <m...@klomp.org> wrote:
>
> Hi Aaron,
>
> On Tue, 2025-02-04 at 16:50 -0500, Aaron Merey wrote:
> > * libdw/dwarf_getsrcfiles.c (dwarf_getsrcfiles): Use dwarf_lock.
> > * libdw/dwarf_getsrclines.c (dwarf_getsrclines): Use dwarf_lock.
> >
> > Signed-off-by: Aaron Merey <ame...@redhat.com>
> > ---
> > v2 changes: Combined from v1 patches 04/15 and 05/15.
> >
> > libdw/dwarf_getsrcfiles.c | 11 +++++++----
> > libdw/dwarf_getsrclines.c | 25 +++++++++++++++++++------
> > 2 files changed, 26 insertions(+), 10 deletions(-)
>
> OK, so here dwarf_lock is used to guard access to the Dwarf_CU fields
> lines and files. Might it make sense to turn these into a Dwarf_CU
> specific lock?
We could add another Dwarf_CU lock for this purpose. However we'll still
need to use the dwarf_lock when lines or files haven't been read. For now
I'd prefer to keep the number of locks as low as possible and focus on
performance improvements once we've achieved thread safety.
OTOH it shouldn't be too much work to add this cu lock so please let me
know if you'd still like to see it added in this patch.
Aaron
libdw/dwarf_getsrcfiles.c | 11 +++++++----
libdw/dwarf_getsrclines.c | 25 +++++++++++++++++++------
libdw/dwarf_macro_getsrcfiles.c | 4 ++++
3 files changed, 30 insertions(+), 10 deletions(-)
diff --git a/libdw/dwarf_getsrcfiles.c b/libdw/dwarf_getsrcfiles.c
index 24e4b7d2..3029ce69 100644
--- a/libdw/dwarf_getsrcfiles.c
+++ b/libdw/dwarf_getsrcfiles.c
@@ -47,9 +47,10 @@ dwarf_getsrcfiles (Dwarf_Die *cudie, Dwarf_Files **files,
size_t *nfiles)
}
int res = -1;
+ struct Dwarf_CU *const cu = cudie->cu;
+ mutex_lock (cudie->cu->dbg->dwarf_lock);
/* Get the information if it is not already known. */
- struct Dwarf_CU *const cu = cudie->cu;
if (cu->files == NULL)
{
/* For split units there might be a simple file table (without lines).
@@ -96,7 +97,10 @@ dwarf_getsrcfiles (Dwarf_Die *cudie, Dwarf_Files **files,
size_t *nfiles)
Dwarf_Off debug_line_offset;
if (__libdw_formptr (stmt_list, IDX_debug_line, DWARF_E_NO_DEBUG_LINE,
NULL, &debug_line_offset) == NULL)
- return -1;
+ {
+ mutex_unlock (cudie->cu->dbg->dwarf_lock);
+ return -1;
+ }
res = __libdw_getsrcfiles (cu->dbg, debug_line_offset,
__libdw_getcompdir (cudie),
@@ -115,8 +119,7 @@ dwarf_getsrcfiles (Dwarf_Die *cudie, Dwarf_Files **files,
size_t *nfiles)
*nfiles = cu->files->nfiles;
}
- // XXX Eventually: unlocking here.
-
+ mutex_unlock (cudie->cu->dbg->dwarf_lock);
return res;
}
INTDEF (dwarf_getsrcfiles)
diff --git a/libdw/dwarf_getsrclines.c b/libdw/dwarf_getsrclines.c
index da78db67..c2e686b1 100644
--- a/libdw/dwarf_getsrclines.c
+++ b/libdw/dwarf_getsrclines.c
@@ -1442,8 +1442,10 @@ dwarf_getsrclines (Dwarf_Die *cudie, Dwarf_Lines
**lines, size_t *nlines)
return -1;
}
- /* Get the information if it is not already known. */
struct Dwarf_CU *const cu = cudie->cu;
+ mutex_lock (cu->dbg->dwarf_lock);
+
+ /* Get the information if it is not already known. */
if (cu->lines == NULL)
{
/* For split units always pick the lines from the skeleton. */
@@ -1464,10 +1466,13 @@ dwarf_getsrclines (Dwarf_Die *cudie, Dwarf_Lines
**lines, size_t *nlines)
*lines = cu->lines;
*nlines = cu->lines->nlines;
}
+
+ mutex_unlock (cu->dbg->dwarf_lock);
return res;
}
__libdw_seterrno (DWARF_E_NO_DEBUG_LINE);
+ mutex_unlock (cu->dbg->dwarf_lock);
return -1;
}
@@ -1485,21 +1490,29 @@ dwarf_getsrclines (Dwarf_Die *cudie, Dwarf_Lines
**lines, size_t *nlines)
Dwarf_Off debug_line_offset;
if (__libdw_formptr (stmt_list, IDX_debug_line, DWARF_E_NO_DEBUG_LINE,
NULL, &debug_line_offset) == NULL)
- return -1;
+ {
+ mutex_unlock (cu->dbg->dwarf_lock);
+ return -1;
+ }
if (__libdw_getsrclines (cu->dbg, debug_line_offset,
__libdw_getcompdir (cudie),
cu->address_size, &cu->lines, &cu->files) < 0)
- return -1;
+ {
+ mutex_unlock (cu->dbg->dwarf_lock);
+ return -1;
+ }
}
else if (cu->lines == (void *) -1l)
- return -1;
+ {
+ mutex_unlock (cu->dbg->dwarf_lock);
+ return -1;
+ }
*lines = cu->lines;
*nlines = cu->lines->nlines;
- // XXX Eventually: unlocking here.
-
+ mutex_unlock (cu->dbg->dwarf_lock);
return 0;
}
INTDEF(dwarf_getsrclines)
diff --git a/libdw/dwarf_macro_getsrcfiles.c b/libdw/dwarf_macro_getsrcfiles.c
index 5e02935d..8d7c2d03 100644
--- a/libdw/dwarf_macro_getsrcfiles.c
+++ b/libdw/dwarf_macro_getsrcfiles.c
@@ -41,6 +41,8 @@ dwarf_macro_getsrcfiles (Dwarf *dbg, Dwarf_Macro *macro,
/* macro is declared NN */
Dwarf_Macro_Op_Table *const table = macro->table;
+
+ mutex_lock (table->dbg->dwarf_lock);
if (table->files == NULL)
{
Dwarf_Off line_offset = table->line_offset;
@@ -48,6 +50,7 @@ dwarf_macro_getsrcfiles (Dwarf *dbg, Dwarf_Macro *macro,
{
*files = NULL;
*nfiles = 0;
+ mutex_unlock (table->dbg->dwarf_lock);
return 0;
}
@@ -79,6 +82,7 @@ dwarf_macro_getsrcfiles (Dwarf *dbg, Dwarf_Macro *macro,
table->files = (void *) -1;
}
+ mutex_unlock (table->dbg->dwarf_lock);
if (table->files == (void *) -1)
return -1;
--
2.48.1
