Hi Mark,
On Wed, Oct 28, 2026 at 11:22 AM Mark Wielaard <[email protected]> wrote:
>
> Hi Aaron,
>
> On Tue, 2026-10-27 at 20:34 -0400, Aaron Merey wrote:
> > Store a Dwfl_Module's ebl field atomically in __libdwfl_module_getebl
> > while holding the associated Dwfl load_lock.
> >
> > Change usage of the ebl field so that before using it's either been
> > atomically read or else it's used after __libdwfl_module_getebl returns
> > DWFL_E_NOERROR.
>
> Why not always use __libdwfl_module_getebl ?
>
> > Signed-off-by: Aaron Merey <[email protected]>
> > ---
> > libdwfl/dwfl_module_getdwarf.c | 24 +++++++++++++++------
> > libdwfl/dwfl_module_getsym.c | 3 ++-
> > libdwfl/dwfl_module_register_names.c | 11 ++++------
> > libdwfl/dwfl_module_return_value_location.c | 11 ++++------
> > 4 files changed, 28 insertions(+), 21 deletions(-)
> >
> > diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwarf.c
> > index 247fbbd2..cb11b1f3 100644
> > --- a/libdwfl/dwfl_module_getdwarf.c
> > +++ b/libdwfl/dwfl_module_getdwarf.c
> > @@ -1345,17 +1345,29 @@ Dwfl_Error
> > internal_function
> > __libdwfl_module_getebl (Dwfl_Module *mod)
> > {
> > + if (likely (atomic_load_acquire (&mod->ebl) != NULL))
> > + return DWFL_E_NOERROR;
> > +
> > + Dwfl_Error ret = DWFL_E_NOERROR;
> > + mutex_lock (mod->dwfl->load_lock);
> > +
> > if (mod->ebl == NULL)
> > {
> > __libdwfl_getelf (mod);
> > - if (mod->elferr != DWFL_E_NOERROR)
> > - return mod->elferr;
> > + ret = mod->elferr;
> >
> > - mod->ebl = ebl_openbackend (mod->main.elf);
> > - if (mod->ebl == NULL)
> > - return DWFL_E_LIBEBL;
> > + if (ret == DWFL_E_NOERROR)
> > + {
> > + Ebl *ebl = ebl_openbackend (mod->main.elf);
> > + if (ebl == NULL)
> > + ret = DWFL_E_LIBEBL;
> > + else
> > + atomic_store_release (&mod->ebl, ebl);
> > + }
> > }
> > - return DWFL_E_NOERROR;
> > +
> > + mutex_unlock (mod->dwfl->load_lock);
> > + return ret;
> > }
> >
> > /* Try to start up libdw on DEBUGFILE. */
>
> OK, the usual dance.
>
> > diff --git a/libdwfl/dwfl_module_getsym.c b/libdwfl/dwfl_module_getsym.c
> > index 2bb07fd8..4905f019 100644
> > --- a/libdwfl/dwfl_module_getsym.c
> > +++ b/libdwfl/dwfl_module_getsym.c
> > @@ -124,7 +124,8 @@ __libdwfl_getsym (Dwfl_Module *mod, int ndx, GElf_Sym
> > *sym, GElf_Addr *addr,
> > descriptors). */
> >
> > char *ident;
> > - GElf_Addr st_value = sym->st_value & ebl_func_addr_mask (mod->ebl);
> > + Ebl *ebl = atomic_load_acquire (&mod->ebl);
> > + GElf_Addr st_value = sym->st_value & ebl_func_addr_mask (ebl);
> > *resolved = false;
> > if (! adjust_st_value && mod->e_type != ET_REL && alloc
> > && (GELF_ST_TYPE (sym->st_info) == STT_FUNC
>
> So why not use __libdwfl_module_getebl here?
This preserves existing behaviour. If mod->ebl is NULL then the mask
from ebl_func_addr_mask leaves the symbol value unchanged. If we use
__libdwfl_module_getebl then the backend could get initialized early
resulting in a different mask.
Aaron
>
> > diff --git a/libdwfl/dwfl_module_register_names.c
> > b/libdwfl/dwfl_module_register_names.c
> > index 9ea09371..a8aa8a65 100644
> > --- a/libdwfl/dwfl_module_register_names.c
> > +++ b/libdwfl/dwfl_module_register_names.c
> > @@ -43,14 +43,11 @@ dwfl_module_register_names (Dwfl_Module *mod,
> > if (unlikely (mod == NULL))
> > return -1;
> >
> > - if (unlikely (mod->ebl == NULL))
> > + Dwfl_Error error = __libdwfl_module_getebl (mod);
> > + if (unlikely (error != DWFL_E_NOERROR))
> > {
> > - Dwfl_Error error = __libdwfl_module_getebl (mod);
> > - if (error != DWFL_E_NOERROR)
> > - {
> > - __libdwfl_seterrno (error);
> > - return -1;
> > - }
> > + __libdwfl_seterrno (error);
> > + return -1;
> > }
> >
> > int nregs = ebl_register_info (mod->ebl, -1, NULL, 0,
>
> OK, __libdwfl_module_getebl simplifies things here.
>
> > diff --git a/libdwfl/dwfl_module_return_value_location.c
> > b/libdwfl/dwfl_module_return_value_location.c
> > index ff6f56f8..8a3e63a9 100644
> > --- a/libdwfl/dwfl_module_return_value_location.c
> > +++ b/libdwfl/dwfl_module_return_value_location.c
> > @@ -40,14 +40,11 @@ dwfl_module_return_value_location (Dwfl_Module *mod,
> > Dwarf_Die *functypedie,
> > if (mod == NULL)
> > return -1;
> >
> > - if (mod->ebl == NULL)
> > + Dwfl_Error error = __libdwfl_module_getebl (mod);
> > + if (error != DWFL_E_NOERROR)
> > {
> > - Dwfl_Error error = __libdwfl_module_getebl (mod);
> > - if (error != DWFL_E_NOERROR)
> > - {
> > - __libdwfl_seterrno (error);
> > - return -1;
> > - }
> > + __libdwfl_seterrno (error);
> > + return -1;
> > }
> >
> > int nops = ebl_return_value_location (mod->ebl, functypedie, locops);
>
> OK, likewise.
>
> Looks ok, except I might be missing something about not using
> __libdwfl_module_getebl in __libdwfl_getsym.
>
> Cheers,
>
> Mark
>
