public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Re: The problem with warning in elf_object_p
@ 2022-12-17  8:12 Alan Modra
  0 siblings, 0 replies; 2+ messages in thread
From: Alan Modra @ 2022-12-17  8:12 UTC (permalink / raw)
  To: binutils

Commit 5aa0f10c424e added a per_xvec_warn array to provide support for
warnings from elf_object_p (and a later patch for warnings from
pe_bfd_object_p) to be cached and then only printed if the target
matches.  It was quite limited in the style of message supported, only
one message could be printed, and didn't really meet the stated aim of
only warning when a target matches:  There are many other errors and
warnings that can be emitted by functions called from elf_object_p.

So this patch extends the error handler functions to support printing
to a string buffer, extends per_xvec_warn to support multiple errors/
warnings, and hooks this all into bfd_check_format_matches.  If
bfd_check_format_matches succeeds then any errors/warnings are printed
for the matching target.  If bfd_check_format_matches fails either due
to no match or to multiple matches and only one target vector produced
errors, then those errors are printed.

	* bfd.c (MAX_ARGS): Define, use throughout.
	(print_func): New typedef.
	(_bfd_doprnt): Add new print param.  Replace calls to fprintf
	with print.
	(PRINT_TYPE): Similarly.
	(error_handler_fprintf): Renamed from error_handler_internal.
	Use _bfd_get_error_program_name.  Add fprintf arg.  Move code
	setting up args..
	(_bfd_doprnt_scan): ..to here.  Add ap param.
	(struct buf_stream): New.
	(err_sprintf): New function.
	(error_handler_bfd): New static variable.
	(error_handler_sprintf): New function.
	(_bfd_set_error_handler_caching): New function.
	(_bfd_get_error_program_name): New function.
	* elfcode.h (elf_swap_shdr_in): Use _bfd_error_handler in
	warning messages.
	(elf_object_p): Likewise.
	* format.c (print_warnmsg): New function.
	(clear_warnmsg): Rewrite.
	(null_error_handler): New function.
	(bfd_check_format_matches): Ignore warnings from recursive calls
	checking first element of an archive.  Use caching error handler
	otherwise.  Print warnings on successful match, or when only one
	target has emitted warnings/errors.
	* peicode.h (pe_bfd_object_p): Use _bfd_error_handler in
	warning messages.
	* targets.c (per_xvec_warn): Change type of array elements.
	(struct per_xvec_message): New.
	(_bfd_per_xvec_warn): Rewrite.
	* Makefile.am (LIBBFD_H_FILES): Add bfd.c.
	* Makefile.in: Regenerate.
	* bfd-in2.h: Regenerate.
	* libbfd.h: Regenerate.

diff --git a/bfd/Makefile.am b/bfd/Makefile.am
index da55a0bb99e..6d8722e0cca 100644
--- a/bfd/Makefile.am
+++ b/bfd/Makefile.am
@@ -932,7 +932,7 @@ BFD_H_FILES = bfd-in.h init.c opncls.c libbfd.c \
 	syms.c bfd.c archive.c corefile.c targets.c format.c \
 	linker.c simple.c compress.c
 BFD64_H_FILES = archive64.c
-LIBBFD_H_FILES = libbfd-in.h libbfd.c bfdio.c bfdwin.c \
+LIBBFD_H_FILES = libbfd-in.h libbfd.c bfd.c bfdio.c bfdwin.c \
 	cache.c reloc.c section.c targets.c archures.c linker.c
 LIBCOFF_H_FILES = libcoff-in.h coffcode.h
 
diff --git a/bfd/Makefile.in b/bfd/Makefile.in
index 60252f2da54..5cd2f3752b2 100644
--- a/bfd/Makefile.in
+++ b/bfd/Makefile.in
@@ -1226,7 +1226,7 @@ BFD_H_FILES = bfd-in.h init.c opncls.c libbfd.c \
 	linker.c simple.c compress.c
 
 BFD64_H_FILES = archive64.c
-LIBBFD_H_FILES = libbfd-in.h libbfd.c bfdio.c bfdwin.c \
+LIBBFD_H_FILES = libbfd-in.h libbfd.c bfd.c bfdio.c bfdwin.c \
 	cache.c reloc.c section.c targets.c archures.c linker.c
 
 LIBCOFF_H_FILES = libcoff-in.h coffcode.h
diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index d407e593a6c..297ce125168 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -7816,6 +7816,13 @@ bfd_keep_unused_section_symbols (const bfd *abfd)
   return abfd->xvec->keep_unused_section_symbols;
 }
 
+/* Cached _bfd_check_format messages are put in this.  */
+struct per_xvec_message
+{
+  struct per_xvec_message *next;
+  char message[];
+};
+
 bool bfd_set_default_target (const char *name);
 
 const bfd_target *bfd_find_target (const char *target_name, bfd *abfd);
diff --git a/bfd/bfd.c b/bfd/bfd.c
index 8d9c9c67743..12cb4bca0ec 100644
--- a/bfd/bfd.c
+++ b/bfd/bfd.c
@@ -890,6 +890,10 @@ union _bfd_doprnt_args
   } type;
 };
 
+/* Maximum number of _bfd_error_handler args.  Don't increase this
+   without changing the code handling positional parameters.  */
+#define MAX_ARGS 9
+
 /* This macro and _bfd_doprnt taken from libiberty _doprnt.c, tidied a
    little and extended to handle '%pA', '%pB' and positional parameters.  */
 
@@ -897,11 +901,14 @@ union _bfd_doprnt_args
   do								\
     {								\
       TYPE value = (TYPE) args[arg_no].FIELD;			\
-      result = fprintf (stream, specifier, value);		\
+      result = print (stream, specifier, value);		\
     } while (0)
 
+typedef int (*print_func) (void *, const char *, ...);
+
 static int
-_bfd_doprnt (FILE *stream, const char *format, union _bfd_doprnt_args *args)
+_bfd_doprnt (print_func print, void *stream, const char *format,
+	     union _bfd_doprnt_args *args)
 {
   const char *ptr = format;
   char specifier[128];
@@ -917,9 +924,9 @@ _bfd_doprnt (FILE *stream, const char *format, union _bfd_doprnt_args *args)
 	  /* While we have regular characters, print them.  */
 	  char *end = strchr (ptr, '%');
 	  if (end != NULL)
-	    result = fprintf (stream, "%.*s", (int) (end - ptr), ptr);
+	    result = print (stream, "%.*s", (int) (end - ptr), ptr);
 	  else
-	    result = fprintf (stream, "%s", ptr);
+	    result = print (stream, "%s", ptr);
 	  ptr += result;
 	}
       else if (ptr[1] == '%')
@@ -1103,9 +1110,9 @@ _bfd_doprnt (FILE *stream, const char *format, union _bfd_doprnt_args *args)
 								 sec)) != NULL)
 		    group = ci->name;
 		  if (group != NULL)
-		    result = fprintf (stream, "%s[%s]", sec->name, group);
+		    result = print (stream, "%s[%s]", sec->name, group);
 		  else
-		    result = fprintf (stream, "%s", sec->name);
+		    result = print (stream, "%s", sec->name);
 		}
 	      else if (*ptr == 'B')
 		{
@@ -1119,11 +1126,11 @@ _bfd_doprnt (FILE *stream, const char *format, union _bfd_doprnt_args *args)
 		    abort ();
 		  else if (abfd->my_archive
 			   && !bfd_is_thin_archive (abfd->my_archive))
-		    result = fprintf (stream, "%s(%s)",
-				      bfd_get_filename (abfd->my_archive),
-				      bfd_get_filename (abfd));
+		    result = print (stream, "%s(%s)",
+				    bfd_get_filename (abfd->my_archive),
+				    bfd_get_filename (abfd));
 		  else
-		    result = fprintf (stream, "%s", bfd_get_filename (abfd));
+		    result = print (stream, "%s", bfd_get_filename (abfd));
 		}
 	      else
 		PRINT_TYPE (void *, p);
@@ -1144,11 +1151,14 @@ _bfd_doprnt (FILE *stream, const char *format, union _bfd_doprnt_args *args)
 /* First pass over FORMAT to gather ARGS.  Returns number of args.  */
 
 static unsigned int
