On Mon, Oct 21, 2019 at 18:13, Mark Wielaard <m...@klomp.org> wrote:
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;
^
Ah, whoops. Thanks for catching that one.
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?
It does, although I would prefer:
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index 9ca17212..6da9e0cd 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -95,7 +95,9 @@ 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,
+
memory_order_relaxed);
while (memp != NULL)
{
struct libdw_memblock *prevp = memp->prev;
Because some idiot thought making seq_cst the default was a good idea.
And this way it notes in the code that this load is non-synchronizing.
-Jonathon
Thanks,
Mark
