[PATCH 2/3] libdw: Rewrite the memory handler to be thread-safe.
Mark Wielaard
mark@klomp.org
Mon Oct 21 16:13:00 GMT 2019
Hi,
Finally back to this patch series.
On Thu, 2019-08-29 at 15:16 +0200, Mark Wielaard wrote:
> diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
> index 29795c10..fc573cb3 100644
> --- a/libdw/dwarf_end.c
> +++ b/libdw/dwarf_end.c
> @@ -94,14 +94,15 @@ dwarf_end (Dwarf *dwarf)
> /* And the split Dwarf. */
> tdestroy (dwarf->split_tree, noop_free);
>
> - struct libdw_memblock *memp = dwarf->mem_tail;
> - /* The first block is allocated together with the Dwarf object. */
> - while (memp->prev != NULL)
> + /* Free the internally allocated memory. */
> + struct libdw_memblock *memp = (struct libdw_memblock *)dwarf->mem_tail;
> + while (memp != NULL)
> {
> struct libdw_memblock *prevp = memp->prev;
> free (memp);
> memp = prevp;
> }
> + pthread_key_delete (dwarf->mem_key);
>
> /* Free the pubnames helper structure. */
> free (dwarf->pubnames_sets);
This doesn't build on older GCCs (I am using 4.8.5) with this compile error:
libdw/dwarf_end.c: In function ‘dwarf_end’:
libdw/dwarf_end.c:98:45: error: cannot convert to a pointer type
struct libdw_memblock *memp = (struct libdw_memblock *)dwarf->mem_tail;
^
This is because mem_tail is defined as:
> diff --git a/libdw/libdwP.h b/libdw/libdwP.h
> index eebb7d12..ad2599eb 100644
> --- a/libdw/libdwP.h
> +++ b/libdw/libdwP.h
> @@ -218,16 +231,11 @@ struct Dwarf
> /* Similar for addrx/constx, which will come from .debug_addr section. */
> struct Dwarf_CU *fake_addr_cu;
>
> - /* Internal memory handling. This is basically a simplified
> - reimplementation of obstacks. Unfortunately the standard obstack
> - implementation is not usable in libraries. */
> - struct libdw_memblock
> - {
> - size_t size;
> - size_t remaining;
> - struct libdw_memblock *prev;
> - char mem[0];
> - } *mem_tail;
> + /* Internal memory handling. Each thread allocates separately and only
> + allocates from its own blocks, while all the blocks are pushed atomically
> + onto a unified stack for easy deallocation. */
> + pthread_key_t mem_key;
> + atomic_uintptr_t mem_tail;
>
> /* Default size of allocated memory blocks. */
> size_t mem_default_size;
And for older compilers, without stdatomic.h, this means atomic_uintptr_t is really:
typedef _Atomic(uintptr_t) atomic_uintptr_t;
#define _Atomic(T) struct { volatile __typeof__(T) __val; }
And you cannot cast a struct to a pointer type directly.
To make this work on both older and newer gcc versions I changed this to:
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index fc573cb3..76ab9954 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -95,7 +95,8 @@ dwarf_end (Dwarf *dwarf)
tdestroy (dwarf->split_tree, noop_free);
/* Free the internally allocated memory. */
- struct libdw_memblock *memp = (struct libdw_memblock *)dwarf->mem_tail;
+ struct libdw_memblock *memp;
+ memp = (struct libdw_memblock *) atomic_load (&dwarf->mem_tail);
while (memp != NULL)
{
struct libdw_memblock *prevp = memp->prev;
Does that look reasonable?
Thanks,
Mark
More information about the Elfutils-devel
mailing list