-_bfd_doprnt_scan (const char *format, union _bfd_doprnt_args *args)
+_bfd_doprnt_scan (const char *format, va_list ap, union _bfd_doprnt_args *args)
 {
   const char *ptr = format;
   unsigned int arg_count = 0;
 
+  for (unsigned int i = 0; i < MAX_ARGS; i++)
+    args[i].type = Bad;
+
   while (*ptr != '\0')
     {
       if (*ptr != '%')
@@ -1190,7 +1200,7 @@ _bfd_doprnt_scan (const char *format, union _bfd_doprnt_args *args)
 		  arg_index = *ptr - '1';
 		  ptr += 2;
 		}
-	      if (arg_index >= 9)
+	      if (arg_index >= MAX_ARGS)
 		abort ();
 	      args[arg_index].type = Int;
 	      arg_count++;
@@ -1215,7 +1225,7 @@ _bfd_doprnt_scan (const char *format, union _bfd_doprnt_args *args)
 		      arg_index = *ptr - '1';
 		      ptr += 2;
 		    }
-		  if (arg_index >= 9)
+		  if (arg_index >= MAX_ARGS)
 		    abort ();
 		  args[arg_index].type = Int;
 		  arg_count++;
@@ -1303,27 +1313,14 @@ _bfd_doprnt_scan (const char *format, union _bfd_doprnt_args *args)
 	      abort();
 	    }
 
-	  if (arg_no >= 9)
+	  if (arg_no >= MAX_ARGS)
 	    abort ();
 	  args[arg_no].type = arg_type;
 	  arg_count++;
 	}
     }
 
-  return arg_count;
-}
-
-static void
-error_handler_internal (const char *fmt, va_list ap)
-{
-  unsigned int i, arg_count;
-  union _bfd_doprnt_args args[9];
-
-  for (i = 0; i < sizeof (args) / sizeof (args[0]); i++)
-    args[i].type = Bad;
-
-  arg_count = _bfd_doprnt_scan (fmt, args);
-  for (i = 0; i < arg_count; i++)
+  for (unsigned int i = 0; i < arg_count; i++)
     {
       switch (args[i].type)
 	{
@@ -1350,15 +1347,24 @@ error_handler_internal (const char *fmt, va_list ap)
 	}
     }
 
+  return arg_count;
+}
+
+/* The standard error handler that prints to stderr.  */
+
+static void
+error_handler_fprintf (const char *fmt, va_list ap)
+{
+  union _bfd_doprnt_args args[MAX_ARGS];
+
+  _bfd_doprnt_scan (fmt, ap, args);
+
   /* PR 4992: Don't interrupt output being sent to stdout.  */
   fflush (stdout);
 
-  if (_bfd_error_program_name != NULL)
-    fprintf (stderr, "%s: ", _bfd_error_program_name);
-  else
-    fprintf (stderr, "BFD: ");
+  fprintf (stderr, "%s: ", _bfd_get_error_program_name ());
 
-  _bfd_doprnt (stderr, fmt, args);
+  _bfd_doprnt ((print_func) fprintf, stderr, fmt, args);
 
   /* On AIX, putc is implemented as a macro that triggers a -Wunused-value
      warning, so use the fputc function to avoid it.  */
@@ -1366,13 +1372,77 @@ error_handler_internal (const char *fmt, va_list ap)
   fflush (stderr);
 }
 
+/* Control printing to a string buffer.  */
+struct buf_stream
+{
+  char *ptr;
+  int left;
+};
+
+/* An fprintf like function that instead prints to a string buffer.  */
+
+static int
+err_sprintf (void *stream, const char *fmt, ...)
+{
+  struct buf_stream *s = stream;
+  va_list ap;
+
+  va_start (ap, fmt);
+  int total = vsnprintf (s->ptr, s->left, fmt, ap);
+  va_end (ap);
+  if (total < 0)
+    ;
+  else if (total > s->left)
+    {
+      s->ptr += s->left;
+      s->left = 0;
+    }
+  else
+    {
+      s->ptr += total;
+      s->left -= total;
+    }
+  return total;
+}
+
+/* Communicate the bfd processed by bfd_check_format_matches to the
+   error handling function error_handler_sprintf.  */
+
+static bfd *error_handler_bfd;
+
+/* An error handler that prints to a string, then dups that string to
+   a per-xvec cache.  */
+
+static void
+error_handler_sprintf (const char *fmt, va_list ap)
+{
+  union _bfd_doprnt_args args[MAX_ARGS];
+  char error_buf[1024];
+  struct buf_stream error_stream;
+
+  _bfd_doprnt_scan (fmt, ap, args);
+
+  error_stream.ptr = error_buf;
+  error_stream.left = sizeof (error_buf);
+  _bfd_doprnt (err_sprintf, &error_stream, fmt, args);
+
+  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)
+    {
+      memcpy ((*warn)->message, error_buf, len);
+      (*warn)->message[len] = 0;
+    }
+}
+
 /* This is a function pointer to the routine which should handle BFD
    error messages.  It is called when a BFD routine encounters an
    error for which it wants to print a message.  Going through a
    function pointer permits a program linked against BFD to intercept
    the messages and deal with them itself.  */
 
