Hi Aaron,
On Mon, May 19, 2025 at 03:10:30PM -0400, Aaron Merey wrote:
> eu_tfind is used to facilitate lazy loading throughout libdw.
> If a result is not found via eu_tfind, work is done to load
> the result and cache it in an eu_search_tree.
>
> Some calls to eu_tfind allow for TOCTOU bugs. Multiple threads
> might race to call eu_tfind on some result that hasn't yet been
> cached. Multiple threads may then attempt to load the result
> which may cause an unnecessary amount of memory may be allocated.
^ ^
One of these mays seems redundant.
> Additionally this memory may not get released when the associated
> libdw data structure is freed.
>
> Fix this by adding additional locking to ensure that only one
> thread performs lazy loading.
>
> One approach used in this patch is to preserve calls to eu_tfind
> without additional locking, but when the result isn't found then
> a lock is then used to synchronize access to the lazy loading code.
> An extra eu_tfind call has been added at the start of these critical
> section to synchronize verification that lazy loading should proceed.
>
> Another approach used is to simply synchronize entire calls to
> functions where lazy loading via eu_tfind might occur (__libdw_find_fde
> and __libdw_intern_expression). In this case, new _nolock variants of
> the eu_t* functions are used to avoid unnecessary double locking.
Nice.
> lib/
> * eu-search.c: Add eu_tsearch_nolock, eu_tfind_nolock and
> eu_tdelete_nolock functions.
> * eu-search.h: Ditto.
>
> libdw/
> * cfi.h (struct Dwarf_CFI_s): Declare new mutex.
> * dwarf_begin_elf.c (valid_p): Initialize all locks for fake CUs.
> * dwarf_cfi_addrframe.c (dwarf_cfi_addrframe): Place lock around
> __libdw_find_fde.
> * dwarf_end.c (cu_free): Deallocate all locks unconditionally,
> whether or not the CU is fake.
> * dwarf_frame_cfa.c (dwarf_frame_cfa): Place lock around
> __libdw_intern_expression.
> * dwarf_frame_register.c (dwarf_frame_register): Ditto.
> * dwarf_getcfi.c (dwarf_getcfi): Initialize cfi lock.
> * dwarf_getlocation.c (is_constant_offset): Synchronize access
> to lazy loading section.
> (getlocation): Place lock around __libdw_intern_expression.
> * dwarf_getmacros.c (cache_op_table): Synchronize access to lazy
> loading section.
> * frame-cache.c (__libdw_destroy_frame_cache): Free Dwarf_CFI
> mutex.
> * libdwP.h (struct Dwarf): Update macro_lock comment.
> (struct Dwarf_CU): Declare new mutex.
> libdw_findcu.c (__libdw_intern_next_unit): Initialize
> intern_lock.
> (__libdw_findcu): Adjust locking so that the first eu_tfind
> can be done without extra lock overhead.
>
> Signed-off-by: Aaron Merey <[email protected]>
>
> ---
> v2:
>
> _nolock variants of eu_t* functions have been added to avoid double
> locking. _nolock functions are now used where appropriate.
>
> Clarified that __libdw_intern_expression should be called with a lock
> held by the owner of the search_tree passed to this function (typically
> a Dwarf_CFI_s or Dwarf_CU).
>
> lib/eu-search.c | 18 ++++++++++
> lib/eu-search.h | 12 +++++++
> libdw/cfi.h | 4 +++
> libdw/dwarf_begin_elf.c | 15 ++++++++
> libdw/dwarf_cfi_addrframe.c | 3 ++
> libdw/dwarf_end.c | 10 +++---
> libdw/dwarf_frame_cfa.c | 2 ++
> libdw/dwarf_frame_register.c | 16 +++++----
> libdw/dwarf_getcfi.c | 1 +
> libdw/dwarf_getlocation.c | 69 ++++++++++++++++++++++--------------
> libdw/dwarf_getmacros.c | 20 +++++++++--
> libdw/fde.c | 6 ++--
> libdw/frame-cache.c | 1 +
> libdw/libdwP.h | 11 ++++--
> libdw/libdw_findcu.c | 9 +++--
> 15 files changed, 150 insertions(+), 47 deletions(-)
>
> diff --git a/lib/eu-search.c b/lib/eu-search.c
> index fc31fe87..a49d8dd3 100644
> --- a/lib/eu-search.c
> +++ b/lib/eu-search.c
> @@ -62,6 +62,24 @@ void *eu_tdelete (const void *key, search_tree *tree,
> return ret;
> }
>
> +void *eu_tsearch_nolock (const void *key, search_tree *tree,
> + int (*compare)(const void *, const void *))
> +{
> + return tsearch (key, &tree->root, compare);
> +}
> +
> +void *eu_tfind_nolock (const void *key, search_tree *tree,
> + int (*compare)(const void *, const void *))
> +{
> + return tfind (key, &tree->root, compare);
> +}
> +
> +void *eu_tdelete_nolock (const void *key, search_tree *tree,
> + int (*compare)(const void *, const void *))
> +{
> + return tdelete (key, &tree->root, compare);
> +}
> +
> void eu_tdestroy (search_tree *tree, void (*free_node)(void *))
> {
> rwlock_wrlock (tree->lock);
>
> diff --git a/lib/eu-search.h b/lib/eu-search.h
> index 67b54c18..841a7f64 100644
> --- a/lib/eu-search.h
> +++ b/lib/eu-search.h
> @@ -52,6 +52,18 @@ extern void *eu_tfind (const void *key, search_tree *tree,
> extern void *eu_tdelete (const void *key, search_tree *tree,
> int (*compare)(const void *, const void *));
>
> +/* Search TREE for KEY and add KEY if not found. No locking is performed.
> */
> +extern void *eu_tsearch_nolock (const void *key, search_tree *tree,
> + int (*compare)(const void *, const void *));
> +
> +/* Search TREE for KEY. No locking is performed. */
> +extern void *eu_tfind_nolock (const void *key, search_tree *tree,
> + int (*compare)(const void *, const void *));
> +
> +/* Delete key from TREE. No locking is performed. */
> +extern void *eu_tdelete_nolock (const void *key, search_tree *tree,
> + int (*compare)(const void *, const void *));
> +
> /* Free all nodes from TREE. */
> void eu_tdestroy (search_tree *tree, void (*free_node)(void *));
Nitpick and probably premature optimization. But in theory you can
define such simple wrapper functions as static inline just in the
header file.
> diff --git a/libdw/cfi.h b/libdw/cfi.h
> index d0134243..f0296de7 100644
> --- a/libdw/cfi.h
> +++ b/libdw/cfi.h
> @@ -98,6 +98,10 @@ struct Dwarf_CFI_s
> /* Search tree for parsed DWARF expressions, indexed by raw pointer. */
> search_tree expr_tree;
>
> + /* Should be held when calling __libdw_find_fde, __libdw_fde_by_offset and
> + when __libdw_intern_expression is called with Dwarf_CFI members. */
> + mutex_define(, lock);
> +
> /* Backend hook. */
> struct ebl *ebl;
Nicely documented. But see my comment below about __libdw_fde_by_offset.
> diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
> index 58a309e9..63e2805d 100644
> --- a/libdw/dwarf_begin_elf.c
> +++ b/libdw/dwarf_begin_elf.c
> @@ -359,6 +359,11 @@ valid_p (Dwarf *result)
> result->fake_loc_cu->version = 4;
> result->fake_loc_cu->split = NULL;
> eu_search_tree_init (&result->fake_loc_cu->locs_tree);
> + rwlock_init (result->fake_loc_cu->abbrev_lock);
> + rwlock_init (result->fake_loc_cu->split_lock);
> + mutex_init (result->fake_loc_cu->src_lock);
> + mutex_init (result->fake_loc_cu->str_off_base_lock);
> + mutex_init (result->fake_loc_cu->intern_lock);
> }
> }
>
> @@ -387,6 +392,11 @@ valid_p (Dwarf *result)
> result->fake_loclists_cu->version = 5;
> result->fake_loclists_cu->split = NULL;
> eu_search_tree_init (&result->fake_loclists_cu->locs_tree);
> + rwlock_init (result->fake_loclists_cu->abbrev_lock);
> + rwlock_init (result->fake_loclists_cu->split_lock);
> + mutex_init (result->fake_loclists_cu->src_lock);
> + mutex_init (result->fake_loclists_cu->str_off_base_lock);
> + mutex_init (result->fake_loclists_cu->intern_lock);
> }
> }
>
> @@ -420,6 +430,11 @@ valid_p (Dwarf *result)
> result->fake_addr_cu->version = 5;
> result->fake_addr_cu->split = NULL;
> eu_search_tree_init (&result->fake_addr_cu->locs_tree);
> + rwlock_init (result->fake_addr_cu->abbrev_lock);
> + rwlock_init (result->fake_addr_cu->split_lock);
> + mutex_init (result->fake_addr_cu->src_lock);
> + mutex_init (result->fake_addr_cu->str_off_base_lock);
> + mutex_init (result->fake_addr_cu->intern_lock);
> }
> }
Ack. Also init locks for the fake CUs.
> diff --git a/libdw/dwarf_cfi_addrframe.c b/libdw/dwarf_cfi_addrframe.c
> index 44240279..f391f9f9 100644
> --- a/libdw/dwarf_cfi_addrframe.c
> +++ b/libdw/dwarf_cfi_addrframe.c
> @@ -39,7 +39,10 @@ dwarf_cfi_addrframe (Dwarf_CFI *cache, Dwarf_Addr address,
> Dwarf_Frame **frame)
> if (cache == NULL)
> return -1;
>
> + mutex_lock (cache->lock);
> struct dwarf_fde *fde = __libdw_find_fde (cache, address);
> + mutex_unlock (cache->lock);
> +
> if (fde == NULL)
> return -1;
OK, lock around __libdw_find_fde.
> diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
> index 1628e448..979b1168 100644
> --- a/libdw/dwarf_end.c
> +++ b/libdw/dwarf_end.c
> @@ -61,17 +61,19 @@ static void
> cu_free (void *arg)
> {
> struct Dwarf_CU *p = (struct Dwarf_CU *) arg;
> +
> eu_search_tree_fini (&p->locs_tree, noop_free);
> + rwlock_fini (p->abbrev_lock);
> + rwlock_fini (p->split_lock);
> + mutex_fini (p->src_lock);
> + mutex_fini (p->str_off_base_lock);
> + mutex_fini (p->intern_lock);
>
> /* Only free the CU internals if its not a fake CU. */
> if (p != p->dbg->fake_loc_cu && p != p->dbg->fake_loclists_cu
> && p != p->dbg->fake_addr_cu)
> {
> Dwarf_Abbrev_Hash_free (&p->abbrev_hash);
> - rwlock_fini (p->abbrev_lock);
> - rwlock_fini (p->split_lock);
> - mutex_fini (p->src_lock);
> - mutex_fini (p->str_off_base_lock);
>
> /* Free split dwarf one way (from skeleton to split). */
> if (p->unit_type == DW_UT_skeleton
Ack, fake cu locking fixed.
> diff --git a/libdw/dwarf_frame_cfa.c b/libdw/dwarf_frame_cfa.c
> index 07f998cd..c18ee499 100644
> --- a/libdw/dwarf_frame_cfa.c
> +++ b/libdw/dwarf_frame_cfa.c
> @@ -57,11 +57,13 @@ dwarf_frame_cfa (Dwarf_Frame *fs, Dwarf_Op **ops, size_t
> *nops)
>
> case cfa_expr:
> /* Parse the expression into internal form. */
> + mutex_lock (fs->cache->lock);
> result = __libdw_intern_expression
> (NULL, fs->cache->other_byte_order,
> fs->cache->e_ident[EI_CLASS] == ELFCLASS32 ? 4 : 8, 4,
> &fs->cache->expr_tree, &fs->cfa_data.expr, false, false,
> ops, nops, IDX_debug_frame);
> + mutex_unlock (fs->cache->lock);
> break;
>
> case cfa_invalid:
Ack, locking around __libdw_intern_expression.
> diff --git a/libdw/dwarf_frame_register.c b/libdw/dwarf_frame_register.c
> index a6b7c4c1..dbd7f1b2 100644
> --- a/libdw/dwarf_frame_register.c
> +++ b/libdw/dwarf_frame_register.c
> @@ -109,12 +109,16 @@ dwarf_frame_register (Dwarf_Frame *fs, int regno,
> Dwarf_Op ops_mem[3],
> block.data = (void *) p;
>
> /* Parse the expression into internal form. */
> - if (__libdw_intern_expression (NULL,
> - fs->cache->other_byte_order,
> - address_size, 4,
> - &fs->cache->expr_tree, &block,
> - true, reg->rule == reg_val_expression,
> - ops, nops, IDX_debug_frame) < 0)
> + mutex_lock (fs->cache->lock);
> + int res = __libdw_intern_expression (NULL,
> + fs->cache->other_byte_order,
> + address_size, 4,
> + &fs->cache->expr_tree, &block,
> + true, reg->rule ==
> reg_val_expression,
> + ops, nops, IDX_debug_frame);
> + mutex_unlock (fs->cache->lock);
> +
> + if (res < 0)
> return -1;
> break;
> }
Likewise.
> diff --git a/libdw/dwarf_getcfi.c b/libdw/dwarf_getcfi.c
> index a4497152..893e3c74 100644
> --- a/libdw/dwarf_getcfi.c
> +++ b/libdw/dwarf_getcfi.c
> @@ -70,6 +70,7 @@ dwarf_getcfi (Dwarf *dbg)
> eu_search_tree_init (&cfi->cie_tree);
> eu_search_tree_init (&cfi->fde_tree);
> eu_search_tree_init (&cfi->expr_tree);
> + mutex_init (cfi->lock);
>
> cfi->ebl = NULL;
Ack. Init lock.
> diff --git a/libdw/dwarf_getlocation.c b/libdw/dwarf_getlocation.c
> index ad1d46ca..2a2fd665 100644
> --- a/libdw/dwarf_getlocation.c
> +++ b/libdw/dwarf_getlocation.c
> @@ -154,7 +154,7 @@ store_implicit_value (Dwarf *dbg, search_tree *cache,
> Dwarf_Op *op)
> block->addr = op;
> block->data = (unsigned char *) data;
> block->length = op->number;
> - if (unlikely (eu_tsearch (block, cache, loc_compare) == NULL))
> + if (unlikely (eu_tsearch_nolock (block, cache, loc_compare) == NULL))
> return 1;
> return 0;
> }
store_implicit_value called from __libdw_intern_expression which
should only be called locked. So nolock usage is ok here.
> @@ -216,27 +216,38 @@ is_constant_offset (Dwarf_Attribute *attr,
>
> if (found == NULL)
> {
This found is the result of an eu_tfind. Why doesn't that one need to
be under the lock? We do another lock and eu_find_nolock below, so I
think it is correct. But wouldn't it be more efficient to have just
one big lock and one tfind?
> - Dwarf_Word offset;
> - if (INTUSE(dwarf_formudata) (attr, &offset) != 0)
> - return -1;
> + mutex_lock (attr->cu->intern_lock);
> +
> + found = eu_tfind_nolock (&fake, &attr->cu->locs_tree, loc_compare);
> + if (found == NULL)
> + {
> + Dwarf_Word offset;
> + if (INTUSE(dwarf_formudata) (attr, &offset) != 0)
> + {
> + mutex_unlock (attr->cu->intern_lock);
> + return -1;
> + }
>
> - Dwarf_Op *result = libdw_alloc (attr->cu->dbg,
> - Dwarf_Op, sizeof (Dwarf_Op), 1);
> + Dwarf_Op *result = libdw_alloc (attr->cu->dbg,
> + Dwarf_Op, sizeof (Dwarf_Op), 1);
>
> - result->atom = DW_OP_plus_uconst;
> - result->number = offset;
> - result->number2 = 0;
> - result->offset = 0;
> + result->atom = DW_OP_plus_uconst;
> + result->number = offset;
> + result->number2 = 0;
> + result->offset = 0;
>
> - /* Insert a record in the search tree so we can find it again later.
> */
> - struct loc_s *newp = libdw_alloc (attr->cu->dbg,
> - struct loc_s, sizeof (struct loc_s),
> - 1);
> - newp->addr = attr->valp;
> - newp->loc = result;
> - newp->nloc = 1;
> + /* Insert a record in the search tree so we can find it again later.
> */
> + struct loc_s *newp = libdw_alloc (attr->cu->dbg,
> + struct loc_s, sizeof (struct loc_s),
> + 1);
> + newp->addr = attr->valp;
> + newp->loc = result;
> + newp->nloc = 1;
> +
> + found = eu_tsearch_nolock (newp, &attr->cu->locs_tree, loc_compare);
> + }
>
> - found = eu_tsearch (newp, &attr->cu->locs_tree, loc_compare);
> + mutex_unlock (attr->cu->intern_lock);
> }
>
> assert ((*found)->nloc == 1);
And unlock, so all calls to nolock are ok.
> @@ -267,7 +278,7 @@ __libdw_intern_expression (Dwarf *dbg, bool
> other_byte_order,
>
> /* Check whether we already looked at this list. */
> struct loc_s fake = { .addr = block->data };
> - struct loc_s **found = eu_tfind (&fake, cache, loc_compare);
> + struct loc_s **found = eu_tfind_nolock (&fake, cache, loc_compare);
> if (found != NULL)
> {
> /* We already saw it. */
> @@ -656,7 +667,7 @@ __libdw_intern_expression (Dwarf *dbg, bool
> other_byte_order,
> newp->addr = block->data;
> newp->loc = result;
> newp->nloc = *listlen;
> - eu_tsearch (newp, cache, loc_compare);
> + eu_tsearch_nolock (newp, cache, loc_compare);
>
> /* We did it. */
> return 0;
OK, all calls to __libdw_intern_expression hold locks associated with
the cache, so we can use the nolock variants here.
> @@ -674,13 +685,17 @@ getlocation (struct Dwarf_CU *cu, const Dwarf_Block
> *block,
> return 0;
> }
>
> - return __libdw_intern_expression (cu->dbg, cu->dbg->other_byte_order,
> - cu->address_size, (cu->version == 2
> - ? cu->address_size
> - : cu->offset_size),
> - &cu->locs_tree, block,
> - false, false,
> - llbuf, listlen, sec_index);
> + mutex_lock (cu->intern_lock);
> + int res = __libdw_intern_expression (cu->dbg, cu->dbg->other_byte_order,
> + cu->address_size, (cu->version == 2
> + ? cu->address_size
> + : cu->offset_size),
> + &cu->locs_tree, block,
> + false, false,
> + llbuf, listlen, sec_index);
> + mutex_unlock (cu->intern_lock);
> +
> + return res;
> }
OK cu intern_lock for __libdw_intern_expression as a whole.
> int
> diff --git a/libdw/dwarf_getmacros.c b/libdw/dwarf_getmacros.c
> index f1c831fa..d7ed7b58 100644
> --- a/libdw/dwarf_getmacros.c
> +++ b/libdw/dwarf_getmacros.c
> @@ -322,15 +322,29 @@ cache_op_table (Dwarf *dbg, int sec_index, Dwarf_Off
> macoff,
> if (found != NULL)
> return *found;
>
> + mutex_lock (dbg->macro_lock);
> +
> + found = eu_tfind_nolock (&fake, &dbg->macro_ops_tree, macro_op_compare);
> + if (found != NULL)
> + {
> + mutex_unlock (dbg->macro_lock);
> + return *found;
> + }
> +
Ack, lock and nolock.
> Dwarf_Macro_Op_Table *table = sec_index == IDX_debug_macro
> ? get_table_for_offset (dbg, macoff, startp, endp, cudie)
> : get_macinfo_table (dbg, macoff, cudie);
>
> if (table == NULL)
> - return NULL;
> + {
> + mutex_unlock (dbg->macro_lock);
> + return NULL;
> + }
> +
> + Dwarf_Macro_Op_Table **ret = eu_tsearch_nolock (table,
> &dbg->macro_ops_tree,
> + macro_op_compare);
> + mutex_unlock (dbg->macro_lock);
>
> - Dwarf_Macro_Op_Table **ret = eu_tsearch (table, &dbg->macro_ops_tree,
> - macro_op_compare);
> if (unlikely (ret == NULL))
> {
> __libdw_seterrno (DWARF_E_NOMEM);
Ack. Likewise.
> diff --git a/libdw/fde.c b/libdw/fde.c
> index a5167805..7f36c4a9 100644
> --- a/libdw/fde.c
> +++ b/libdw/fde.c
> @@ -122,7 +122,8 @@ intern_fde (Dwarf_CFI *cache, const Dwarf_FDE *entry)
> fde->instructions += cie->fde_augmentation_data_size;
>
> /* Add the new entry to the search tree. */
> - struct dwarf_fde **tres = eu_tsearch (fde, &cache->fde_tree, &compare_fde);
> + struct dwarf_fde **tres = eu_tsearch_nolock (fde, &cache->fde_tree,
> + &compare_fde);
> if (tres == NULL)
> {
> free (fde);
intern_fde is called from __libdw_fde_by_offset and __libdw_find_fde.
__libdw_fde_by_offset is only called from __libdw_find_fde. And
__libdw_find_fde should always be locked. So using eu_tsearch_nolock
here is fine.
As a followup patch we could make __libdw_fde_by_offset static in
libdw/fde.c and remove the extern definition in libdw/cfi.h.
> @@ -252,7 +253,8 @@ __libdw_find_fde (Dwarf_CFI *cache, Dwarf_Addr address)
> /* Look for a cached FDE covering this address. */
>
> const struct dwarf_fde fde_key = { .start = address, .end = 0 };
> - struct dwarf_fde **found = eu_tfind (&fde_key, &cache->fde_tree,
> &compare_fde);
> + struct dwarf_fde **found = eu_tfind_nolock (&fde_key, &cache->fde_tree,
> + &compare_fde);
> if (found != NULL)
> return *found;
>
Right. __libdw_find_fde should only be called locked. So the nolock
call is correct here.
> diff --git a/libdw/frame-cache.c b/libdw/frame-cache.c
> index 6c89858a..6f6f777e 100644
> --- a/libdw/frame-cache.c
> +++ b/libdw/frame-cache.c
> @@ -64,6 +64,7 @@ __libdw_destroy_frame_cache (Dwarf_CFI *cache)
> eu_search_tree_fini (&cache->fde_tree, free_fde);
> eu_search_tree_fini (&cache->cie_tree, free_cie);
> eu_search_tree_fini (&cache->expr_tree, free_expr);
> + mutex_fini (cache->lock);
>
> if (cache->ebl != NULL && cache->ebl != (void *) -1l)
> ebl_closebackend (cache->ebl);
Ack.
> diff --git a/libdw/libdwP.h b/libdw/libdwP.h
> index de80cd4e..25d53d31 100644
> --- a/libdw/libdwP.h
> +++ b/libdw/libdwP.h
> @@ -269,7 +269,7 @@ struct Dwarf
> __libdw_intern_next_unit. */
> mutex_define(, dwarf_lock);
>
> - /* Synchronize access to dwarf_macro_getsrcfiles. */
> + /* Synchronize access to dwarf_macro_getsrcfiles and cache_op_table. */
> mutex_define(, macro_lock);
>
> /* Internal memory handling. This is basically a simplified thread-local
> @@ -471,6 +471,10 @@ struct Dwarf_CU
> Covers __libdw_str_offsets_base_off. */
> mutex_define(, str_off_base_lock);
>
> + /* Synchronize access to is_constant_offset. Should also be held
> + when calling __libdw_intern_expression with Dwarf_CU members. */
> + mutex_define(, intern_lock);
> +
> /* Memory boundaries of this CU. */
> void *startp;
> void *endp;
Ack.
> @@ -949,8 +953,9 @@ extern int __libdw_visit_scopes (unsigned int depth,
> void *arg)
> __nonnull_attribute__ (2, 4) internal_function;
>
> -/* Parse a DWARF Dwarf_Block into an array of Dwarf_Op's,
> - and cache the result (via tsearch). */
> +/* Parse a DWARF Dwarf_Block into an array of Dwarf_Op's, and cache the
> + result (via tsearch). The owner of CACHE (typically a Dwarf_CU or
> + Dwarf_CFI_s) must hold a lock when calling this function. */
> extern int __libdw_intern_expression (Dwarf *dbg,
> bool other_byte_order,
> unsigned int address_size,
Nice docks.
> diff --git a/libdw/libdw_findcu.c b/libdw/libdw_findcu.c
> index 0e4dcc37..21c8ed0e 100644
> --- a/libdw/libdw_findcu.c
> +++ b/libdw/libdw_findcu.c
> @@ -181,6 +181,7 @@ __libdw_intern_next_unit (Dwarf *dbg, bool debug_types)
> rwlock_init (newp->split_lock);
> mutex_init (newp->src_lock);
> mutex_init (newp->str_off_base_lock);
> + mutex_init (newp->intern_lock);
>
> /* v4 debug type units have version == 4 and unit_type == DW_UT_type. */
> if (debug_types)
Ack.
> @@ -240,8 +241,6 @@ struct Dwarf_CU *
> internal_function
> __libdw_findcu (Dwarf *dbg, Dwarf_Off start, bool v4_debug_types)
> {
> - mutex_lock (dbg->dwarf_lock);
> -
> search_tree *tree = v4_debug_types ? &dbg->tu_tree : &dbg->cu_tree;
> Dwarf_Off *next_offset
> = v4_debug_types ? &dbg->next_tu_offset : &dbg->next_cu_offset;
> @@ -250,6 +249,12 @@ __libdw_findcu (Dwarf *dbg, Dwarf_Off start, bool
> v4_debug_types)
> struct Dwarf_CU fake = { .start = start, .end = 0 };
> struct Dwarf_CU **found = eu_tfind (&fake, tree, findcu_cb);
> struct Dwarf_CU *result = NULL;
> + if (found != NULL)
> + return *found;
> +
> + mutex_lock (dbg->dwarf_lock);
> +
> + found = eu_tfind_nolock (&fake, tree, findcu_cb);
> if (found != NULL)
> {
> mutex_unlock (dbg->dwarf_lock);
OK, so the dwarf_lock has to be held to call __libdw_intern_next_unit
below. But why is it correct to call eu_tfind_nolock here without
holding the tree lock?
Thanks,
Mark
