mirror/binutils-gdb
2
0
Fork 0
mirror of https://sourceware.org/git/binutils-gdb.git synced 2024-11-21 01:12:32 +08:00
Code Issues Packages Projects Releases Wiki Activity

Thread-safety improvements for bfd_check_format_matches

Browse Source 
A gdb bug found that bfd_check_format_matches has some data races when
called from multiple threads.

In particular, it changes the BFD error handler, which is a global.
It also has a local static variable ("in_check_format") that is used
for recursion detection.  And, finally, it may emit warnings to the
per-xvec warning array, which is a global.

This patch removes all the races here.

The first part of patch is to change _bfd_error_handler to directly
handle the needs of bfd_check_format_matches.  This way, the error
handler does not need to be changed.

This change lets us use the new per-thread global
(error_handler_messages, replacing error_handler_bfd) to also remove
the need for in_check_format -- a single variable suffices.

Finally, the global per-xvec array is replaced with a new type that
holds the error messages.  The outermost such type is stack-allocated
in bfd_check_format_matches.

I tested this using the binutils test suite.  I also built gdb with
thread sanitizer and ran the test case that was noted as failing.
Finally, Alan sent me the test file that caused the addition of the
xvec warning code in the first place, and I confirmed that "nm-new"
has the same behavior on this file both before and after this patch.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31264
Co-Authored-By: Alan Modra <amodra@gmail.com>
This commit is contained in:
Tom Tromey 2024-02-12 18:06:56 -07:00
parent 6732c57eee
commit bacc61fd3e
4 changed files with 183 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

142
bfd/bfd.c
View File

