This is the mail archive of the
mailing list for the GCC project.
Re: libbacktrace: walk the elf_syminfo_data chain in elf_syminfo
- From: Ian Lance Taylor <iant at google dot com>
- To: Alexander Monakov <amonakov at ispras dot ru>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Mon, 22 Jul 2013 13:23:19 -0700
- Subject: Re: libbacktrace: walk the elf_syminfo_data chain in elf_syminfo
- References: <alpine dot LNX dot 2 dot 00 dot 1307221718150 dot 1061 at monopod dot intra dot ispras dot ru> <CAKOQZ8z016SmW9i3EuuV4s_TmCi2YYwL51k+HgYz=O9T=DpPtA at mail dot gmail dot com> <alpine dot LNX dot 2 dot 00 dot 1307222213520 dot 1061 at monopod dot intra dot ispras dot ru>
On Mon, Jul 22, 2013 at 11:20 AM, Alexander Monakov <email@example.com> 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
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 <firstname.lastname@example.org>
> * 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.