Re: [PATCH 1/3] Make several more BFD globals thread-local

[Tom Tromey <tom@tromey.com>]

Tom> What I meant when I wrote the above is that basically there are two sets
Tom> of globals to consider in this patch: the ones related to error emission
Tom> (_bfd_error_internal and error_handler_bfd), and then the other ones
Tom> related to the caching error message code (per_xvec_warn,
Tom> in_check_format).

Tom> I thought that making the latter ones thread-local felt hackish, because
Tom> they could somewhat easily be attributes of a BFD and not globals at
Tom> all.

I took a stab at some of this tonight and discovered that since
in_check_format is only used by the error-reporting code, it can be
combined with the other flag.

I've appended a rough draft of this.

I haven't tackled the xvec stuff yet.

Also I think it'd be good if print_warnmsg re-emitted the errors using
_bfd_error_handler.  This would let gdb print them the way it likes.
However... that's a change to semantics so I don't know if that's OK or
whether another error emitter is required.  I guess I don't really know
how to provoke one of these messages and when they would matter.  Like,
I see that ldmain.c installs an error handler -- is it intentional that
this be bypassed by errors occurring in bfd_check_format_matches?

thanks,
Tom

diff --git a/bfd/bfd.c b/bfd/bfd.c
index 6c822656cc8..238aa71b4c5 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -1528,9 +1528,15 @@ err_sprintf (void *stream, const char *fmt, ...)
 }
 
 /* Communicate the bfd processed by bfd_check_format_matches to the
-   error handling function error_handler_sprintf.  */
+   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 bfd *error_handler_bfd;
+static TLS bfd *error_handler_bfd;
+
+/* A special value for error_handler_bfd that indicates that the error
+   should simply be ignored.  */
+#define IGNORE_ERROR_BFD ((bfd *) -1)
 
 /* An error handler that prints to a string, then dups that string to
    a per-xvec cache.  */
@@ -1590,7 +1596,14 @@ _bfd_error_handler (const char *fmt, ...)
   va_list ap;
 
   va_start (ap, fmt);
-  _bfd_error_internal (fmt, ap);
+  if (error_handler_bfd == IGNORE_ERROR_BFD)
+    {
+      /* Nothing.  */
+    }
+  else if (error_handler_bfd != NULL)
+    error_handler_sprintf (fmt, ap);
+  else
+    _bfd_error_internal (fmt, ap);
   va_end (ap);
 }
 
@@ -1621,18 +1634,42 @@ INTERNAL_FUNCTION
 	_bfd_set_error_handler_caching
 
 SYNOPSIS
-	bfd_error_handler_type _bfd_set_error_handler_caching (bfd *);
+	bfd *_bfd_set_error_handler_caching (bfd *);
 
 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_warn array.  Returns the previous BFD to which
+	messages are stored.  Note that two sequential calls to this
+	with a non-NULL BFD will cause output to be dropped, rather
+	than gathered.
 */
 
-bfd_error_handler_type
+bfd *
 _bfd_set_error_handler_caching (bfd *abfd)
+{
+  bfd *old = error_handler_bfd;
+  if (old == NULL)
+    error_handler_bfd = abfd;
+  else
+    error_handler_bfd = IGNORE_ERROR_BFD;
+  return old;
+}
+
+/*
+INTERNAL_FUNCTION
+	_bfd_restore_error_handler_caching
+
+SYNOPSIS
+	void _bfd_restore_error_handler_caching (bfd *);
+
+DESCRIPTION
+	Reset the BFD error handler function an earlier value.
+*/
+
+void
+_bfd_restore_error_handler_caching (bfd *abfd)
 {
   error_handler_bfd = abfd;
-  return bfd_set_error_handler (error_handler_sprintf);
 }
 
 /*
diff --git a/bfd/format.c b/bfd/format.c
index 47c3e9ba35a..42b50421288 100644
--- a/bfd/format.c
+++ b/bfd/format.c
@@ -279,12 +279,6 @@ clear_warnmsg (struct per_xvec_message **list)
   *list = NULL;
 }
 
-static void
-null_error_handler (const char *fmt ATTRIBUTE_UNUSED,
-		    va_list ap ATTRIBUTE_UNUSED)
-{
-}
-
 /*
 FUNCTION
 	bfd_check_format_matches
@@ -320,8 +314,7 @@ 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;
+  bfd *orig_error_bfd;
 
   if (matching != NULL)
     *matching = NULL;
@@ -350,13 +343,10 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
   abfd->format = format;
   save_targ = abfd->xvec;
 
-  /* 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;
+  /* We don't want to report errors on recursive calls checking the
+     first element of an archive.  This is handled by the
+     error-handler code, which see.  */
+  orig_error_bfd = _bfd_set_error_handler_caching (abfd);
 
   preserve_match.marker = NULL;
   if (!bfd_preserve_save (abfd, &preserve, NULL))
@@ -598,7 +588,7 @@ 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_error_bfd);
 
       struct per_xvec_message **list = _bfd_per_xvec_warn (abfd->xvec, 0);
       if (*list)
@@ -606,7 +596,6 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
       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;
 
       /* File position has moved, BTW.  */
       return true;
@@ -650,7 +639,7 @@ 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);
+  _bfd_restore_error_handler_caching (orig_error_bfd);
   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++)
@@ -670,7 +659,6 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
     print_warnmsg (one);
   for (size_t i = 0; i < _bfd_target_vector_entries + 1; i++)
     clear_warnmsg (list++);
-  --in_check_format;
   return false;
 }
 
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index 40bbe6a3886..40e9c88e711 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -933,7 +933,9 @@ 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;
+bfd *_bfd_set_error_handler_caching (bfd *) ATTRIBUTE_HIDDEN;
+
+void _bfd_restore_error_handler_caching (bfd *) ATTRIBUTE_HIDDEN;
 
 const char *_bfd_get_error_program_name (void) ATTRIBUTE_HIDDEN;