-static bfd_error_handler_type _bfd_error_internal = error_handler_internal;
+static bfd_error_handler_type _bfd_error_internal = error_handler_fprintf;
 
 /*
 FUNCTION
@@ -1426,6 +1496,25 @@ bfd_set_error_handler (bfd_error_handler_type pnew)
   return pold;
 }
 
+/*
+INTERNAL_FUNCTION
+	_bfd_set_error_handler_caching
+
+SYNOPSIS
+	bfd_error_handler_type _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.
+*/
+
+bfd_error_handler_type
+_bfd_set_error_handler_caching (bfd *abfd)
+{
+  error_handler_bfd = abfd;
+  return bfd_set_error_handler (error_handler_sprintf);
+}
+
 /*
 FUNCTION
 	bfd_set_error_program_name
@@ -1446,6 +1535,25 @@ bfd_set_error_program_name (const char *name)
   _bfd_error_program_name = name;
 }
 
+/*
+INTERNAL_FUNCTION
+	_bfd_get_error_program_name
+
+SYNOPSIS
+	const char *_bfd_get_error_program_name (void);
+
+DESCRIPTION
+	Get the program name used when printing a BFD error.
+*/
+
+const char *
+_bfd_get_error_program_name (void)
+{
+  if (_bfd_error_program_name != NULL)
+    return _bfd_error_program_name;
+  return "BFD";
+}
+
 /*
 SUBSECTION
 	BFD assert handler
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 1392240f481..7a4de82a0df 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -326,8 +326,8 @@ elf_swap_shdr_in (bfd *abfd,
 	      || dst->sh_size > filesize - dst->sh_offset)
 	  && !abfd->read_only)
 	{
-	  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
-	  *warn = _("warning: %pB has a section extending past end of file");
+	  _bfd_error_handler (_("warning: %pB has a section "
+				"extending past end of file"), abfd);
 	  abfd->read_only = 1;
 	}
     }
@@ -773,8 +773,8 @@ elf_object_p (bfd *abfd)
 	  i_ehdrp->e_shstrndx = SHN_UNDEF;
 	  if (!abfd->read_only)
 	    {
-	      const char **warn = _bfd_per_xvec_warn (abfd->xvec);
-	      *warn = _("warning: %pB has a corrupt string table index");
+	      _bfd_error_handler
+		(_("warning: %pB has a corrupt string table index"), abfd);
 	      abfd->read_only = 1;
 	    }
 	}
@@ -821,9 +821,8 @@ elf_object_p (bfd *abfd)
 	      i_phdr->p_align &= -i_phdr->p_align;
 	      if (!abfd->read_only)
 		{
-		  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
-		  *warn = _("warning: %pB has a program header "
-			    "with invalid alignment");
+		  _bfd_error_handler (_("warning: %pB has a program header "
+					"with invalid alignment"), abfd);
 		  abfd->read_only = 1;
 		}
 	    }
diff --git a/bfd/format.c b/bfd/format.c
index 7e2813c97a4..97fba30213f 100644
--- a/bfd/format.c
+++ b/bfd/format.c
@@ -203,10 +203,36 @@ bfd_preserve_finish (bfd *abfd ATTRIBUTE_UNUSED, struct bfd_preserve *preserve)
 }
 
 static void
-clear_warnmsg (const bfd_target *targ)
+print_warnmsg (struct per_xvec_message **list)
+{
+  fflush (stdout);
+  fprintf (stderr, "%s: ", _bfd_get_error_program_name ());
+
+  for (struct per_xvec_message *warn = *list; warn; warn = warn->next)
+    {
+      fputs (warn->message, stderr);
+      fputc ('\n', stderr);
+    }
+  fflush (stderr);
+}
+
+static void
+clear_warnmsg (struct per_xvec_message **list)
+{
+  struct per_xvec_message *warn = *list;
+  while (warn)
+    {
+      struct per_xvec_message *next = warn->next;
+      free (warn);
+      warn = next;
+    }
+  *list = NULL;
+}
+
+static void
+null_error_handler (const char *fmt ATTRIBUTE_UNUSED,
+		    va_list ap ATTRIBUTE_UNUSED)
 {
-  const char **warn = _bfd_per_xvec_warn (targ);
-  *warn = NULL;
 }
 
 /*
@@ -244,6 +270,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;
 
   if (matching != NULL)
     *matching = NULL;
@@ -272,6 +300,14 @@ 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;
+
   preserve_match.marker = NULL;
   if (!bfd_preserve_save (abfd, &preserve, NULL))
     goto err_ret;
@@ -282,7 +318,6 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
       if (bfd_seek (abfd, (file_ptr) 0, SEEK_SET) != 0)	/* rewind! */
 	goto err_ret;
 
