Re: [PATCH] backends: allocate enough stace for null terminator
Hi Sergei,
On Mon, 2024-07-15 at 22:23 +0100, Sergei Trofimovich wrote:
> `gcc-15` added a new warning in https://gcc.gnu.org/PR115185:
>
> i386_regs.c:88:11: error: initializer-string for array of 'char' is too
> long [-Werror=unterminated-string-initialization]
>88 | "ax", "cx", "dx", "bx", "sp", "bp", "si", "di", "ip"
> | ^~~~
OK, I see why you want to warn for this. There is a real chance you use
those char array's with some str functions and then there is trouble.
> `elfutils` does not need to store '\0'. We could either initialize the
> arrays with individual bytes or allocate extra byte for null.
With initialize with individual bytes you mean:
diff --git a/backends/i386_regs.c b/backends/i386_regs.c
index 7ec93bb9fc13..ead55ef7f931 100644
--- a/backends/i386_regs.c
+++ b/backends/i386_regs.c
@@ -85,7 +85,15 @@ i386_register_info (Ebl *ebl __attribute__
((unused)),
{
static const char baseregs[][2] =
{
- "ax", "cx", "dx", "bx", "sp", "bp", "si", "di", "ip"
+ {'a', 'x'},
+ {'c', 'x'},
+ {'d', 'x'},
+ {'b', 'x'},
+ {'s', 'p'},
+ {'b', 'p'},
+ {'s', 'i'},
+ {'d', 'i'},
+ {'i', 'p'},
};
case 4:
?
Since the use of this array is basically just:
name[0] = 'r';
name[1] = baseregs[regno][0];
name[2] = baseregs[regno][1];
I think I prefer the individual bytes init way. It makes more clear
what we really use these arrays for. imho.
Thanks,
Mark
[PATCH v2] backends: allocate enough stace for null terminator
`gcc-15` added a new warning in https://gcc.gnu.org/PR115185:
i386_regs.c:88:11: error: initializer-string for array of 'char' is too
long [-Werror=unterminated-string-initialization]
88 | "ax", "cx", "dx", "bx", "sp", "bp", "si", "di", "ip"
| ^~~~
`elfutils` does not need to store '\0'. We could either initialize the
arrays with individual bytes or allocate extra byte for null.
This change initializes the array bytewise.
* backends/i386_regs.c (i386_register_info): Initialize the
array bytewise to fix gcc-15 warning.
* backends/x86_64_regs.c (x86_64_register_info): Ditto.
Signed-off-by: Sergei Trofimovich
---
backends/i386_regs.c | 10 +-
backends/x86_64_regs.c | 9 -
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/backends/i386_regs.c b/backends/i386_regs.c
index 7ec93bb9..ead55ef7 100644
--- a/backends/i386_regs.c
+++ b/backends/i386_regs.c
@@ -85,7 +85,15 @@ i386_register_info (Ebl *ebl __attribute__ ((unused)),
{
static const char baseregs[][2] =
{
- "ax", "cx", "dx", "bx", "sp", "bp", "si", "di", "ip"
+ {'a', 'x'},
+ {'c', 'x'},
+ {'d', 'x'},
+ {'b', 'x'},
+ {'s', 'p'},
+ {'b', 'p'},
+ {'s', 'i'},
+ {'d', 'i'},
+ {'i', 'p'},
};
case 4:
diff --git a/backends/x86_64_regs.c b/backends/x86_64_regs.c
index ef987daf..dab8f27f 100644
--- a/backends/x86_64_regs.c
+++ b/backends/x86_64_regs.c
@@ -82,7 +82,14 @@ x86_64_register_info (Ebl *ebl __attribute__ ((unused)),
{
static const char baseregs[][2] =
{
- "ax", "dx", "cx", "bx", "si", "di", "bp", "sp"
+ {'a', 'x'},
+ {'d', 'x'},
+ {'c', 'x'},
+ {'b', 'x'},
+ {'s', 'i'},
+ {'d', 'i'},
+ {'b', 'p'},
+ {'s', 'p'},
};
case 6 ... 7:
--
2.45.2
[PATCH 0/9 v2] Fix thread-safety for elfutils
v1 can be found at https://sourceware.org/pipermail/elfutils-devel/2023q3/006329.html Heather McIntyre is the original author of v1 of these patches. Heather and myself co-wrote v2. Aaron Merey (9): libelf: Fix deadlock in __libelf_readall libelf: Fix deadlock in elf_cntl lib: Add eu_tsearch, eu_tfind, eu_tdelete and eu_tdestroy libdw: make dwarf_getalt thread-safe libdwP.h: Add locking to __libdw_dieabbrev libdw: Make libdw_find_split_unit thread-safe libdw: Make libdw_findcu thread-safe tests: Add eu-search tests configure: No longer mark --enable-thread-safety as EXPERIMENTAL configure.ac | 6 +- lib/Makefile.am | 5 +- lib/eu-config.h | 30 +--- lib/eu-search.c | 85 +++ lib/eu-search.h | 64 + lib/locks.h | 62 libdw/cfi.h | 6 +- libdw/cie.c | 10 +- libdw/dwarf_begin_elf.c | 7 +- libdw/dwarf_end.c | 17 +-- libdw/dwarf_formref_die.c | 2 + libdw/dwarf_getalt.c | 18 ++- libdw/dwarf_getcfi.c | 5 +- libdw/dwarf_getlocation.c | 24 ++-- libdw/dwarf_getmacros.c | 6 +- libdw/dwarf_getsrclines.c | 8 +- libdw/dwarf_setalt.c | 4 + libdw/fde.c | 6 +- libdw/frame-cache.c | 8 +- libdw/libdwP.h| 49 +-- libdw/libdw_find_split_unit.c | 18 ++- libdw/libdw_findcu.c | 65 + libdwfl/cu.c | 8 +- libdwfl/dwfl_module.c | 4 +- libdwfl/libdwflP.h| 3 +- libelf/common.h | 16 +-- libelf/elf_begin.c| 2 + libelf/elf_cntl.c | 7 +- libelf/elf_end.c | 13 +- libelf/elf_getdata_rawchunk.c | 12 +- libelf/elf_readall.c | 4 +- libelf/libelfP.h | 10 +- tests/.gitignore | 4 + tests/Makefile.am | 19 ++- tests/eu_search_cfi.c | 234 ++ tests/eu_search_die.c | 262 ++ tests/eu_search_lines.c | 228 + tests/eu_search_macros.c | 216 tests/run-eu-search-tests.sh | 168 ++ 39 files changed, 1553 insertions(+), 162 deletions(-) create mode 100644 lib/eu-search.c create mode 100644 lib/eu-search.h create mode 100644 lib/locks.h create mode 100644 tests/eu_search_cfi.c create mode 100644 tests/eu_search_die.c create mode 100644 tests/eu_search_lines.c create mode 100644 tests/eu_search_macros.c create mode 100755 tests/run-eu-search-tests.sh -- 2.45.2
[PATCH 2/9 v2] libelf: Fix deadlock in elf_cntl
From: Heather McIntyre
* libelf/elf_cntl.c (elf_cntl): Move rwlock_wrlock, rwlock_unlock,
inside case switch statements.
Signed-off-by: Heather S. McIntyre
Signed-off-by: Aaron Merey
Signed-off-by: Mark Wielaard
---
v2 changes:
Remove unnecessary locking and checking of elf->map_address
libelf/elf_cntl.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/libelf/elf_cntl.c b/libelf/elf_cntl.c
index 04aa9132..3fbc7d97 100644
--- a/libelf/elf_cntl.c
+++ b/libelf/elf_cntl.c
@@ -48,13 +48,12 @@ elf_cntl (Elf *elf, Elf_Cmd cmd)
return -1;
}
- rwlock_wrlock (elf->lock);
switch (cmd)
{
case ELF_C_FDREAD:
/* If not all of the file is in the memory read it now. */
- if (elf->map_address == NULL && __libelf_readall (elf) == NULL)
+ if (__libelf_readall (elf) == NULL)
{
/* We were not able to read everything. */
result = -1;
@@ -64,7 +63,9 @@ elf_cntl (Elf *elf, Elf_Cmd cmd)
case ELF_C_FDDONE:
/* Mark the file descriptor as not usable. */
+ rwlock_wrlock (elf->lock);
elf->fildes = -1;
+ rwlock_unlock (elf->lock);
break;
default:
@@ -73,7 +74,5 @@ elf_cntl (Elf *elf, Elf_Cmd cmd)
break;
}
- rwlock_unlock (elf->lock);
-
return result;
}
--
2.45.2
[PATCH 1/9 v2] libelf: Fix deadlock in __libelf_readall
From: Heather McIntyre
Apply locking during __libelf_readall.
Signed-off-by: Heather S. McIntyre
Signed-off-by: Aaron Merey
Signed-off-by: Mark Wielaard
---
v2 changes:
Add locking for all early returns in __libelf_readall.
libelf_{acquire,release}_all have been renamed to
libelf_{acquire,release}_all_children. These functions also no longer
acquire/release the parent's lock. This is done in order to simplify
lock management in __libelf_readall.
libelf/common.h | 16
libelf/elf_readall.c | 4 ++--
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/libelf/common.h b/libelf/common.h
index 9b2a856d..b7226ee8 100644
--- a/libelf/common.h
+++ b/libelf/common.h
@@ -92,10 +92,8 @@ allocate_elf (int fildes, void *map_address, int64_t offset,
size_t maxsize,
/* Acquire lock for the descriptor and all children. */
static void
__attribute__ ((unused))
-libelf_acquire_all (Elf *elf)
+libelf_acquire_all_children (Elf *elf)
{
- rwlock_wrlock (elf->lock);
-
if (elf->kind == ELF_K_AR)
{
Elf *child = elf->state.ar.children;
@@ -103,7 +101,9 @@ libelf_acquire_all (Elf *elf)
while (child != NULL)
{
if (child->ref_count != 0)
- libelf_acquire_all (child);
+ libelf_acquire_all_children (child);
+
+ rwlock_wrlock(child->lock);
child = child->next;
}
}
@@ -112,7 +112,7 @@ libelf_acquire_all (Elf *elf)
/* Release own lock and those of the children. */
static void
__attribute__ ((unused))
-libelf_release_all (Elf *elf)
+libelf_release_all_children (Elf *elf)
{
if (elf->kind == ELF_K_AR)
{
@@ -121,12 +121,12 @@ libelf_release_all (Elf *elf)
while (child != NULL)
{
if (child->ref_count != 0)
- libelf_release_all (child);
+ libelf_release_all_children (child);
+
+ rwlock_unlock (child->lock);
child = child->next;
}
}
-
- rwlock_unlock (elf->lock);
}
diff --git a/libelf/elf_readall.c b/libelf/elf_readall.c
index d0f9a28c..4ef8fe97 100644
--- a/libelf/elf_readall.c
+++ b/libelf/elf_readall.c
@@ -84,7 +84,7 @@ __libelf_readall (Elf *elf)
/* If this is an archive and we have derived descriptors get the
locks for all of them. */
- libelf_acquire_all (elf);
+ libelf_acquire_all_children (elf);
if (elf->maximum_size == ~((size_t) 0))
{
@@ -141,7 +141,7 @@ __libelf_readall (Elf *elf)
__libelf_seterrno (ELF_E_NOMEM);
/* Free the locks on the children. */
- libelf_release_all (elf);
+ libelf_release_all_children (elf);
}
rwlock_unlock (elf->lock);
--
2.45.2
[PATCH 4/9 v2] libdw: make dwarf_getalt thread-safe
From: Heather McIntyre
* libdw/dwarf_getalt.c (dwarf_getalt): Add locking.
Signed-off-by: Heather S. McIntyre
Signed-off-by: Aaron Merey
Signed-off-by: Mark Wielaard
v2 changes:
Write lock now applies to all of dwarf_getalt.
---
libdw/dwarf_getalt.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/libdw/dwarf_getalt.c b/libdw/dwarf_getalt.c
index 0a12dfae..3b827d0b 100644
--- a/libdw/dwarf_getalt.c
+++ b/libdw/dwarf_getalt.c
@@ -160,15 +160,27 @@ find_debug_altlink (Dwarf *dbg)
}
}
+/* find_debug_altlink() modifies "dbg->alt_dwarf".
+ dwarf_getalt() reads "main->alt_dwarf".
+ Mutual exclusion is enforced to prevent a race. */
+
Dwarf *
dwarf_getalt (Dwarf *main)
{
+ rwlock_wrlock(main->dwarf_lock);
+
/* Only try once. */
if (main == NULL || main->alt_dwarf == (void *) -1)
-return NULL;
+{
+ rwlock_unlock (main->dwarf_lock);
+ return NULL;
+}
if (main->alt_dwarf != NULL)
-return main->alt_dwarf;
+{
+ rwlock_unlock (main->dwarf_lock);
+ return main->alt_dwarf;
+}
find_debug_altlink (main);
@@ -176,9 +188,11 @@ dwarf_getalt (Dwarf *main)
if (main->alt_dwarf == NULL)
{
main->alt_dwarf = (void *) -1;
+ rwlock_unlock (main->dwarf_lock);
return NULL;
}
+ rwlock_unlock (main->dwarf_lock);
return main->alt_dwarf;
}
INTDEF (dwarf_getalt)
--
2.45.2
[PATCH 6/9 v2] libdw: Make libdw_find_split_unit thread-safe
From: Heather McIntyre
* (__libdw_find_split_unit): Add lock for cu->split.
Signed-off-by: Heather S. McIntyre
Signed-off-by: Aaron Merey
Signed-off-by: Mark Wielaard
---
v2 changes:
Locking applied to __libdw_find_split_unit instead of try_split_file.
libdw/libdw_find_split_unit.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/libdw/libdw_find_split_unit.c b/libdw/libdw_find_split_unit.c
index 67d31a9c..eb3d88d7 100644
--- a/libdw/libdw_find_split_unit.c
+++ b/libdw/libdw_find_split_unit.c
@@ -150,9 +150,14 @@ Dwarf_CU *
internal_function
__libdw_find_split_unit (Dwarf_CU *cu)
{
+ rwlock_wrlock(cu->split_lock);
+
/* Only try once. */
if (cu->split != (Dwarf_CU *) -1)
-return cu->split;
+{
+ rwlock_unlock(cu->split_lock);
+ return cu->split;
+}
/* We need a skeleton unit with a comp_dir and [GNU_]dwo_name attributes.
The split unit will be the first in the dwo file and should have the
@@ -207,5 +212,6 @@ __libdw_find_split_unit (Dwarf_CU *cu)
if (cu->split == (Dwarf_CU *) -1)
cu->split = NULL;
+ rwlock_unlock(cu->split_lock);
return cu->split;
}
--
2.45.2
[PATCH 5/9 v2] libdwP.h: Add locking to __libdw_dieabbrev,
From: Heather McIntyre
Signed-off-by: Heather S. McIntyre
Signed-off-by: Aaron Merey
Signed-off-by: Mark Wielaard
---
v2 changes:
This replaces patch "libdw: Add locking around __libdw_dieabbrev for
dwarf_hasattr". Mark suggested that we remove lazy abbrev reading
in order to simplify the locking of __libdw_dieabbrev. This is
a fair bit of work so for now lets just put a write lock around all
of __libdw_dieabbrev. The removal of lazy reading of abbrev will be
done in a future patch.
libdw/dwarf_formref_die.c | 2 ++
libdw/dwarf_setalt.c | 4
libdw/libdwP.h| 23 +++
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/libdw/dwarf_formref_die.c b/libdw/dwarf_formref_die.c
index 48ba8194..f8a51970 100644
--- a/libdw/dwarf_formref_die.c
+++ b/libdw/dwarf_formref_die.c
@@ -92,7 +92,9 @@ dwarf_formref_die (Dwarf_Attribute *attr, Dwarf_Die *result)
bool scan_debug_types = false;
do
{
+rwlock_wrlock(attr->cu->dbg->dwarf_lock);
cu = __libdw_intern_next_unit (attr->cu->dbg, scan_debug_types);
+rwlock_unlock(attr->cu->dbg->dwarf_lock);
if (cu == NULL)
{
if (scan_debug_types == false)
diff --git a/libdw/dwarf_setalt.c b/libdw/dwarf_setalt.c
index dc9b61cb..f7d70d9d 100644
--- a/libdw/dwarf_setalt.c
+++ b/libdw/dwarf_setalt.c
@@ -35,6 +35,8 @@
void
dwarf_setalt (Dwarf *main, Dwarf *alt)
{
+ rwlock_wrlock(main->dwarf_lock);
+
if (main->alt_fd != -1)
{
INTUSE(dwarf_end) (main->alt_dwarf);
@@ -43,5 +45,7 @@ dwarf_setalt (Dwarf *main, Dwarf *alt)
}
main->alt_dwarf = alt;
+
+ rwlock_unlock(main->dwarf_lock);
}
INTDEF (dwarf_setalt)
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index a4f26b82..9dce10e5 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -804,15 +804,28 @@ static inline Dwarf_Abbrev *
__nonnull_attribute__ (1)
__libdw_dieabbrev (Dwarf_Die *die, const unsigned char **readp)
{
+ if (unlikely (die->cu == NULL))
+{
+ die->abbrev = DWARF_END_ABBREV;
+ return DWARF_END_ABBREV;
+}
+
+ rwlock_wrlock (die->cu->abbrev_lock);
+
/* Do we need to get the abbreviation, or need to read after the code? */
if (die->abbrev == NULL || readp != NULL)
{
/* Get the abbreviation code. */
unsigned int code;
const unsigned char *addr = die->addr;
- if (unlikely (die->cu == NULL
- || addr >= (const unsigned char *) die->cu->endp))
- return die->abbrev = DWARF_END_ABBREV;
+
+ if (addr >= (const unsigned char *) die->cu->endp)
+ {
+ die->abbrev = DWARF_END_ABBREV;
+ rwlock_unlock (die->cu->abbrev_lock);
+ return DWARF_END_ABBREV;
+ }
+
get_uleb128 (code, addr, die->cu->endp);
if (readp != NULL)
*readp = addr;
@@ -821,7 +834,9 @@ __libdw_dieabbrev (Dwarf_Die *die, const unsigned char
**readp)
if (die->abbrev == NULL)
die->abbrev = __libdw_findabbrev (die->cu, code);
}
- return die->abbrev;
+
+ rwlock_unlock (die->cu->abbrev_lock);
+ return (Dwarf_Abbrev *) die->abbrev;
}
/* Helper functions for form handling. */
--
2.45.2
[PATCH 8/9 v2] tests: Add eu-search tests
From: Heather McIntyre * tests/eu_search_cfi.c: New file. * tests/eu_search_die.c: New file. * tests/eu_search_lines.c: New file. * tests/eu_search_macros.c: New file. * tests/run-eu-search-tests.sh: New test. * tests/Makefile.am: Add USE_LOCKS condition for -pthread. (check_PROGRAMS): Add eu_search_cfi, eu_search_die, eu_search_lines, and eu_search_macros. (TESTS, EXTRA_DIST): Add run-eu-search-tests.sh (eu_search_cfi_LDADD): New variable. (eu_search_die_LDADD): New variable. (eu_search_lines_LDADD): New variable. (eu_search_macros_LDADD): New variable. (eu_search_cfi_LDFLAGS): New variable. Add -pthread if USE_LOCKS is not defined. (eu_search_die_LDFLAGS): Likewise. (eu_search_lines_LDFLAGS): Likewise. (eu_search_macros_LDFLAGS): Likewise. Signed-off-by: Heather S. McIntyre Signed-off-by: Aaron Merey Signed-off-by: Mark Wielaard --- v2 changes: Rebase and add run-eu-search-tests.sh to EXTRA_DIST. The elfutils testsuite (including the tests added in this patch) passes for me on the sourceware buildbots. However the buildbots do not configure elfutils with --enable-thread-safety. I think we should include this configure option when the buildbots run elfutils tests. tests/.gitignore | 4 + tests/Makefile.am| 19 ++- tests/eu_search_cfi.c| 234 +++ tests/eu_search_die.c| 262 +++ tests/eu_search_lines.c | 228 ++ tests/eu_search_macros.c | 216 + tests/run-eu-search-tests.sh | 168 ++ 7 files changed, 1129 insertions(+), 2 deletions(-) create mode 100644 tests/eu_search_cfi.c create mode 100644 tests/eu_search_die.c create mode 100644 tests/eu_search_lines.c create mode 100644 tests/eu_search_macros.c create mode 100755 tests/run-eu-search-tests.sh diff --git a/tests/.gitignore b/tests/.gitignore index 30f5800b..360c13a3 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -67,6 +67,10 @@ /elfstrtab /elf-print-reloc-syms /emptyfile +/eu_search_cfi +/eu_search_die +/eu_search_lines +/eu_search_macros /fillfile /find-prologues /funcretval diff --git a/tests/Makefile.am b/tests/Makefile.am index cfed54b7..1642d4e9 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -32,6 +32,10 @@ else tests_rpath = no endif +if USE_LOCKS + AM_CFLAGS += -pthread +endif + check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \ showptable update1 update2 update3 update4 test-nlist \ show-die-info get-files next-files get-lines next-lines \ @@ -64,6 +68,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \ msg_tst system-elf-libelf-test system-elf-gelf-test \ nvidia_extended_linemap_libdw elf-print-reloc-syms \ cu-dwp-section-info declfiles \ + eu_search_cfi eu_search_die eu_search_lines eu_search_macros \ $(asm_TESTS) asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \ @@ -216,7 +221,7 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile test-nlist \ run-readelf-dw-form-indirect.sh run-strip-largealign.sh \ run-readelf-Dd.sh run-dwfl-core-noncontig.sh run-cu-dwp-section-info.sh \ run-declfiles.sh \ - run-sysroot.sh + run-sysroot.sh run-eu-search-tests.sh if !BIARCH export ELFUTILS_DISABLE_BIARCH = 1 @@ -669,7 +674,8 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \ testfile-dwp-4-cu-index-overflow.dwp.bz2 \ testfile-dwp-cu-index-overflow.source \ testfile-define-file.bz2 \ -testfile-sysroot.tar.bz2 run-sysroot.sh +testfile-sysroot.tar.bz2 run-sysroot.sh \ +run-eu-search-tests.sh if USE_VALGRIND @@ -849,6 +855,15 @@ nvidia_extended_linemap_libdw_LDADD = $(libelf) $(libdw) elf_print_reloc_syms_LDADD = $(libelf) cu_dwp_section_info_LDADD = $(libdw) declfiles_LDADD = $(libdw) +eu_search_cfi_LDFLAGS = $(if $(filter undefined,$(origin USE_LOCKS)),-pthread) $(AM_LDFLAGS) +eu_search_die_LDFLAGS = $(if $(filter undefined,$(origin USE_LOCKS)),-pthread) $(AM_LDFLAGS) +eu_search_lines_LDFLAGS = $(if $(filter undefined,$(origin USE_LOCKS)),-pthread) $(AM_LDFLAGS) +eu_search_macros_LDFLAGS = $(if $(filter undefined,$(origin USE_LOCKS)),-pthread) $(AM_LDFLAGS) +eu_search_cfi_LDADD = $(libeu) $(libelf) $(libdw) +eu_search_die_LDADD = $(libdw) +eu_search_lines_LDADD = $(libdw) $(libelf) +eu_search_macros_LDADD = $(libdw) # We want to test the libelf headers against the system elf.h header. # Don't include any -I CPPFLAGS. Except when we install our own elf.h. diff --git a/tests/eu_search_cfi.c b/tests/eu_search_cfi.c new file mode 100644 index ..
[PATCH 3/9 v2] lib: Add eu_tsearch, eu_tfind, eu_tdelete and eu_tdestroy
From: Heather McIntyre
Add new 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.
Move the rwlock_* macros out of eu-config.h and into a new header file
locks.h. This was done so that the rwlock_* macros can be included
in libdwP.h without having to also include the rest of eu-config.h.
Signed-off-by: Heather S. McIntyre
Signed-off-by: Aaron Merey
Signed-off-by: Mark Wielaard
v2 changes:
This patch replaces v1 03/16 and 14/16.
---
lib/Makefile.am | 5 ++-
lib/eu-config.h | 30 +
lib/eu-search.c | 85 +++
lib/eu-search.h | 64 ++
lib/locks.h | 62 +
libdw/cfi.h | 6 +--
libdw/cie.c | 10 +++--
libdw/dwarf_begin_elf.c | 7 +--
libdw/dwarf_end.c | 17 +++
libdw/dwarf_getcfi.c | 5 ++-
libdw/dwarf_getlocation.c | 24 +-
libdw/dwarf_getmacros.c | 6 +--
libdw/dwarf_getsrclines.c | 8 ++--
libdw/fde.c | 6 +--
libdw/frame-cache.c | 8 ++--
libdw/libdwP.h| 26 ---
libdw/libdw_find_split_unit.c | 10 ++---
libdw/libdw_findcu.c | 18
libdwfl/cu.c | 8 ++--
libdwfl/dwfl_module.c | 4 +-
libdwfl/libdwflP.h| 3 +-
libelf/elf_begin.c| 2 +
libelf/elf_end.c | 13 +++---
libelf/elf_getdata_rawchunk.c | 12 ++---
libelf/libelfP.h | 10 +++--
25 files changed, 331 insertions(+), 118 deletions(-)
create mode 100644 lib/eu-search.c
create mode 100644 lib/eu-search.h
create mode 100644 lib/locks.h
diff --git a/lib/Makefile.am b/lib/Makefile.am
index b3bb929f..e324c18d 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -34,10 +34,11 @@ AM_CPPFLAGS += -I$(srcdir)/../libelf
noinst_LIBRARIES = libeu.a
libeu_a_SOURCES = xasprintf.c xstrdup.c xstrndup.c xmalloc.c next_prime.c \
- crc32.c crc32_file.c \
+ crc32.c crc32_file.c eu-search.c \
color.c error.c printversion.c
noinst_HEADERS = fixedsizehash.h libeu.h system.h dynamicsizehash.h list.h \
eu-config.h color.h printversion.h bpf.h \
-atomics.h stdatomic-fbsd.h dynamicsizehash_concurrent.h
+atomics.h stdatomic-fbsd.h dynamicsizehash_concurrent.h \
+eu-search.h locks.h
EXTRA_DIST = dynamicsizehash.c dynamicsizehash_concurrent.c
diff --git a/lib/eu-config.h b/lib/eu-config.h
index feb079db..a38d75da 100644
--- a/lib/eu-config.h
+++ b/lib/eu-config.h
@@ -29,35 +29,7 @@
#ifndef EU_CONFIG_H
#define EU_CONFIG_H1
-#ifdef USE_LOCKS
-# include
-# include
-# define rwlock_define(class,name) class pthread_rwlock_t name
-# define once_define(class,name) class pthread_once_t name = PTHREAD_ONCE_INIT
-# define RWLOCK_CALL(call) \
- ({ int _err = pthread_rwlock_ ## call; assert_perror (_err); })
-# define ONCE_CALL(call) \
- ({ int _err = pthread_ ## call; assert_perror (_err); })
-# define rwlock_init(lock) RWLOCK_CALL (init (&lock, NULL))
-# define rwlock_fini(lock) RWLOCK_CALL (destroy (&lock))
-# define rwlock_rdlock(lock) RWLOCK_CALL (rdlock (&lock))
-# define rwlock_wrlock(lock) RWLOCK_CALL (wrlock (&lock))
-# define rwlock_unlock(lock) RWLOCK_CALL (unlock (&lock))
-# define once(once_control, init_routine) \
- ONCE_CALL (once (&once_control, init_routine))
-#else
-/* Eventually we will allow multi-threaded applications to use the
- libraries. Therefore we will add the necessary locking although
- the macros used expand to nothing for now. */
-# define rwlock_define(class,name) class int name
-# define rwlock_init(lock) ((void) (lock))
-# define rwlock_fini(lock) ((void) (lock))
-# define rwlock_rdlock(lock) ((void) (lock))
-# define rwlock_wrlock(lock) ((void) (lock))
-# define rwlock_unlock(lock) ((void) (lock))
-# define once_define(class,name)
-# define once(once_control, init_routine) init_routine()
-#endif /* USE_LOCKS */
+#include
#include
/* gettext helper macros. */
diff --git a/lib/eu-search.c b/lib/eu-search.c
new file mode 100644
index ..b7256eba
--- /dev/null
+++ b/lib/eu-search.c
@@ -0,0 +1,85 @@
+/* Definitions for thread-safe tsearch/tfind
+ Copyright (C) 2023 Rice University
+ This file is part of elfutils.
+
+ This file is free software; you can redistribute it and/or modify
+ it under the terms of either
+
+ * the GNU Lesser General Public License as published by the Free
+ Software Foundation; either version 3 of the License, or (at
+ your option) any later version
+
+ or
+
+ * the GNU General Public License as published by the Free
+ Software Foundation; either version
[PATCH 7/9 v2] libdw: Make libdw_findcu thread-safe
From: Heather McIntyre
* libdw/libdw_findcu.c (__libdw_findcu): Use eu_tfind
and dwarf_lock
(__libdw_intern_next_unit): Use per-Dwarf_CU locks.
Signed-off-by: Heather S. McIntyre
Signed-off-by: Aaron Merey
Signed-off-by: Mark Wielaard
---
v2 changes:
Use per-Dwarf_CU lock instead of a global lock.
libdw/libdw_findcu.c | 47
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/libdw/libdw_findcu.c b/libdw/libdw_findcu.c
index 72cf261c..8acff448 100644
--- a/libdw/libdw_findcu.c
+++ b/libdw/libdw_findcu.c
@@ -177,6 +177,8 @@ __libdw_intern_next_unit (Dwarf *dbg, bool debug_types)
newp->startp = data->d_buf + newp->start;
newp->endp = data->d_buf + newp->end;
eu_search_tree_init (&newp->locs_tree);
+ rwlock_init (newp->abbrev_lock);
+ rwlock_init (newp->split_lock);
/* v4 debug type units have version == 4 and unit_type == DW_UT_type. */
if (debug_types)
@@ -243,27 +245,38 @@ __libdw_findcu (Dwarf *dbg, Dwarf_Off start, bool
v4_debug_types)
/* Maybe we already know that CU. */
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;
- if (start < *next_offset)
-{
- __libdw_seterrno (DWARF_E_INVALID_DWARF);
- return NULL;
-}
+ rwlock_wrlock (dbg->dwarf_lock);
- /* No. Then read more CUs. */
- while (1)
-{
- struct Dwarf_CU *newp = __libdw_intern_next_unit (dbg, v4_debug_types);
- if (newp == NULL)
- return NULL;
-
- /* Is this the one we are looking for? */
- if (start < *next_offset || start == newp->start)
- return newp;
-}
- /* NOTREACHED */
+ if (start < *next_offset)
+__libdw_seterrno (DWARF_E_INVALID_DWARF);
+ else
+{
+ /* No. Then read more CUs. */
+ while (1)
+{
+ struct Dwarf_CU *newp = __libdw_intern_next_unit (dbg,
+v4_debug_types);
+ if (newp == NULL)
+{
+ result = NULL;
+ break;
+}
+
+ /* Is this the one we are looking for? */
+ if (start < *next_offset || start == newp->start)
+{
+ result = newp;
+ break;
+}
+}
+}
+
+ rwlock_unlock (dbg->dwarf_lock);
+ return result;
}
struct Dwarf_CU *
--
2.45.2
[PATCH 9/9 v2] configure: No longer mark --enable-thread-safety as EXPERIMENTAL
From: Heather McIntyre
* configure.ac (--enable-thread-safety): Remove experimental
warning.
Signed-off-by: Heather S. McIntyre
Signed-off-by: Aaron Merey
Signed-off-by: Mark Wielaard
---
No changes in v2. However PR31967 (datarace in elf_compress[_gnu])
was recently filed and this patch set does not address it. Maybe
we should hold off on merging this patch and keep thread safety
"experimental" until PR31967 is fixed.
configure.ac | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index 24e68d94..2c885c8a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -79,12 +79,10 @@ AC_DEFINE_UNQUOTED(DEFAULT_AR_DETERMINISTIC,
$default_ar_deterministic,
AC_ARG_ENABLE([thread-safety],
AS_HELP_STRING([--enable-thread-safety],
- [enable thread safety of libraries EXPERIMENTAL]),
+ [enable thread safety of libraries]),
use_locks=$enableval, use_locks=no)
AM_CONDITIONAL(USE_LOCKS, test "$use_locks" = yes)
AS_IF([test "$use_locks" = yes], [AC_DEFINE(USE_LOCKS)])
-AS_IF([test "$use_locks" = yes],
- [AC_MSG_WARN([thread-safety is EXPERIMENTAL tests might fail.])])
AH_TEMPLATE([USE_LOCKS], [Defined if libraries should be thread-safe.])
@@ -939,10 +937,10 @@ AC_MSG_NOTICE([
Symbol versioning : ${enable_symbol_versioning}
NOT RECOMMENDED FEATURES (should all be no)
-Experimental thread safety : ${use_locks}
install elf.h : ${install_elfh}
OTHER FEATURES
+Enable thread safety : ${use_locks}
Deterministic archives by default : ${default_ar_deterministic}
Native language support: ${USE_NLS}
Extra Valgrind annotations : ${use_vg_annotations}
--
2.45.2
