Re: [PATCH 2/3] libdw: Rewrite the memory handler to be thread-safe.

[Mark Wielaard]

Hi,

On Mon, 2019-10-21 at 11:28 -0500, Jonathon Anderson wrote:
> On Mon, Oct 21, 2019 at 18:13, Mark Wielaard <m...@klomp.org> wrote:
> > 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.

Lets avoid the "strong" language about people. But lets see if we can
make the load less "strong" for the atomics :)

I think we cannot use the atomic_load () function, but have to use
atomic_load_explicit. So it would become:

diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index fc573cb3..a2e94436 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -95,7 +95,10 @@ 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_explicit
+                                       (&dwarf->mem_tail,
+                                        memory_order_relaxed));
       while (memp != NULL)
        {
          struct libdw_memblock *prevp = memp->prev;

Cheers,

Mark