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? > 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
