Hi Aaron,
On Fri, 2026-10-30 at 01:16 -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.
>
> Signed-off-by: Aaron Merey <[email protected]>
> ---
> v2 changes:
> in __libdwfl_set_cfi_locked, return early if slot isn't either &mod->eh_cfi
> or &mod->dwarf_cfi.
Good sanity check. I wouldn't have minded an assert here, because it is
an internal inconsistency, but setting error and returning NULL is
fine.
> Also acquire cfi->lock before setting cfi->ebl.
in __libdwfl_set_cfi_locked. Missed that in v1. Looks correct.
Thanks,
Mark
> libdwfl/dwfl_module.c | 2 +-
> libdwfl/dwfl_module_dwarf_cfi.c | 38 ++++++++++++++++++++++++++-------
> libdwfl/dwfl_module_eh_cfi.c | 26 +++++++++++++++-------
> libdwfl/libdwflP.h | 7 +++---
> 4 files changed, 53 insertions(+), 20 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)
> diff --git a/libdwfl/dwfl_module_dwarf_cfi.c b/libdwfl/dwfl_module_dwarf_cfi.c
> index 05a14f7c..a8b2d9a5 100644
> --- a/libdwfl/dwfl_module_dwarf_cfi.c
> +++ b/libdwfl/dwfl_module_dwarf_cfi.c
> @@ -35,15 +35,26 @@
>
> 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)
> + if (slot != &mod->eh_cfi && slot != &mod->dwarf_cfi)
> + {
> + __libdwfl_seterrno (DWFL_E_INVALID_ARGUMENT);
> + return NULL;
> + }
> +
> + if (cfi == NULL)
> + return NULL;
> +
> + mutex_lock (cfi->lock);
> + if (cfi->ebl == NULL)
> {
> Dwfl_Error error = __libdwfl_module_getebl (mod);
> if (error == DWFL_E_NOERROR)
> cfi->ebl = mod->ebl;
> else
> {
> + mutex_unlock (cfi->lock);
> if (slot == &mod->eh_cfi)
> INTUSE(dwarf_cfi_end) (cfi);
> __libdwfl_seterrno (error);
> @@ -51,7 +62,9 @@ __libdwfl_set_cfi (Dwfl_Module *mod, Dwarf_CFI **slot,
> Dwarf_CFI *cfi)
> }
> }
>
> - return *slot = cfi;
> + atomic_store_release (slot, cfi);
> + mutex_unlock (cfi->lock);
> + return cfi;
> }
>
> Dwarf_CFI *
> @@ -60,14 +73,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;
> }
>
> - 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;
> }
> 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;
> }
>
> - __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)
> diff --git a/libdwfl/libdwflP.h b/libdwfl/libdwflP.h
> index 6e2c2f8b..3b4df7e8 100644
> --- a/libdwfl/libdwflP.h
> +++ b/libdwfl/libdwflP.h
> @@ -529,9 +529,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