-      clear_warnmsg (abfd->xvec);
       cleanup = BFD_SEND_FMT (abfd, _bfd_check_format, (abfd));
 
       if (cleanup)
@@ -349,7 +384,6 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
       if (bfd_seek (abfd, (file_ptr) 0, SEEK_SET) != 0)
 	goto err_ret;
 
-      clear_warnmsg (abfd->xvec);
       cleanup = BFD_SEND_FMT (abfd, _bfd_check_format, (abfd));
       if (cleanup)
 	{
@@ -514,13 +548,15 @@ 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);
 
-      if (!abfd->my_archive)
-	{
-	  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
-	  if (*warn)
-	    _bfd_error_handler (*warn, abfd);
-	}
+      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;
 
       /* File position has moved, BTW.  */
       return true;
@@ -536,10 +572,7 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
       abfd->xvec = save_targ;
       abfd->format = bfd_unknown;
       free (matching_vector);
-      if (preserve_match.marker != NULL)
-	bfd_preserve_finish (abfd, &preserve_match);
-      bfd_preserve_restore (abfd, &preserve);
-      return false;
+      goto out;
     }
 
   /* Restore original target type and format.  */
@@ -563,9 +596,31 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
     free (matching_vector);
   if (cleanup)
     cleanup (abfd);
+ out:
   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;
   return false;
 }
 
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index 2dec8109bb8..cba6d42ccee 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -1,6 +1,6 @@
 /* DO NOT EDIT!  -*- buffer-read-only: t -*-  This file is automatically
-   generated from "libbfd-in.h", "libbfd.c", "bfdio.c", "bfdwin.c",
-   "cache.c", "reloc.c", "section.c", "targets.c", "archures.c"
+   generated from "libbfd-in.h", "libbfd.c", "bfd.c", "bfdio.c",
+   "bfdwin.c", "cache.c", "reloc.c", "section.c", "targets.c", "archures.c"
    and "linker.c".
    Run "make headers" in your build bfd/ to regenerate.  */
 
@@ -989,6 +989,11 @@ bool bfd_write_bigendian_4byte_int (bfd *, unsigned int);
 
 unsigned int bfd_log2 (bfd_vma x);
 
