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 <aburgess@redhat.com>
This commit is contained in:
Tom Tromey
2024-11-09 13:45:50 -07:00
parent e1093de6a0
commit a357defdfe
7 changed files with 47 additions and 9 deletions

View File

@@ -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."));
}
}

View File

@@ -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 ();

View File

@@ -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<std::mutex> 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<std::mutex> 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."

View File

@@ -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

View File

@@ -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;

View File

@@ -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);

View File

@@ -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