Hi Aaron,
On Tue, 2025-02-04 at 16:50 -0500, Aaron Merey wrote:
> * libdw/dwarf_getsrclines.c (read_srcfiles): Initialize Dwarf
> member.
> * libdw/dwarf_filesrc.c (dwarf_filesrc): Use dwarf_lock.
> * libdw/libdwP.h (struct Dwarf_Files_s): Add Dwarf member.
>
> Signed-off-by: Aaron Merey <ame...@redhat.com>
> ---
> v2 changes: Combined v1 patches 03/15 and 06/15 into one patch.
>
> libdw/dwarf_filesrc.c | 13 +++++++++++--
> libdw/dwarf_getsrclines.c | 2 ++
> libdw/libdwP.h | 1 +
> 3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/libdw/dwarf_filesrc.c b/libdw/dwarf_filesrc.c
> index d866ce72..a0881f36 100644
> --- a/libdw/dwarf_filesrc.c
> +++ b/libdw/dwarf_filesrc.c
> @@ -38,14 +38,23 @@ const char *
> dwarf_filesrc (Dwarf_Files *file, size_t idx, Dwarf_Word *mtime,
> Dwarf_Word *length)
> {
> - if (file == NULL || idx >= file->nfiles)
> + if (file == NULL)
> return NULL;
>
> + mutex_lock (file->dbg->dwarf_lock);
> + if (idx >= file->nfiles)
> + {
> + mutex_unlock (file->dbg->dwarf_lock);
> + return NULL;
> + }
> +
> if (mtime != NULL)
> *mtime = file->info[idx].mtime;
>
> if (length != NULL)
> *length = file->info[idx].length;
>
> - return file->info[idx].name;
> + const char *res = file->info[idx].name;
> + mutex_unlock (file->dbg->dwarf_lock);
> + return res;
> }
Sorry if we already discussed this, I obviously forgot.
But why is this locking needed?
Isn't Dwarf_Files fully populated when the user calls this function?
The contents of the object will not be changed in any other thread at
this point, or does it?
> diff --git a/libdw/dwarf_getsrclines.c b/libdw/dwarf_getsrclines.c
> index da78db67..b0b5f2eb 100644
> --- a/libdw/dwarf_getsrclines.c
> +++ b/libdw/dwarf_getsrclines.c
> @@ -717,6 +717,7 @@ read_srcfiles (Dwarf *dbg,
> if (unlikely (files == NULL))
> goto no_mem;
>
> + files->dbg = dbg;
> const char **dirs = (void *) &files->info[nfilelist];
>
> struct filelist *fileslist = filelist;
> @@ -1198,6 +1199,7 @@ read_srclines (Dwarf *dbg,
> + (ndirs + 1) * sizeof (char *),
> 1);
>
> + newfiles->dbg = dbg;
>
> /* Copy prevfiles to newfiles. */
> for (size_t n = 0; n < nprevfiles; n++)
> diff --git a/libdw/libdwP.h b/libdw/libdwP.h
> index 9d2ffab0..a6bd7e5b 100644
> --- a/libdw/libdwP.h
> +++ b/libdw/libdwP.h
> @@ -305,6 +305,7 @@ struct Dwarf_Abbrev
> /* Files in line information records. */
> struct Dwarf_Files_s
> {
> + Dwarf *dbg;
> unsigned int ndirs;
> unsigned int nfiles;
> struct Dwarf_Fileinfo_s
So the extra Dwarf dbg field is to have a lock (the dwarf_lock dbg
field) that is only used in dwarf_filesrc? If so, I think it isn't
necessary. But maybe I am confused.
Cheers,
Mark
