[PATCH] bfd: Print cached message in _bfd_abort

[PATCH] bfd: Print cached message in _bfd_abort

[H.J. Lu]

bfd_check_format_matches sets _bfd_error_handler to error_handler_sprintf
to cache warning and error messages before restoring the original
_bfd_error_handler.  When something went wrong in between and _bfd_abort
is called, there is no abort message.  Update _bfd_abort to print cached
messages.

	PR ld/31444
	* bfd.c (_bfd_abort): Check and print cached messages.
---
 bfd/bfd.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/bfd/bfd.c b/bfd/bfd.c
index c9f374e0948..be4c9af0f62 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -2077,7 +2077,26 @@ _bfd_abort (const char *file, int line, const char *fn)
       /* xgettext:c-format */
       (_("BFD %s internal error, aborting at %s:%d\n"),
        BFD_VERSION_STRING, file, line);
-  _bfd_error_handler (_("Please report this bug.\n"));
+
+  struct per_xvec_message **list
+    = _bfd_per_xvec_warn (error_handler_bfd->xvec, 0);
+  if (*list)
+    {
+      fflush (stdout);
+      for (struct per_xvec_message *warn = *list; warn;)
+	{
+	  struct per_xvec_message *next = warn->next;
+	  fputs (warn->message, stderr);
+	  fputc ('\n', stderr);
+	  free (warn);
+	  warn = next;
+	}
+      fputs (_("Please report this bug.\n"), stderr);
+      fflush (stderr);
+    }
+  else
+    _bfd_error_handler (_("Please report this bug.\n"));
+
   _exit (EXIT_FAILURE);
 }
 
-- 
2.44.0

Re: [PATCH] bfd: Print cached message in _bfd_abort

[H.J. Lu]

On Sat, Mar 2, 2024 at 5:21 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> bfd_check_format_matches sets _bfd_error_handler to error_handler_sprintf
> to cache warning and error messages before restoring the original
> _bfd_error_handler.  When something went wrong in between and _bfd_abort
> is called, there is no abort message.  Update _bfd_abort to print cached
> messages.
>
>         PR ld/31444
>         * bfd.c (_bfd_abort): Check and print cached messages.
> ---
>  bfd/bfd.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/bfd/bfd.c b/bfd/bfd.c
> index c9f374e0948..be4c9af0f62 100644
> --- a/bfd/bfd.c
> +++ b/bfd/bfd.c
> @@ -2077,7 +2077,26 @@ _bfd_abort (const char *file, int line, const char *fn)
>        /* xgettext:c-format */
>        (_("BFD %s internal error, aborting at %s:%d\n"),
>         BFD_VERSION_STRING, file, line);
> -  _bfd_error_handler (_("Please report this bug.\n"));
> +
> +  struct per_xvec_message **list
> +    = _bfd_per_xvec_warn (error_handler_bfd->xvec, 0);
> +  if (*list)
> +    {
> +      fflush (stdout);
> +      for (struct per_xvec_message *warn = *list; warn;)
> +       {
> +         struct per_xvec_message *next = warn->next;
> +         fputs (warn->message, stderr);
> +         fputc ('\n', stderr);
> +         free (warn);
> +         warn = next;
> +       }
> +      fputs (_("Please report this bug.\n"), stderr);
> +      fflush (stderr);
> +    }
> +  else
> +    _bfd_error_handler (_("Please report this bug.\n"));
> +
>    _exit (EXIT_FAILURE);
>  }
>
> --
> 2.44.0
>

Hi Nick, Alan,

Do you have any comments on this patch?

Thanks.

-- 
H.J.

Re: [PATCH] bfd: Print cached message in _bfd_abort

[Alan Modra]

On Tue, Mar 05, 2024 at 06:37:56AM -0800, H.J. Lu wrote:
> On Sat, Mar 2, 2024 at 5:21 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > bfd_check_format_matches sets _bfd_error_handler to error_handler_sprintf
> > to cache warning and error messages before restoring the original
> > _bfd_error_handler.  When something went wrong in between and _bfd_abort
> > is called, there is no abort message.  Update _bfd_abort to print cached
> > messages.
> Hi Nick, Alan,
> 
> Do you have any comments on this patch?

Hi HJ,
Thanks for finding this hole.

Is it worth worrying about any cached messages?  A bfd_abort is for
something the programmer who wrote the code said "can't happen", or
for catastrophic conditions where you can't recover.  I'd be inclined
to make the following patch instead, on the grounds that the fewer
things we need to rely on in an abort situation the better.

	* bfd.c (_bfd_abort): Don't use _bfd_error_handler.

