Lazy-loading of the srclines and srcfiles is protected by a per-CU mutex src_lock, but multiple threads querying different CUs might try to load the srclines/files concurrently. This creates a data race in get_lines_or_files where one thread attempts to read a files_lines_s field while another thread sets it.
Fix this by adding a new per-Dwarf mutex lines_files_lock that is held for the duration of get_lines_or_files. eu_tfind and eu_tsearch calls in get_lines_or_files have been replaced with their _nolock variants. Signed-off-by: Aaron Merey <[email protected]> --- libdw/dwarf_begin_elf.c | 1 + libdw/dwarf_end.c | 1 + libdw/dwarf_getsrclines.c | 31 ++++++++++++++++++++----------- libdw/libdwP.h | 4 ++++ 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c index e157d103..0521e53c 100644 --- a/libdw/dwarf_begin_elf.c +++ b/libdw/dwarf_begin_elf.c @@ -449,6 +449,7 @@ valid_p (Dwarf *result) mutex_init (result->dwarf_lock); mutex_init (result->macro_lock); mutex_init (result->dwp_lock); + mutex_init (result->lines_files_lock); eu_search_tree_init (&result->cu_tree); eu_search_tree_init (&result->tu_tree); eu_search_tree_init (&result->split_tree); diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c index a7799773..aeb0c422 100644 --- a/libdw/dwarf_end.c +++ b/libdw/dwarf_end.c @@ -136,6 +136,7 @@ dwarf_end (Dwarf *dwarf) mutex_fini (dwarf->dwarf_lock); mutex_fini (dwarf->macro_lock); mutex_fini (dwarf->dwp_lock); + mutex_fini (dwarf->lines_files_lock); /* Free the pubnames helper structure. */ free (dwarf->pubnames_sets); diff --git a/libdw/dwarf_getsrclines.c b/libdw/dwarf_getsrclines.c index 5d48a934..cdd3bcc0 100644 --- a/libdw/dwarf_getsrclines.c +++ b/libdw/dwarf_getsrclines.c @@ -1332,9 +1332,13 @@ get_lines_or_files (Dwarf *dbg, Dwarf_Off debug_line_offset, const char *comp_dir, unsigned address_size, Dwarf_Lines **linesp, Dwarf_Files **filesp) { + int ret = -1; + + mutex_lock (dbg->lines_files_lock); + struct files_lines_s fake = { .debug_line_offset = debug_line_offset }; - struct files_lines_s **found = eu_tfind (&fake, &dbg->files_lines_tree, - files_lines_compare); + struct files_lines_s **found = eu_tfind_nolock (&fake, &dbg->files_lines_tree, + files_lines_compare); if (found == NULL) { /* This .debug_line is being read for the first time. */ @@ -1342,7 +1346,7 @@ get_lines_or_files (Dwarf *dbg, Dwarf_Off debug_line_offset, if (data == NULL || __libdw_offset_in_section (dbg, IDX_debug_line, debug_line_offset, 1) != 0) - return -1; + goto out; const unsigned char *linep = data->d_buf + debug_line_offset; const unsigned char *lineendp = data->d_buf + data->d_size; @@ -1359,19 +1363,20 @@ get_lines_or_files (Dwarf *dbg, Dwarf_Off debug_line_offset, { if (read_srcfiles (dbg, linep, lineendp, comp_dir, address_size, NULL, &node->files) != 0) - return -1; + goto out; } else if (read_srclines (dbg, linep, lineendp, comp_dir, address_size, &node->lines, &node->files, false) != 0) - return -1; + goto out; node->debug_line_offset = debug_line_offset; - found = eu_tsearch (node, &dbg->files_lines_tree, files_lines_compare); + found = eu_tsearch_nolock (node, &dbg->files_lines_tree, + files_lines_compare); if (found == NULL) { __libdw_seterrno (DWARF_E_NOMEM); - return -1; + goto out; } } else if (*found != NULL @@ -1384,7 +1389,7 @@ get_lines_or_files (Dwarf *dbg, Dwarf_Off debug_line_offset, if (data == NULL || __libdw_offset_in_section (dbg, IDX_debug_line, debug_line_offset, 1) != 0) - return -1; + goto out; const unsigned char *linep = data->d_buf + debug_line_offset; const unsigned char *lineendp = data->d_buf + data->d_size; @@ -1393,7 +1398,7 @@ get_lines_or_files (Dwarf *dbg, Dwarf_Off debug_line_offset, if (read_srclines (dbg, linep, lineendp, comp_dir, address_size, &node->lines, &node->files, true) != 0) - return -1; + goto out; } else if (*found != NULL && (*found)->files == NULL @@ -1401,7 +1406,7 @@ get_lines_or_files (Dwarf *dbg, Dwarf_Off debug_line_offset, { /* If srclines were read then srcfiles should have also been read. */ __libdw_seterrno (DWARF_E_INVALID_DEBUG_LINE); - return -1; + goto out; } if (linesp != NULL) @@ -1410,7 +1415,11 @@ get_lines_or_files (Dwarf *dbg, Dwarf_Off debug_line_offset, if (filesp != NULL) *filesp = (*found)->files; - return 0; + ret = 0; + +out: + mutex_unlock (dbg->lines_files_lock); + return ret; } int diff --git a/libdw/libdwP.h b/libdw/libdwP.h index d21164db..7d0b09a0 100644 --- a/libdw/libdwP.h +++ b/libdw/libdwP.h @@ -268,6 +268,10 @@ struct Dwarf /* Synchronize lazy-loading of dwp_dwarf and dwp_fd in try_dwp_file. */ mutex_define(, dwp_lock); + /* Synchronize lazy-loading of Dwarf_Lines and Dwarf_Files in + get_lines_or_files. */ + mutex_define(, lines_files_lock); + /* Internal memory handling. This is basically a simplified thread-local reimplementation of obstacks. Unfortunately the standard obstack implementation is not usable in libraries. */ -- 2.55.0