+/* Extracted from bfd.c.  */
+bfd_error_handler_type _bfd_set_error_handler_caching (bfd *);
+
+const char *_bfd_get_error_program_name (void);
+
 /* Extracted from bfdio.c.  */
 struct bfd_iovec
 {
@@ -3554,7 +3559,7 @@ bool _bfd_unrecognized_reloc
 bool _bfd_section_size_insane (bfd *abfd, asection *sec);
 
 /* Extracted from targets.c.  */
-const char **_bfd_per_xvec_warn (const bfd_target *);
+struct per_xvec_message **_bfd_per_xvec_warn (const bfd_target *, size_t);
 
 /* Extracted from archures.c.  */
 extern const bfd_arch_info_type bfd_default_arch_struct;
diff --git a/bfd/peicode.h b/bfd/peicode.h
index f7ba24ae10a..6462b8e47ff 100644
--- a/bfd/peicode.h
+++ b/bfd/peicode.h
@@ -1527,8 +1527,8 @@ pe_bfd_object_p (bfd * abfd)
       if ((a->SectionAlignment & -a->SectionAlignment) != a->SectionAlignment
 	  || a->SectionAlignment >= 0x80000000)
 	{
-	  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
-	  *warn = _("%pB: adjusting invalid SectionAlignment");
+	  _bfd_error_handler (_("%pB: adjusting invalid SectionAlignment"),
+				abfd);
 	  a->SectionAlignment &= -a->SectionAlignment;
 	  if (a->SectionAlignment >= 0x80000000)
 	    a->SectionAlignment = 0x40000000;
@@ -1537,18 +1537,15 @@ pe_bfd_object_p (bfd * abfd)
       if ((a->FileAlignment & -a->FileAlignment) != a->FileAlignment
 	  || a->FileAlignment > a->SectionAlignment)
 	{
-	  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
-	  *warn = _("%pB: adjusting invalid FileAlignment");
+	  _bfd_error_handler (_("%pB: adjusting invalid FileAlignment"),
+			      abfd);
 	  a->FileAlignment &= -a->FileAlignment;
 	  if (a->FileAlignment > a->SectionAlignment)
 	    a->FileAlignment = a->SectionAlignment;
 	}
 
       if (a->NumberOfRvaAndSizes > IMAGE_NUMBEROF_DIRECTORY_ENTRIES)
-	{
-	  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
-	  *warn = _("%pB: invalid NumberOfRvaAndSizes");
-	}
+	_bfd_error_handler (_("%pB: invalid NumberOfRvaAndSizes"), abfd);
     }
 
   result = coff_real_object_p (abfd, internal_f.f_nscns, &internal_f,
diff --git a/bfd/targets.c b/bfd/targets.c
index 0bb85b64cc2..cf928a5900a 100644
--- a/bfd/targets.c
+++ b/bfd/targets.c
@@ -1460,7 +1460,8 @@ const bfd_target *const *const bfd_associated_vector = _bfd_associated_vector;
 const size_t _bfd_target_vector_entries = ARRAY_SIZE (_bfd_target_vector);
 
 /* A place to stash a warning from _bfd_check_format.  */
-static const char *per_xvec_warn[ARRAY_SIZE (_bfd_target_vector) + 1];
+static struct per_xvec_message *per_xvec_warn[ARRAY_SIZE (_bfd_target_vector)
+					      + 1];
 \f
 /* This array maps configuration triplets onto BFD vectors.  */
 
@@ -1481,26 +1482,58 @@ static const struct targmatch bfd_target_match[] = {
 };
 
 /*
+CODE_FRAGMENT
+.{* 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
-	const char **_bfd_per_xvec_warn (const bfd_target *);
+	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.
+	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.
 */
 
-const char **
-_bfd_per_xvec_warn (const bfd_target *targ)
+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;
-  return per_xvec_warn + idx;
+  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.  */

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 2+ messages in thread

* The problem with warning in elf_object_p
@ 2022-09-24  2:08 Alan Modra
  0 siblings, 0 replies; 2+ messages in thread
From: Alan Modra @ 2022-09-24  2:08 UTC (permalink / raw)
  To: binutils

elfcode.h can emit three warnings in elf_object_p for various things,
"section extending past end of file", "corrupt string table index",
and "program header with invalid alignment".  The problem with doing
this is that the warning can be emitted for multiple possible targets
as each one is tried.  I was looking at a fuzzer testcase that had an
object file with 6144 program headers, 5316 of which had invalid
alignment.  It would be bad enough to get 5316 messages all the same,
but this object was contained in an archive and resulted in 4975776
repeats!

Some trimming can be done by not warning if the bfd is already marked
read_only, as is done for the "section extending past end of file"
warning, but that still results in an unacceptable number of
warnings for object files in archives.  Besides that, it is just wrong
to warn about a problem detected by a target elf_object_p other than
the one that actually matches.  At some point we might have more
target specific warnings.

So what to do?  One obvious solution is to remove the warnings.
Another is to poke any warning strings into the target xvec, emitting
them if that xvec is the final one chosen.  This also has the benefit
of solving the archive problem.  A warning when recursing into
_bfd_check_format for the first element of the archive (to find the
correct target for the archive) will still be on the xvec at the point
that target is chosen for the archive.  However, target xvecs are
read-only.  Thus the need for per_xvec_warn to logically extend
bfd_target with a writable field.  I've made per_xvec_warn one larger
than bfd_target_vector to provide one place for user code that makes
private copies of target xvecs.

	* elfcode.h (elf_swap_shdr_in, elf_object_p): Stash potential
	warnings in _bfd_per_xvec_warn location.
	* format.c (clear_warnmsg): New function.
	(bfd_check_format_matches): Call clear_warnmsg before trying
	a new xvec.  Print warnings for the successful non-archive
	match.
	* targets.c: Include libiberty.h.
	(_bfd_target_vector_entries): Use ARRAY_SIZE.
	(per_xvec_warn): New.
	(_bfd_per_xvec_warn): New function.
	* Makefile.am (LIBBFD_H_FILES): Add targets.c.
	* Makefile.in: Regenerate.
	* libbfd.h: Regenerate.

diff --git a/bfd/Makefile.am b/bfd/Makefile.am
index c23dff6cac3..a5652b2a9c7 100644
--- a/bfd/Makefile.am
+++ b/bfd/Makefile.am
@@ -927,7 +927,7 @@ BFD_H_FILES = bfd-in.h init.c opncls.c libbfd.c \
 	linker.c simple.c compress.c
 BFD64_H_FILES = archive64.c
 LIBBFD_H_FILES = libbfd-in.h libbfd.c bfdio.c bfdwin.c \
-	cache.c reloc.c archures.c linker.c
+	cache.c reloc.c targets.c archures.c linker.c
 LIBCOFF_H_FILES = libcoff-in.h coffcode.h
 
 headers: stmp-bin2-h stmp-lbfd-h stmp-lcoff-h
diff --git a/bfd/Makefile.in b/bfd/Makefile.in
index 82843d2d61d..83d686529a0 100644
--- a/bfd/Makefile.in
+++ b/bfd/Makefile.in
@@ -1214,7 +1214,7 @@ BFD_H_FILES = bfd-in.h init.c opncls.c libbfd.c \
 
 BFD64_H_FILES = archive64.c
 LIBBFD_H_FILES = libbfd-in.h libbfd.c bfdio.c bfdwin.c \
-	cache.c reloc.c archures.c linker.c
+	cache.c reloc.c targets.c archures.c linker.c
 
 LIBCOFF_H_FILES = libcoff-in.h coffcode.h
 
diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 39c84b4d0fd..1392240f481 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -323,11 +323,11 @@ elf_swap_shdr_in (bfd *abfd,
 
       if (filesize != 0
 	  && ((ufile_ptr) dst->sh_offset > filesize
-	      || dst->sh_size > filesize - dst->sh_offset))
+	      || dst->sh_size > filesize - dst->sh_offset)
+	  && !abfd->read_only)
 	{
-	  if (!abfd->read_only)
-	    _bfd_error_handler (_("warning: %pB has a section "
-				  "extending past end of file"), abfd);
+	  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
+	  *warn = _("warning: %pB has a section extending past end of file");
 	  abfd->read_only = 1;
 	}
     }
