[5/9,v2] libdwP.h: Add locking to __libdw_dieabbrev,
Commit Message
From: Heather McIntyre <hsm2@rice.edu>
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:
This replaces patch "libdw: Add locking around __libdw_dieabbrev for
dwarf_hasattr". Mark suggested that we remove lazy abbrev reading
in order to simplify the locking of __libdw_dieabbrev. This is
a fair bit of work so for now lets just put a write lock around all
of __libdw_dieabbrev. The removal of lazy reading of abbrev will be
done in a future patch.
libdw/dwarf_formref_die.c | 2 ++
libdw/dwarf_setalt.c | 4 ++++
libdw/libdwP.h | 23 +++++++++++++++++++----
3 files changed, 25 insertions(+), 4 deletions(-)
Comments
Hi,
On Wed, Jul 17, 2024 at 06:34:04PM -0400, Aaron Merey wrote:
> From: Heather McIntyre <hsm2@rice.edu>
>
> 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:
> This replaces patch "libdw: Add locking around __libdw_dieabbrev for
> dwarf_hasattr". Mark suggested that we remove lazy abbrev reading
> in order to simplify the locking of __libdw_dieabbrev. This is
> a fair bit of work so for now lets just put a write lock around all
> of __libdw_dieabbrev. The removal of lazy reading of abbrev will be
> done in a future patch.
I have to think a bit about this. It sounds expensive to have to wrap
each abbrev read in a lock.
> diff --git a/libdw/dwarf_setalt.c b/libdw/dwarf_setalt.c
> index dc9b61cb..f7d70d9d 100644
> --- a/libdw/dwarf_setalt.c
> +++ b/libdw/dwarf_setalt.c
> @@ -35,6 +35,8 @@
> void
> dwarf_setalt (Dwarf *main, Dwarf *alt)
> {
> + rwlock_wrlock(main->dwarf_lock);
> +
> if (main->alt_fd != -1)
> {
> INTUSE(dwarf_end) (main->alt_dwarf);
> @@ -43,5 +45,7 @@ dwarf_setalt (Dwarf *main, Dwarf *alt)
> }
>
> main->alt_dwarf = alt;
> +
> + rwlock_unlock(main->dwarf_lock);
> }
> INTDEF (dwarf_setalt)
This part should go with the previous patch "libdw: make dwarf_getalt
thread-safe".
Cheers,
Mark
@@ -92,7 +92,9 @@ dwarf_formref_die (Dwarf_Attribute *attr, Dwarf_Die *result)
bool scan_debug_types = false;
do
{
+ rwlock_wrlock(attr->cu->dbg->dwarf_lock);
cu = __libdw_intern_next_unit (attr->cu->dbg, scan_debug_types);
+ rwlock_unlock(attr->cu->dbg->dwarf_lock);
if (cu == NULL)
{
if (scan_debug_types == false)
@@ -35,6 +35,8 @@
void
dwarf_setalt (Dwarf *main, Dwarf *alt)
{
+ rwlock_wrlock(main->dwarf_lock);
+
if (main->alt_fd != -1)
{
INTUSE(dwarf_end) (main->alt_dwarf);
@@ -43,5 +45,7 @@ dwarf_setalt (Dwarf *main, Dwarf *alt)
}
main->alt_dwarf = alt;
+
+ rwlock_unlock(main->dwarf_lock);
}
INTDEF (dwarf_setalt)
@@ -804,15 +804,28 @@ static inline Dwarf_Abbrev *
__nonnull_attribute__ (1)
__libdw_dieabbrev (Dwarf_Die *die, const unsigned char **readp)
{
+ if (unlikely (die->cu == NULL))
+ {
+ die->abbrev = DWARF_END_ABBREV;
+ return DWARF_END_ABBREV;
+ }
+
+ rwlock_wrlock (die->cu->abbrev_lock);
+
/* Do we need to get the abbreviation, or need to read after the code? */
if (die->abbrev == NULL || readp != NULL)
{
/* Get the abbreviation code. */
unsigned int code;
const unsigned char *addr = die->addr;
- if (unlikely (die->cu == NULL
- || addr >= (const unsigned char *) die->cu->endp))
- return die->abbrev = DWARF_END_ABBREV;
+
+ if (addr >= (const unsigned char *) die->cu->endp)
+ {
+ die->abbrev = DWARF_END_ABBREV;
+ rwlock_unlock (die->cu->abbrev_lock);
+ return DWARF_END_ABBREV;
+ }
+
get_uleb128 (code, addr, die->cu->endp);
if (readp != NULL)
*readp = addr;
@@ -821,7 +834,9 @@ __libdw_dieabbrev (Dwarf_Die *die, const unsigned char **readp)
if (die->abbrev == NULL)
die->abbrev = __libdw_findabbrev (die->cu, code);
}
- return die->abbrev;
+
+ rwlock_unlock (die->cu->abbrev_lock);
+ return (Dwarf_Abbrev *) die->abbrev;
}
/* Helper functions for form handling. */