Hi Mark,
On Wed, Oct 28, 2026 at 10:05 AM Mark Wielaard <[email protected]> wrote:
>
> Hi Aaron,
>
> On Tue, 2026-10-27 at 20:34 -0400, Aaron Merey wrote:
> > Use a CFI cache's corresponding dwfl load_lock to serialize its
> > initialization. Rename __libdwfl_set_cfi to __libdwfl_set_cfi_locked
> > to indicate that the caller must hold the load_lock. Atomics are used
> > to acquire an initialized CFI cache without needing to obtain the
> > load_lock.
>
> This is slightly more complicated because __libdwfl_set_cfi is used to
> set either the Dwfl_Module eh_cfi or dwarf_cfi field and we assume the
> caller provides the correct/matching arguments. Luckily there are only
> two callers of __libdwfl_set_cfi, so it is easy to double check.
>
> > Signed-off-by: Aaron Merey <[email protected]>
> > ---
> > libdwfl/dwfl_module.c | 2 +-
> > libdwfl/dwfl_module_dwarf_cfi.c | 25 ++++++++++++++++++-------
> > libdwfl/dwfl_module_eh_cfi.c | 26 ++++++++++++++++++--------
> > libdwfl/libdwflP.h | 7 ++++---
> > 4 files changed, 41 insertions(+), 19 deletions(-)
> >
> > diff --git a/libdwfl/dwfl_module.c b/libdwfl/dwfl_module.c
> > index b4ddf806..27e6c064 100644
> > --- a/libdwfl/dwfl_module.c
> > +++ b/libdwfl/dwfl_module.c
> > @@ -74,7 +74,7 @@ __libdwfl_module_free (Dwfl_Module *mod)
> > }
> >
> > /* We might have primed the Dwarf_CFI ebl cache with our own ebl
> > - in __libdwfl_set_cfi. Make sure we don't free it twice. */
> > + in __libdwfl_set_cfi_locked. Make sure we don't free it twice. */
> > if (mod->eh_cfi != NULL)
> > {
> > if (mod->eh_cfi->ebl != NULL && mod->eh_cfi->ebl == mod->ebl)
>
> OK, just comment update, __libdwfl_module_free doesn't need any
> locking.
>
> > diff --git a/libdwfl/dwfl_module_dwarf_cfi.c
> > b/libdwfl/dwfl_module_dwarf_cfi.c
> > index 05a14f7c..2da02b32 100644
> > --- a/libdwfl/dwfl_module_dwarf_cfi.c
> > +++ b/libdwfl/dwfl_module_dwarf_cfi.c
> > @@ -35,7 +35,7 @@
> >
> > Dwarf_CFI *
> > internal_function
> > -__libdwfl_set_cfi (Dwfl_Module *mod, Dwarf_CFI **slot, Dwarf_CFI *cfi)
> > +__libdwfl_set_cfi_locked (Dwfl_Module *mod, Dwarf_CFI **slot, Dwarf_CFI
> > *cfi)
> > {
> > if (cfi != NULL && cfi->ebl == NULL)
> > {
> > @@ -51,7 +51,9 @@ __libdwfl_set_cfi (Dwfl_Module *mod, Dwarf_CFI **slot,
> > Dwarf_CFI *cfi)
> > }
> > }
> >
> > - return *slot = cfi;
> > + if (cfi != NULL)
> > + atomic_store_release (slot, cfi);
> > + return cfi;
> > }
>
> OK, name changed.
> This just uses atomic_store to signal the slot is set.
> No other locking needed.
>
> Maybe there should be a sanity check that
> *slot == &mod->eh_cfi || *slot == &mod->dwarf->cfi?
I will add this to v2.
Aaron
>
> > Dwarf_CFI *
> > @@ -60,14 +62,23 @@ dwfl_module_dwarf_cfi (Dwfl_Module *mod, Dwarf_Addr
> > *bias)
> > if (mod == NULL)
> > return NULL;
> >
> > - if (mod->dwarf_cfi != NULL)
> > + Dwarf_CFI *cfi = atomic_load_acquire (&mod->dwarf_cfi);
> > + if (cfi != NULL)
> > {
> > *bias = dwfl_adjusted_dwarf_addr (mod, 0);
> > - return mod->dwarf_cfi;
> > + return cfi;
> > }
>
> OK, use atomic_load to get the field, return it directly instead of
> reloading through mod.
>
> > - return __libdwfl_set_cfi (mod, &mod->dwarf_cfi,
> > - INTUSE(dwarf_getcfi)
> > - (INTUSE(dwfl_module_getdwarf) (mod, bias)));
> > + mutex_lock (mod->dwfl->load_lock);
> > + cfi = mod->dwarf_cfi;
> > + if (cfi == NULL)
> > + cfi = __libdwfl_set_cfi_locked (mod, &mod->dwarf_cfi,
> > + INTUSE(dwarf_getcfi)
> > + (INTUSE(dwfl_module_getdwarf) (mod,
> > bias)));
> > + else
> > + *bias = dwfl_adjusted_dwarf_addr (mod, 0);
> > +
> > + mutex_unlock (mod->dwfl->load_lock);
> > + return cfi;
>
> OK, get the lock, reload the dwarf_cfi through mod to see if someone
> raced passed us and set it.
>
> > }
> > INTDEF (dwfl_module_dwarf_cfi)
> > diff --git a/libdwfl/dwfl_module_eh_cfi.c b/libdwfl/dwfl_module_eh_cfi.c
> > index aac6657d..cc28ad60 100644
> > --- a/libdwfl/dwfl_module_eh_cfi.c
> > +++ b/libdwfl/dwfl_module_eh_cfi.c
> > @@ -39,21 +39,31 @@ dwfl_module_eh_cfi (Dwfl_Module *mod, Dwarf_Addr *bias)
> > if (mod == NULL)
> > return NULL;
> >
> > - if (mod->eh_cfi != NULL)
> > + Dwarf_CFI *cfi = atomic_load_acquire (&mod->eh_cfi);
> > + if (cfi != NULL)
> > {
> > *bias = dwfl_adjusted_address (mod, 0);
> > - return mod->eh_cfi;
> > + return cfi;
> > }
>
> OK, like above but for eh_cfi field.
>
> > - __libdwfl_getelf (mod);
> > - if (mod->elferr != DWFL_E_NOERROR)
> > + mutex_lock (mod->dwfl->load_lock);
> > + cfi = mod->eh_cfi;
> > + if (cfi == NULL)
> > {
> > - __libdwfl_seterrno (mod->elferr);
> > - return NULL;
> > + __libdwfl_getelf (mod);
> > + if (mod->elferr != DWFL_E_NOERROR)
> > + {
> > + __libdwfl_seterrno (mod->elferr);
> > + mutex_unlock (mod->dwfl->load_lock);
> > + return NULL;
> > + }
> > +
> > + cfi = __libdwfl_set_cfi_locked (mod, &mod->eh_cfi,
> > + INTUSE(dwarf_getcfi_elf)
> > (mod->main.elf));
> > }
> >
> > + mutex_unlock (mod->dwfl->load_lock);
> > *bias = dwfl_adjusted_address (mod, 0);
> > - return __libdwfl_set_cfi (mod, &mod->eh_cfi,
> > - INTUSE(dwarf_getcfi_elf) (mod->main.elf));
> > + return cfi;
> > }
> > INTDEF (dwfl_module_eh_cfi)
>
> OK, likewise.
>
> > diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
> > index 7a181029..56014ff6 100644
> > --- a/libdwfl/libdwflP.h
> > +++ b/libdwfl/libdwflP.h
> > @@ -521,9 +521,10 @@ extern Dwfl_Error __libdwfl_relocate_value
> > (Dwfl_Module *mod, Elf *elf,
> > /* Ensure that MOD->ebl is set up. */
> > extern Dwfl_Error __libdwfl_module_getebl (Dwfl_Module *mod)
> > internal_function;
> >
> > -/* Install a new Dwarf_CFI in *SLOT (MOD->eh_cfi or MOD->dwarf_cfi). */
> > -extern Dwarf_CFI *__libdwfl_set_cfi (Dwfl_Module *mod, Dwarf_CFI **slot,
> > - Dwarf_CFI *cfi)
> > +/* Install a new Dwarf_CFI in *SLOT (MOD->eh_cfi or MOD->dwarf_cfi).
> > + MOD->dwfl->load_lock must be held by the caller. */
> > +extern Dwarf_CFI *__libdwfl_set_cfi_locked (Dwfl_Module *mod, Dwarf_CFI
> > **slot,
> > + Dwarf_CFI *cfi)
> > internal_function;
> >
> > /* Iterate through all the CU's in the module. Start by passing a null
>
> And rename it.
>
> Looks ok and correct.
>
> Thanks,
>
> Mark
>