@@ -771,10 +771,12 @@ elf_object_p (bfd *abfd)
 	     So we are kind, and reset the string index value to 0
 	     so that at least some processing can be done.  */
 	  i_ehdrp->e_shstrndx = SHN_UNDEF;
-	  abfd->read_only = 1;
-	  _bfd_error_handler
-	    (_("warning: %pB has a corrupt string table index - ignoring"),
-	     abfd);
+	  if (!abfd->read_only)
+	    {
+	      const char **warn = _bfd_per_xvec_warn (abfd->xvec);
+	      *warn = _("warning: %pB has a corrupt string table index");
+	      abfd->read_only = 1;
+	    }
 	}
     }
   else if (i_ehdrp->e_shstrndx != SHN_UNDEF)
@@ -816,9 +818,14 @@ elf_object_p (bfd *abfd)
 	     two, as required by the ELF spec.  */
 	  if (i_phdr->p_align != (i_phdr->p_align & -i_phdr->p_align))
 	    {
-	      abfd->read_only = 1;
-	      _bfd_error_handler (_("warning: %pB has a program header "
-				    "with invalid alignment"), abfd);
+	      i_phdr->p_align &= -i_phdr->p_align;
+	      if (!abfd->read_only)
+		{
+		  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
+		  *warn = _("warning: %pB has a program header "
+			    "with invalid alignment");
+		  abfd->read_only = 1;
+		}
 	    }
 	}
     }
diff --git a/bfd/format.c b/bfd/format.c
index 489ffcffd53..7e2813c97a4 100644
--- a/bfd/format.c
+++ b/bfd/format.c
@@ -202,6 +202,13 @@ bfd_preserve_finish (bfd *abfd ATTRIBUTE_UNUSED, struct bfd_preserve *preserve)
   preserve->marker = NULL;
 }
 
+static void
+clear_warnmsg (const bfd_target *targ)
+{
+  const char **warn = _bfd_per_xvec_warn (targ);
+  *warn = NULL;
+}
+
 /*
 FUNCTION
 	bfd_check_format_matches
@@ -275,6 +282,7 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
       if (bfd_seek (abfd, (file_ptr) 0, SEEK_SET) != 0)	/* rewind! */
 	goto err_ret;
 
