From: Tom Tromey Date: Sat, 9 Nov 2024 20:45:50 +0000 (-0700) Subject: Lock bfd_stat and bfd_get_mtime X-Git-Tag: gdb-16-branchpoint~118 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a357defdfe2af758d5ec9f0040949bf1db4cbe53;p=thirdparty%2Fbinutils-gdb.git Lock bfd_stat and bfd_get_mtime PR gdb/31713 points out some races when using the background DWARF reader. This particular patch fixes some of these, namely the ones when using the sim. In this case, the 'load' command calls reopen_exec_file, which calls bfd_stat, which introduces a race. BFD only locks globals -- concurrent use of a single BFD must be handled by the application. To this end, this patch adds locked wrappers for bfd_stat and bfd_get_mtime. I couldn't reproduce these data races but the original reporter tested the patch and confirms that it helps. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31713 Approved-By: Andrew Burgess --- diff --git a/gdb/corefile.c b/gdb/corefile.c index f6ec3cd5ca1..be0f1075488 100644 --- a/gdb/corefile.c +++ b/gdb/corefile.c @@ -51,7 +51,7 @@ reopen_exec_file (void) /* If the timestamp of the exec file has changed, reopen it. */ struct stat st; - int res = bfd_stat (exec_bfd, &st); + int res = gdb_bfd_stat (exec_bfd, &st); if (res == 0 && current_program_space->ebfd_mtime != 0 @@ -70,8 +70,8 @@ validate_files (void) if (!core_file_matches_executable_p (current_program_space->core_bfd (), current_program_space->exec_bfd ())) warning (_("core file may not match specified executable file.")); - else if (bfd_get_mtime (current_program_space->exec_bfd ()) - > bfd_get_mtime (current_program_space->core_bfd ())) + else if (gdb_bfd_get_mtime (current_program_space->exec_bfd ()) + > gdb_bfd_get_mtime (current_program_space->core_bfd ())) warning (_("exec file is newer than core file.")); } } diff --git a/gdb/exec.c b/gdb/exec.c index 82d9266b7e3..73280ad59e7 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -483,7 +483,7 @@ exec_file_attach (const char *filename, int from_tty) = build_section_table (current_program_space->exec_bfd ()); current_program_space->ebfd_mtime - = bfd_get_mtime (current_program_space->exec_bfd ()); + = gdb_bfd_get_mtime (current_program_space->exec_bfd ()); validate_files (); diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c index 3683eeba52b..c233551b4e8 100644 --- a/gdb/gdb_bfd.c +++ b/gdb/gdb_bfd.c @@ -1121,6 +1121,32 @@ gdb_bfd_get_full_section_contents (bfd *abfd, asection *section, section_size); } +/* See gdb_bfd.h. */ + +int +gdb_bfd_stat (bfd *abfd, struct stat *sbuf) +{ +#if CXX_STD_THREAD + gdb_bfd_data *gdata = (gdb_bfd_data *) bfd_usrdata (abfd); + std::lock_guard guard (gdata->per_bfd_mutex); +#endif + + return bfd_stat (abfd, sbuf); +} + +/* See gdb_bfd.h. */ + +long +gdb_bfd_get_mtime (bfd *abfd) +{ +#if CXX_STD_THREAD + gdb_bfd_data *gdata = (gdb_bfd_data *) bfd_usrdata (abfd); + std::lock_guard guard (gdata->per_bfd_mutex); +#endif + + return bfd_get_mtime (abfd); +} + #define AMBIGUOUS_MESS1 ".\nMatching formats:" #define AMBIGUOUS_MESS2 \ ".\nUse \"set gnutarget format-name\" to specify the format." diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h index 331570d769c..a93d9aa92e8 100644 --- a/gdb/gdb_bfd.h +++ b/gdb/gdb_bfd.h @@ -251,6 +251,17 @@ gdb_bfd_sections (const gdb_bfd_ref_ptr &abfd) return gdb_bfd_section_range (abfd->sections); }; +/* A wrapper for bfd_stat that acquires the per-BFD lock on ABFD. */ + +extern int gdb_bfd_stat (bfd *abfd, struct stat *sbuf) + ATTRIBUTE_WARN_UNUSED_RESULT; + +/* A wrapper for bfd_get_mtime that acquires the per-BFD lock on + ABFD. */ + +extern long gdb_bfd_get_mtime (bfd *abfd) + ATTRIBUTE_WARN_UNUSED_RESULT; + /* A wrapper for bfd_errmsg to produce a more helpful error message in the case of bfd_error_file_ambiguously recognized. MATCHING, if non-NULL, is the corresponding argument to diff --git a/gdb/machoread.c b/gdb/machoread.c index ef6cf660629..ac764c09496 100644 --- a/gdb/machoread.c +++ b/gdb/machoread.c @@ -434,7 +434,8 @@ macho_add_oso_symfile (oso_el *oso, const gdb_bfd_ref_ptr &abfd, return; } - if (abfd->my_archive == NULL && oso->mtime != bfd_get_mtime (abfd.get ())) + if (abfd->my_archive == nullptr + && oso->mtime != gdb_bfd_get_mtime (abfd.get ())) { warning (_("`%s': file time stamp mismatch."), oso->name); return; diff --git a/gdb/objfiles.c b/gdb/objfiles.c index 76d17f548a5..84866e1e30a 100644 --- a/gdb/objfiles.c +++ b/gdb/objfiles.c @@ -281,7 +281,7 @@ objfile::objfile (gdb_bfd_ref_ptr bfd_, program_space *pspace, if (obfd != nullptr) { - mtime = bfd_get_mtime (obfd.get ()); + mtime = gdb_bfd_get_mtime (obfd.get ()); /* Build section table. */ build_objfile_section_table (this); diff --git a/gdb/symfile.c b/gdb/symfile.c index fd21d2d2c23..4d5f3d26262 100644 --- a/gdb/symfile.c +++ b/gdb/symfile.c @@ -1275,9 +1275,9 @@ separate_debug_file_exists (const std::string &name, unsigned long crc, numbers will never set st_ino to zero, this is merely an optimization, so we do not need to worry about false negatives. */ - if (bfd_stat (abfd.get (), &abfd_stat) == 0 + if (gdb_bfd_stat (abfd.get (), &abfd_stat) == 0 && abfd_stat.st_ino != 0 - && bfd_stat (parent_objfile->obfd.get (), &parent_stat) == 0) + && gdb_bfd_stat (parent_objfile->obfd.get (), &parent_stat) == 0) { if (abfd_stat.st_dev == parent_stat.st_dev && abfd_stat.st_ino == parent_stat.st_ino) @@ -2496,7 +2496,7 @@ reread_symbols (int from_tty) continue; struct stat new_statbuf; - int res = bfd_stat (objfile->obfd.get (), &new_statbuf); + int res = gdb_bfd_stat (objfile->obfd.get (), &new_statbuf); if (res != 0) { /* If this object is from an archive (what you usually create