@ -1565,10 +1565,91 @@ err_sprintf (void *stream, const char *fmt, ...)
return total;
}
/* Communicate the bfd processed by bfd_check_format_matches to the
error handling function error_handler_sprintf. */
/*
INTERNAL
.{* Cached _bfd_check_format messages are put in this. *}
.struct per_xvec_message
.{
. struct per_xvec_message *next;
. char message[];
.};
.
.{* A list of per_xvec_message objects. The targ field indicates
. which xvec this list holds; PER_XVEC_NO_TARGET is only set for the
. root of the list and indicates that the entry isn't yet used. The
. abfd field is only needed in the root entry of the list. *}
.struct per_xvec_messages
.{
. bfd *abfd;
. const bfd_target *targ;
. struct per_xvec_message *messages;
. struct per_xvec_messages *next;
.};
.
.#define PER_XVEC_NO_TARGET ((const bfd_target *) -1)
*/
static bfd *error_handler_bfd;
/* Helper function to find or allocate the correct per-xvec object
when emitting a message. */
static struct per_xvec_message *
_bfd_per_xvec_warn (struct per_xvec_messages *messages, size_t alloc)
{
const bfd_target *targ = messages->abfd->xvec;
struct per_xvec_messages *prev = NULL;
struct per_xvec_messages *iter = messages;
if (iter->targ == PER_XVEC_NO_TARGET)
iter->targ = targ;
else
for (; iter != NULL; iter = iter->next)
{
if (iter->targ == targ)
break;
prev = iter;
}
if (iter == NULL)
{
iter = bfd_malloc (sizeof (*iter));
if (iter == NULL)
return NULL;
iter->abfd = messages->abfd;
iter->targ = targ;
iter->messages = NULL;
iter->next = NULL;
prev->next = iter;
}
struct per_xvec_message **m = &iter->messages;
int count = 0;
while (*m)
{
m = &(*m)->next;
count++;
}
/* Anti-fuzzer measure. Don't cache more than 5 messages. */
if (count < 5)
{
*m = bfd_malloc (sizeof (**m) + alloc);
if (*m != NULL)
(*m)->next = NULL;
}
return *m;
}
/* Communicate the error-message container processed by
bfd_check_format_matches to the error handling function
error_handler_sprintf. When non-NULL, _bfd_error_handler will call
error_handler_sprintf; when NULL, _bfd_error_internal will be used
instead. */
static TLS struct per_xvec_messages *error_handler_messages;
/* A special value for error_handler_messages that indicates that the
error should simply be ignored. */
#define IGNORE_ERROR_MESSAGES ((struct per_xvec_messages *) -1)
/* An error handler that prints to a string, then dups that string to
a per-xvec cache. */
@ -1585,12 +1666,12 @@ error_handler_sprintf (const char *fmt, va_list ap)
_bfd_print (err_sprintf, &error_stream, fmt, ap);
size_t len = error_stream.ptr - error_buf;
struct per_xvec_message **warn
= _bfd_per_xvec_warn (error_handler_bfd->xvec, len + 1);
if (*warn)
struct per_xvec_message *warn
= _bfd_per_xvec_warn (error_handler_messages, len + 1);
if (warn)
{
memcpy ((*warn)->message, error_buf, len);
(*warn)->message[len] = 0;
memcpy (warn->message, error_buf, len);
warn->message[len] = 0;
}
}
@ -1628,7 +1709,14 @@ _bfd_error_handler (const char *fmt, ...)
va_list ap;
va_start (ap, fmt);
_bfd_error_internal (fmt, ap);
if (error_handler_messages == IGNORE_ERROR_MESSAGES)
{
/* Nothing. */
}
else if (error_handler_messages != NULL)
error_handler_sprintf (fmt, ap);
else
_bfd_error_internal (fmt, ap);
va_end (ap);
}
@ -1659,18 +1747,42 @@ INTERNAL_FUNCTION
_bfd_set_error_handler_caching
SYNOPSIS
bfd_error_handler_type _bfd_set_error_handler_caching (bfd *);
struct per_xvec_messages *_bfd_set_error_handler_caching (struct per_xvec_messages *);
DESCRIPTION
Set the BFD error handler function to one that stores messages
to the per_xvec_warn array. Returns the previous function.
to the per_xvec_messages object. Returns the previous object
to which messages are stored. Note that two sequential calls
to this with a non-NULL argument will cause output to be
dropped, rather than gathered.
*/
bfd_error_handler_type
_bfd_set_error_handler_caching (bfd *abfd)
struct per_xvec_messages *
_bfd_set_error_handler_caching (struct per_xvec_messages *messages)
{
error_handler_bfd = abfd;
return bfd_set_error_handler (error_handler_sprintf);
struct per_xvec_messages *old = error_handler_messages;
if (old == NULL)
error_handler_messages = messages;
else
error_handler_messages = IGNORE_ERROR_MESSAGES;
return old;
}
/*
INTERNAL_FUNCTION
_bfd_restore_error_handler_caching
SYNOPSIS
void _bfd_restore_error_handler_caching (struct per_xvec_messages *);
DESCRIPTION
Reset the BFD error handler object to an earlier value.
*/
void
_bfd_restore_error_handler_caching (struct per_xvec_messages *old)
{
error_handler_messages = old;
}
/*

71
bfd/format.c
View File

@ -272,10 +272,34 @@ clear_warnmsg (struct per_xvec_message **list)
*list = NULL;
}
/* Free all the storage in LIST. Note that the first element of LIST
is special and is assumed to be stack-allocated. TARG is used for
re-issuing warning messages. If TARG is PER_XVEC_NO_TARGET, then
it acts like a sort of wildcard -- messages are only reissued if
they are all associated with a single BFD target, regardless of
which one it is. If TARG is anything else, then only messages
associated with TARG are emitted. */
static void
null_error_handler (const char *fmt ATTRIBUTE_UNUSED,
va_list ap ATTRIBUTE_UNUSED)
print_and_clear_messages (struct per_xvec_messages *list,
const bfd_target *targ)
{
struct per_xvec_messages *iter = list;
if (targ == PER_XVEC_NO_TARGET && list->next == NULL)
print_warnmsg (&list->messages);
while (iter != NULL)
{
struct per_xvec_messages *next = iter->next;
if (iter->targ == targ)
print_warnmsg (&iter->messages);
clear_warnmsg (&iter->messages);
if (iter != list)
free (iter);
iter = next;
}
}
/* This a copy of lto_section defined in GCC (lto-streamer.h). */
@ -357,8 +381,8 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
unsigned int initial_section_id = _bfd_section_id;
struct bfd_preserve preserve, preserve_match;
bfd_cleanup cleanup = NULL;
bfd_error_handler_type orig_error_handler;
static int in_check_format;
struct per_xvec_messages messages = { abfd, PER_XVEC_NO_TARGET, NULL, NULL };
struct per_xvec_messages *orig_messages;
if (matching != NULL)
*matching = NULL;
@ -392,11 +416,7 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
/* Don't report errors on recursive calls checking the first element
of an archive. */
if (in_check_format)
orig_error_handler = bfd_set_error_handler (null_error_handler);
else
orig_error_handler = _bfd_set_error_handler_caching (abfd);
++in_check_format;
orig_messages = _bfd_set_error_handler_caching (&messages);
preserve_match.marker = NULL;
if (!bfd_preserve_save (abfd, &preserve, NULL))
@ -638,15 +658,9 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
if (preserve_match.marker != NULL)
bfd_preserve_finish (abfd, &preserve_match);
bfd_preserve_finish (abfd, &preserve);
bfd_set_error_handler (orig_error_handler);
_bfd_restore_error_handler_caching (orig_messages);
struct per_xvec_message **list = _bfd_per_xvec_warn (abfd->xvec, 0);
if (*list)
print_warnmsg (list);
list = _bfd_per_xvec_warn (NULL, 0);
for (size_t i = 0; i < _bfd_target_vector_entries + 1; i++)
clear_warnmsg (list++);
--in_check_format;
print_and_clear_messages (&messages, abfd->xvec);
bfd_set_lto_type (abfd);
@ -692,27 +706,8 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
if (preserve_match.marker != NULL)
bfd_preserve_finish (abfd, &preserve_match);
bfd_preserve_restore (abfd, &preserve);
bfd_set_error_handler (orig_error_handler);
struct per_xvec_message **list = _bfd_per_xvec_warn (NULL, 0);
struct per_xvec_message **one = NULL;
for (size_t i = 0; i < _bfd_target_vector_entries + 1; i++)
{
if (list[i])
{
if (!one)
one = list + i;
else
{
one = NULL;
break;
}
}
}
if (one)
print_warnmsg (one);
for (size_t i = 0; i < _bfd_target_vector_entries + 1; i++)
clear_warnmsg (list++);
--in_check_format;
_bfd_restore_error_handler_caching (orig_messages);
print_and_clear_messages (&messages, PER_XVEC_NO_TARGET);
return false;
}