diff --git a/bfd/bfd.c b/bfd/bfd.c
index 6c822656cc8..71732a0f92b 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -2050,17 +2050,17 @@ bfd_assert (const char *file, int line)
 void
 _bfd_abort (const char *file, int line, const char *fn)
 {
+  fflush (stdout);
+
   if (fn != NULL)
-    _bfd_error_handler
-      /* xgettext:c-format */
-      (_("BFD %s internal error, aborting at %s:%d in %s\n"),
-       BFD_VERSION_STRING, file, line, fn);
+    fprintf (stderr, _("%s: BFD %s internal error, aborting at %s:%d in %s\n"),
+	     _bfd_get_error_program_name (), BFD_VERSION_STRING,
+	     file, line, fn);
   else
-    _bfd_error_handler
-      /* xgettext:c-format */
-      (_("BFD %s internal error, aborting at %s:%d\n"),
-       BFD_VERSION_STRING, file, line);
-  _bfd_error_handler (_("Please report this bug.\n"));
+    fprintf (stderr, _("%s: BFD %s internal error, aborting at %s:%d\n"),
+	     _bfd_get_error_program_name (), BFD_VERSION_STRING,
+	     file, line);
+  fprintf (stderr, _("Please report this bug.\n"));
   _exit (EXIT_FAILURE);
 }
 

-- 
Alan Modra
Australia Development Lab, IBM
commit 57e52b2260b5a4567cd12a7c181f3e5be2b26e87

Re: [PATCH] bfd: Print cached message in _bfd_abort

[H.J. Lu]

On Tue, Mar 5, 2024 at 1:49 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Tue, Mar 05, 2024 at 06:37:56AM -0800, H.J. Lu wrote:
> > On Sat, Mar 2, 2024 at 5:21 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > bfd_check_format_matches sets _bfd_error_handler to error_handler_sprintf
> > > to cache warning and error messages before restoring the original
> > > _bfd_error_handler.  When something went wrong in between and _bfd_abort
> > > is called, there is no abort message.  Update _bfd_abort to print cached
> > > messages.
> > Hi Nick, Alan,
> >
> > Do you have any comments on this patch?
>
> Hi HJ,
> Thanks for finding this hole.
>
> Is it worth worrying about any cached messages?  A bfd_abort is for
> something the programmer who wrote the code said "can't happen", or
> for catastrophic conditions where you can't recover.  I'd be inclined
> to make the following patch instead, on the grounds that the fewer
> things we need to rely on in an abort situation the better.
>
>         * bfd.c (_bfd_abort): Don't use _bfd_error_handler.
>
> diff --git a/bfd/bfd.c b/bfd/bfd.c
> index 6c822656cc8..71732a0f92b 100644
> --- a/bfd/bfd.c
> +++ b/bfd/bfd.c
> @@ -2050,17 +2050,17 @@ bfd_assert (const char *file, int line)
>  void
>  _bfd_abort (const char *file, int line, const char *fn)
>  {
> +  fflush (stdout);
> +
>    if (fn != NULL)
> -    _bfd_error_handler
> -      /* xgettext:c-format */
> -      (_("BFD %s internal error, aborting at %s:%d in %s\n"),
> -       BFD_VERSION_STRING, file, line, fn);
> +    fprintf (stderr, _("%s: BFD %s internal error, aborting at %s:%d in %s\n"),
> +            _bfd_get_error_program_name (), BFD_VERSION_STRING,
> +            file, line, fn);
>    else
> -    _bfd_error_handler
> -      /* xgettext:c-format */
> -      (_("BFD %s internal error, aborting at %s:%d\n"),
> -       BFD_VERSION_STRING, file, line);
> -  _bfd_error_handler (_("Please report this bug.\n"));
> +    fprintf (stderr, _("%s: BFD %s internal error, aborting at %s:%d\n"),
> +            _bfd_get_error_program_name (), BFD_VERSION_STRING,
> +            file, line);
> +  fprintf (stderr, _("Please report this bug.\n"));
>    _exit (EXIT_FAILURE);
>  }
>
>
> --
> Alan Modra
> Australia Development Lab, IBM
> commit 57e52b2260b5a4567cd12a7c181f3e5be2b26e87

LGTM.   Can you add PR ld/31444 to the commit log?

-- 
H.J.