Skip to content

Commit

Permalink
dwarf_loader: Use libdw__lock for dwarf_getlocation(s)
Browse files Browse the repository at this point in the history
Eduard noticed [1] intermittent segmentation faults triggered by caching
done internally in dwarf_getlocation(s).

A binary tree of location information is cached in the CU, and if
multiple threads access it concurrently we can get segmentation faults.

It is possible to compile elfutils with experimental thread-safe
support, but safer for now to add locking to pahole.

No additional overhead in pahole encoding was observed
as a result of these changes.

[1] https://lore.kernel.org/dwarves/[email protected]/

Reported-by: Eduard Zingerman <[email protected]>
Suggested-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: Alan Maguire <[email protected]>
Tested-by: Jiri Olsa <[email protected]>
Acked-by: Eduard Zingerman <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Alexei Starovoitov <[email protected]>
Cc: Andrii Nakryiko <[email protected]>
Cc: Daniel Borkmann <[email protected]>
Cc: Song Liu <[email protected]>
Cc: Yonghong Song <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
  • Loading branch information
alan-maguire authored and acmel committed Nov 15, 2024
1 parent da2f7e5 commit 65b7fd6
Showing 1 changed file with 15 additions and 2 deletions.
17 changes: 15 additions & 2 deletions dwarf_loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,14 @@ static bool attr_type(Dwarf_Die *die, uint32_t attr_name, Dwarf_Off *offset)
static int attr_location(Dwarf_Die *die, Dwarf_Op **expr, size_t *exprlen)
{
Dwarf_Attribute attr;
int ret = 1;

if (dwarf_attr(die, DW_AT_location, &attr) != NULL) {
/* use libdw__lock as dwarf_getlocation(s) has concurrency
* issues when libdw is not compiled with experimental
* --enable-thread-safety
*/
pthread_mutex_lock(&libdw__lock);
if (dwarf_getlocation(&attr, expr, exprlen) == 0) {
/* DW_OP_addrx needs additional lookup for real addr. */
if (*exprlen != 0 && expr[0]->atom == DW_OP_addrx) {
Expand All @@ -462,11 +469,12 @@ static int attr_location(Dwarf_Die *die, Dwarf_Op **expr, size_t *exprlen)

expr[0]->number = address;
}
return 0;
ret = 0;
}
pthread_mutex_unlock(&libdw__lock);
}

return 1;
return ret;
}

/* The struct dwarf_tag has a fixed size while the 'struct tag' is just the base
Expand Down Expand Up @@ -1194,6 +1202,10 @@ static int parameter__reg(Dwarf_Attribute *attr, int expected_reg)
int loc_num = -1;
int ret = -1;

/* use libdw__lock as dwarf_getlocation(s) has concurrency issues
* when libdw is not compiled with experimental --enable-thread-safety
*/
pthread_mutex_lock(&libdw__lock);
while ((offset = __dwarf_getlocations(attr, offset, &base, &start, &end, &expr, &exprlen)) > 0) {
loc_num++;

Expand Down Expand Up @@ -1230,6 +1242,7 @@ static int parameter__reg(Dwarf_Attribute *attr, int expected_reg)
}
}
out:
pthread_mutex_unlock(&libdw__lock);
return ret;
}

Expand Down

0 comments on commit 65b7fd6

Please sign in to comment.