33
bfd/libbfd.h
View File

@ -963,7 +963,29 @@ void _bfd_clear_error_data (void) ATTRIBUTE_HIDDEN;
char *bfd_asprintf (const char *fmt, ...) ATTRIBUTE_HIDDEN;
bfd_error_handler_type _bfd_set_error_handler_caching (bfd *) ATTRIBUTE_HIDDEN;
/* Cached _bfd_check_format messages are put in this. */
struct per_xvec_message
{
struct per_xvec_message *next;
char message[];
};
/* A list of per_xvec_message objects. The targ field indicates
which xvec this list holds; PER_XVEC_NO_TARGET is only set for the
root of the list and indicates that the entry isn't yet used. The
abfd field is only needed in the root entry of the list. */
struct per_xvec_messages
{
bfd *abfd;
const bfd_target *targ;
struct per_xvec_message *messages;
struct per_xvec_messages *next;
};
#define PER_XVEC_NO_TARGET ((const bfd_target *) -1)
struct per_xvec_messages *_bfd_set_error_handler_caching (struct per_xvec_messages *) ATTRIBUTE_HIDDEN;
void _bfd_restore_error_handler_caching (struct per_xvec_messages *) ATTRIBUTE_HIDDEN;
const char *_bfd_get_error_program_name (void) ATTRIBUTE_HIDDEN;
@ -3708,15 +3730,6 @@ bool _bfd_write_stab_strings (bfd *, struct stab_info *) ATTRIBUTE_HIDDEN;
bfd_vma _bfd_stab_section_offset (asection *, void *, bfd_vma) ATTRIBUTE_HIDDEN;
/* Extracted from targets.c. */
/* Cached _bfd_check_format messages are put in this. */
struct per_xvec_message
{
struct per_xvec_message *next;
char message[];
};
struct per_xvec_message **_bfd_per_xvec_warn (const bfd_target *, size_t) ATTRIBUTE_HIDDEN;
#ifdef __cplusplus
}
#endif

58
bfd/targets.c
View File

@ -1454,9 +1454,6 @@ const bfd_target *const *const bfd_associated_vector = _bfd_associated_vector;
number of entries that the array could possibly need. */
const size_t _bfd_target_vector_entries = ARRAY_SIZE (_bfd_target_vector);
/* A place to stash a warning from _bfd_check_format. */
static struct per_xvec_message *per_xvec_warn[ARRAY_SIZE (_bfd_target_vector)
+ 1];
/* This array maps configuration triplets onto BFD vectors. */
@ -1476,61 +1473,6 @@ static const struct targmatch bfd_target_match[] = {
{ NULL, NULL }
};
/*
INTERNAL
.{* Cached _bfd_check_format messages are put in this. *}
.struct per_xvec_message
.{
. struct per_xvec_message *next;
. char message[];
.};
.
INTERNAL_FUNCTION
_bfd_per_xvec_warn
SYNOPSIS
struct per_xvec_message **_bfd_per_xvec_warn (const bfd_target *, size_t);
DESCRIPTION
Return a location for the given target xvec to use for
warnings specific to that target. If TARG is NULL, returns
the array of per_xvec_message pointers, otherwise if ALLOC is
zero, returns a pointer to a pointer to the list of messages
for TARG, otherwise (both TARG and ALLOC non-zero), allocates
a new per_xvec_message with space for a string of ALLOC
bytes and returns a pointer to a pointer to it. May return a
pointer to a NULL pointer on allocation failure.
*/
struct per_xvec_message **
_bfd_per_xvec_warn (const bfd_target *targ, size_t alloc)
{
size_t idx;
if (!targ)
return per_xvec_warn;
for (idx = 0; idx < ARRAY_SIZE (_bfd_target_vector); idx++)
if (_bfd_target_vector[idx] == targ)
break;
struct per_xvec_message **m = per_xvec_warn + idx;
if (!alloc)
return m;
int count = 0;
while (*m)
{
m = &(*m)->next;
count++;
}
/* Anti-fuzzer measure. Don't cache more than 5 messages. */
if (count < 5)
{
*m = bfd_malloc (sizeof (**m) + alloc);
if (*m != NULL)
(*m)->next = NULL;
}
return m;
}
/* Find a target vector, given a name or configuration triplet. */
static const bfd_target *