libbacktrace: walk the elf_syminfo_data chain in elf_syminfo

[Ian Lance Taylor]

On Mon, Jul 22, 2013 at 11:20 AM, Alexander Monakov <amonakov@ispras.ru> wrote:
> On Mon, 22 Jul 2013, Ian Lance Taylor wrote:
>> Thanks for noticing the problem.  This patch isn't enough by itself.
>> The code has to protect itself against the list changing in
>> mid-stream.  See dwarf_fileline in dwarf.c.
>
> Thank you.  Here's the updated patch, however I have to admit it's not
> entirely clear to me what __sync_bool_compare_and_swap should achieve.  Is it
> only to ensure that we do not use a partially updated pointer (which shouldn't
> happen with a naturally aligned pointer on hardware that updates cache lines
> atomically)?

We not only need to get a valid pointer, we need to ensure when thread
T1 creates the pointer and thread T2 loads the pointer, thread T2 sees
all the global stores that thread T1 made before T1 set the pointer.
That is, we need an acquire-load.  The hope here is that any sane
implementation of __sync_bool_compare_and_swap would ensure sequential
consistency between T1 and T2, implying an acquire-load.  On x86 every
load is an acquire-load, but that is not true on other processors.

If we added some configure tests, or didn't worry about letting people
use this library with older versions of GCC, we could use
__atomic_load_n (pp, __ATOMIC_ACQUIRE) here, and use something like
__atomic_compare_exchange_n (pp, NULL, edata, true, __ATOMIC_RELEASE,
__ATOMIC_RELAXED) in elf_add_syminfo_data.  I don't think it would
make any difference on x86, though.

> 2013-07-22  Alexander Monakov  <amonakov@ispras.ru>
>
>         * elf.c (elf_syminfo): Loop over the elf_syminfo_data chain.


> +      for (edata = (struct elf_syminfo_data *) state->syminfo_data;
> +          edata;

Please explicitly write edata != NULL.

This is OK with that change.

Thanks.

Ian