Bug 26930 - tsearch/tfind tree caches need locking
Summary: tsearch/tfind tree caches need locking
Status: RESOLVED FIXED
Alias: None
Product: elfutils
Classification: Unclassified
Component: libdw (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-21 21:58 UTC by Mark Wielaard
Modified: 2025-06-06 16:28 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
Project(s) to access:
ssh public key:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Wielaard 2020-11-21 21:58:19 UTC
libdw uses various search trees as (lazy) caches.

Specifically struct Dwarf has:

  /* Search tree for the CUs.  */
  void *cu_tree;
  Dwarf_Off next_cu_offset;

  /* Search tree and sig8 hash table for .debug_types type units.  */
  void *tu_tree;
  Dwarf_Off next_tu_offset;
  Dwarf_Sig8_Hash sig8_hash;

  /* Search tree for split Dwarf associated with CUs in this debug.  */
  void *split_tree;

  /* Search tree for .debug_macro operator tables.  */
  void *macro_ops;

  /* Search tree for decoded .debug_line units.  */
  void *files_lines;

struct Dwarf_CU has:

  /* Known location lists.  */
  void *locs;

struct Dwarf_CFI_s has:

  /* Search tree for the CIEs, indexed by CIE_pointer (section offset).  */
  void *cie_tree;

  /* Search tree for the FDEs, indexed by PC address.  */
  void *fde_tree;

  /* Search tree for parsed DWARF expressions, indexed by raw pointer.  */
  void *expr_tree;

struct Dwfl_Module has:

  void *lazy_cu_root;           /* Table indexed by Dwarf_Off of CU.  */

When used in a concurrent program they need to be read locked when searched (with tfind) and write locked when updating (with tsearch).

See several backtraces in bug #26921 which describes a different concurrent unsafe update mechanism.
Comment 1 Milian Wolff 2020-11-24 12:17:21 UTC
as a suggestion to catch and fix these issues: if there'd be a unit test that runs through the "supposedly thread safe" API, one could compile it with tsan and verify that the code is actually thread safe. with elfutils using plain c, tsan should work pretty well from what I know.
Comment 3 Aaron Merey 2025-06-06 16:28:40 UTC
Addressed in the following commit:

commit d6443d1a4df6057f9012d105037f52daaca911f1
Author: Heather McIntyre <hsm2@rice.edu>
Date:   Fri Jul 12 18:23:56 2024 -0400

    lib: Add eu_tsearch, eu_tfind, eu_tdelete and eu_tdestroy
    
    Add struct search_tree to hold tree root and lock.  Add new eu_t*
    functions for ensuring synchronized tree access.
    
    Replace tsearch, tfind, etc with eu_t* equivalents.
    
    lib:
            * Makefile.am (libeu_a_SOURCES): Add eu-search.c.
            (noinst_HEADERS): Add eu-search.h and locks.h.
            * eu-config.h: Move rwlock macros to locks.h.
            * eu-search.c: New file containing tree search functions with
              locking.
            * eu-search.h: New file.
            * locks.h: New file containing rwlock macros previously in
              eu-config.h.
    libdw:
            * cfi.h (struct Dwarf_CFI_s): Change type of search tree members
              from void * to search_tree.
            * cie.c: Replace tree search functions with eu-search equivalents.
            * dwarf_begin_elf.c (valid_p): Initialize search trees.
            * dwarf_end.c (cu_free): Replace tree search functions
              with eu-search equivalents.
            * dwarf_getcfi.c (dwarf_getcfi): Initialize search trees.
            * dwarf_getlocations.c: Replace search tree functions with
              eu-search equivalents.
              (__libdw_intern_expression): Change type of cache parameter to
              search_tree *.
            * dwarf_getmacros.c: Replace tree search functions with
              eu-search equivalents.
            * dwarf_getsrclines.c: Ditto.
            * fde.c: Ditto.
            * frame-cache.c (__libdw_destroy_frame_cache): Initialize search
              trees.
            * libdwP.h (struct Dwarf): Change type of search tree members
              from void * to search_tree.
              (struct Dwarf_CU): Ditto.
              (__libdw_intern_expression): Change type of cache parameter to
              search_tree *.
            * libdw_find_split_unit.c: Replace tree search functions with
              eu-search equivalents.
            * libdw_findcu.c: Ditto.
    libdwfl:
            * cu.c: Ditto.
            * libdwflP.h (struct Dwfl_Module): Replace void *lazy_cu_root
              with search_tree lazy_cu_tree.
    libelf:
            * elf_begin.c (file_read_elf): Initialize rawchunck_tree.
            * elf_end.c (elf_end): Replace tree search function with
              eu-search equivalent.
            * elf_getdata_rawchunck.c: Ditto.
            * libelfP.h (struct Elf): Replace void * rawchuncks member with
            search_tree rawchunk_tree.