+      clear_warnmsg (abfd->xvec);
       cleanup = BFD_SEND_FMT (abfd, _bfd_check_format, (abfd));
 
       if (cleanup)
@@ -341,6 +349,7 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
       if (bfd_seek (abfd, (file_ptr) 0, SEEK_SET) != 0)
 	goto err_ret;
 
+      clear_warnmsg (abfd->xvec);
       cleanup = BFD_SEND_FMT (abfd, _bfd_check_format, (abfd));
       if (cleanup)
 	{
@@ -506,6 +515,13 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
 	bfd_preserve_finish (abfd, &preserve_match);
       bfd_preserve_finish (abfd, &preserve);
 
+      if (!abfd->my_archive)
+	{
+	  const char **warn = _bfd_per_xvec_warn (abfd->xvec);
+	  if (*warn)
+	    _bfd_error_handler (*warn, abfd);
+	}
+
       /* File position has moved, BTW.  */
       return true;
     }
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index 68d0c4278b3..a17b98e8e30 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -1,6 +1,6 @@
 /* DO NOT EDIT!  -*- buffer-read-only: t -*-  This file is automatically
    generated from "libbfd-in.h", "libbfd.c", "bfdio.c", "bfdwin.c",
-   "cache.c", "reloc.c", "archures.c" and "linker.c".
+   "cache.c", "reloc.c", "targets.c", "archures.c" and "linker.c".
    Run "make headers" in your build bfd/ to regenerate.  */
 
 /* libbfd.h -- Declarations used by bfd library *implementation*.
@@ -3546,6 +3546,9 @@ bool _bfd_unrecognized_reloc
     sec_ptr section,
     unsigned int r_type);
 
+/* Extracted from targets.c.  */
+const char **_bfd_per_xvec_warn (const bfd_target *);
+
 /* Extracted from archures.c.  */
 extern const bfd_arch_info_type bfd_default_arch_struct;
 const bfd_arch_info_type *bfd_default_compatible
diff --git a/bfd/targets.c b/bfd/targets.c
index dc331230aff..7dbc3a5dbf6 100644
--- a/bfd/targets.c
+++ b/bfd/targets.c
@@ -20,6 +20,7 @@
    MA 02110-1301, USA.  */
 
 #include "sysdep.h"
+#include "libiberty.h"
 #include "bfd.h"
 #include "libbfd.h"
 #include "fnmatch.h"
@@ -1454,7 +1455,10 @@ const bfd_target *const *const bfd_associated_vector = _bfd_associated_vector;
 /* When there is an ambiguous match, bfd_check_format_matches puts the
    names of the matching targets in an array.  This variable is the maximum
    number of entries that the array could possibly need.  */
-const size_t _bfd_target_vector_entries = sizeof (_bfd_target_vector)/sizeof (*_bfd_target_vector);
+const size_t _bfd_target_vector_entries = ARRAY_SIZE (_bfd_target_vector);
+
+/* A place to stash a warning from _bfd_check_format.  */
+static const char *per_xvec_warn[ARRAY_SIZE (_bfd_target_vector) + 1];
 \f
 /* This array maps configuration triplets onto BFD vectors.  */
 
@@ -1474,6 +1478,29 @@ static const struct targmatch bfd_target_match[] = {
   { NULL, NULL }
 };
 
+/*
+INTERNAL_FUNCTION
+	_bfd_per_xvec_warn
+
+SYNOPSIS
+	const char **_bfd_per_xvec_warn (const bfd_target *);
+
+DESCRIPTION
+	Return a location for the given target xvec to use for
+	warnings specific to that target.
+*/
+
+const char **
+_bfd_per_xvec_warn (const bfd_target *targ)
+{
+  size_t idx;
+
+  for (idx = 0; idx < ARRAY_SIZE (_bfd_target_vector); idx++)
+    if (_bfd_target_vector[idx] == targ)
+      break;
+  return per_xvec_warn + idx;
+}
+
 /* Find a target vector, given a name or configuration triplet.  */
 
 static const bfd_target *

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-12-17  8:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-17  8:12 The problem with warning in elf_object_p Alan Modra
  -- strict thread matches above, loose matches on Subject: below --
2022-09-24  2:08 Alan Modra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).