Re: [PATCH v2 dwarves 2/2] dwarf_loader: use libdw__lock for

[Eduard Zingerman <eddyz87@gmail.com>]

On Thu, 2024-11-14 at 15:58 +0000, Alan Maguire wrote:
> 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.
> 
> Reported-by: Eduard Zingerman <eddyz87@gmail.com>
> Suggested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> 
> [1] https://lore.kernel.org/dwarves/080794545d8eb3df3d6eba90ac621111ab7171f5.camel@gmail.com/
> ---

Looking at the libdw code, both dwarf_getlocations() and
dwarf_getlocation() might end up calling __libdw_intern_expression(),
where race condition occurs. So I think that this lock positioning
should be safe. In theory, the locking could be pushed down,
to only occur around __dwarf_getlocations / dwarf_getlocation,
but that would complicate the code a bit.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]