public inbox for binutils-cvs@sourceware.org
 help / color / mirror / Atom feed
* [binutils-gdb] COFF swap_aux_in
@ 2023-08-28 13:43 Alan Modra
  0 siblings, 0 replies; only message in thread
From: Alan Modra @ 2023-08-28 13:43 UTC (permalink / raw)
  To: bfd-cvs

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=daafebb58dac3de93ac4696dd334530b762ed67f

commit daafebb58dac3de93ac4696dd334530b762ed67f
Author: Alan Modra <amodra@gmail.com>
Date:   Mon Aug 28 21:07:52 2023 +0930

    COFF swap_aux_in
    
    A low level function like coff_swap_aux_in really has no business
    concatenating multiple auxents for the old PE multi-aux scheme of
    handling long file names.  In doing so, it assumes multiple internal
    auxent buffers are available, which they are not in most calls to
    bfd_coff_swap_aux_in, both inside BFD and outside, eg. GDB.  Buffer
    overflow fun.  Concatenating multiple auxents belongs at a higher
    level.
    
    This required some changes to coff_get_normalized_symtab, which now
    uses the external auxents to access the concatenated file name.
    (Internal auxents are larger than the x_fname array, so the pieces of
    the file name are not adjacent as they are in the external auxents.)
    
            * coffswap.h (coff_swap_aux_in): Do not write more than one
            internal auxent.
            * coffcode.h (coff_bigobj_swap_aux_in): Likewise.
            * coffgen.c (coff_get_normalized_symtab): Normalize strings
            after swapping in each symbol so that external auxents are
            available.  Use external auxents for multi-aux long file
            names.  Formatting.  Wrap long lines.  Remove excess parens
            and unnecessary casts.  Don't zalloc when only the string
            terminator needs zeroing, and memcpy rather than strncpy.
            Delete unnecessary sanity check with unsigned _n_offset.
            Return with failure if debug section can't be read, to avoid
            trying to read it multiple times.  Correct sanity check
            against debug section size.

Diff:
---
 bfd/coffcode.h |  13 ++---
 bfd/coffgen.c  | 152 +++++++++++++++++++++++++++------------------------------
 bfd/coffswap.h |  13 ++---
 3 files changed, 77 insertions(+), 101 deletions(-)

diff --git a/bfd/coffcode.h b/bfd/coffcode.h
index 908dc93c64a..0487555dda1 100644
--- a/bfd/coffcode.h
+++ b/bfd/coffcode.h
@@ -5813,8 +5813,8 @@ coff_bigobj_swap_aux_in (bfd *abfd,
 			 void * ext1,
 			 int type,
 			 int in_class,
-			 int indx,
-			 int numaux,
+			 int indx ATTRIBUTE_UNUSED,
+			 int numaux ATTRIBUTE_UNUSED,
 			 void * in1)
 {
   AUXENT_BIGOBJ *ext = (AUXENT_BIGOBJ *) ext1;
@@ -5826,14 +5826,7 @@ coff_bigobj_swap_aux_in (bfd *abfd,
   switch (in_class)
     {
     case C_FILE:
-      if (numaux > 1)
-	{
-	  if (indx == 0)
-	    memcpy (in->x_file.x_n.x_fname, ext->File.Name,
-		    numaux * sizeof (AUXENT_BIGOBJ));
-	}
-      else
-	memcpy (in->x_file.x_n.x_fname, ext->File.Name, sizeof (ext->File.Name));
+      memcpy (in->x_file.x_n.x_fname, ext->File.Name, sizeof (ext->File.Name));
       break;
 
     case C_STAT:
diff --git a/bfd/coffgen.c b/bfd/coffgen.c
index 91667267cbc..c1aacc707b5 100644
--- a/bfd/coffgen.c
+++ b/bfd/coffgen.c
@@ -1843,8 +1843,6 @@ coff_get_normalized_symtab (bfd *abfd)
 {
   combined_entry_type *internal;
   combined_entry_type *internal_ptr;
-  combined_entry_type *symbol_ptr;
-  combined_entry_type *internal_end;
   size_t symesz;
   char *raw_src;
   char *raw_end;
@@ -1867,7 +1865,6 @@ coff_get_normalized_symtab (bfd *abfd)
   internal = (combined_entry_type *) bfd_zalloc (abfd, size);
   if (internal == NULL && size != 0)
     return NULL;
-  internal_end = internal + obj_raw_syment_count (abfd);
 
   raw_src = (char *) obj_coff_external_syms (abfd);
 
@@ -1887,48 +1884,32 @@ coff_get_normalized_symtab (bfd *abfd)
 
       bfd_coff_swap_sym_in (abfd, (void *) raw_src,
 			    (void *) & internal_ptr->u.syment);
-      symbol_ptr = internal_ptr;
       internal_ptr->is_sym = true;
+      combined_entry_type *sym = internal_ptr;
 
       /* PR 17512: Prevent buffer overrun.  */
-      if (symbol_ptr->u.syment.n_numaux > ((raw_end - 1) - raw_src) / symesz)
+      if (sym->u.syment.n_numaux > ((raw_end - 1) - raw_src) / symesz)
 	return NULL;
 
-      for (i = 0;
-	   i < symbol_ptr->u.syment.n_numaux;
-	   i++)
+      for (i = 0; i < sym->u.syment.n_numaux; i++)
 	{
 	  internal_ptr++;
 	  raw_src += symesz;
 
 	  bfd_coff_swap_aux_in (abfd, (void *) raw_src,
-				symbol_ptr->u.syment.n_type,
-				symbol_ptr->u.syment.n_sclass,
-				(int) i, symbol_ptr->u.syment.n_numaux,
+				sym->u.syment.n_type,
+				sym->u.syment.n_sclass,
+				(int) i, sym->u.syment.n_numaux,
 				&(internal_ptr->u.auxent));
 
 	  internal_ptr->is_sym = false;
-	  coff_pointerize_aux (abfd, internal, symbol_ptr, i, internal_ptr);
+	  coff_pointerize_aux (abfd, internal, sym, i, internal_ptr);
 	}
-    }
 
-  /* Free the raw symbols.  */
-  if (obj_coff_external_syms (abfd) != NULL
-      && ! obj_coff_keep_syms (abfd))
-    {
-      free (obj_coff_external_syms (abfd));
-      obj_coff_external_syms (abfd) = NULL;
-    }
-
-  for (internal_ptr = internal; internal_ptr < internal_end;
-       internal_ptr++)
-    {
-      BFD_ASSERT (internal_ptr->is_sym);
-
-      if (internal_ptr->u.syment.n_sclass == C_FILE
-	  && internal_ptr->u.syment.n_numaux > 0)
+      if (sym->u.syment.n_sclass == C_FILE
+	  && sym->u.syment.n_numaux > 0)
 	{
-	  combined_entry_type * aux = internal_ptr + 1;
+	  combined_entry_type * aux = sym + 1;
 
 	  /* Make a file symbol point to the name in the auxent, since
 	     the text ".file" is redundant.  */
@@ -1944,12 +1925,12 @@ coff_get_normalized_symtab (bfd *abfd)
 		    return NULL;
 		}
 
-	      if ((bfd_size_type)(aux->u.auxent.x_file.x_n.x_n.x_offset)
+	      if ((bfd_size_type) aux->u.auxent.x_file.x_n.x_n.x_offset
 		  >= obj_coff_strings_len (abfd))
-		internal_ptr->u.syment._n._n_n._n_offset =
+		sym->u.syment._n._n_n._n_offset =
 		  (uintptr_t) _("<corrupt>");
 	      else
-		internal_ptr->u.syment._n._n_n._n_offset =
+		sym->u.syment._n._n_n._n_offset =
 		  (uintptr_t) (string_table
 			       + aux->u.auxent.x_file.x_n.x_n.x_offset);
 	    }
@@ -1958,30 +1939,35 @@ coff_get_normalized_symtab (bfd *abfd)
 	      /* Ordinary short filename, put into memory anyway.  The
 		 Microsoft PE tools sometimes store a filename in
 		 multiple AUX entries.  */
-	      if (internal_ptr->u.syment.n_numaux > 1 && obj_pe (abfd))
-		internal_ptr->u.syment._n._n_n._n_offset =
-		  ((uintptr_t)
-		   copy_name (abfd,
-			      aux->u.auxent.x_file.x_n.x_fname,
-			      internal_ptr->u.syment.n_numaux * symesz));
+	      size_t len;
+	      char *src;
+	      if (sym->u.syment.n_numaux > 1 && obj_pe (abfd))
+		{
+		  len = sym->u.syment.n_numaux * symesz;
+		  src = raw_src - (len - symesz);
+		}
 	      else
-		internal_ptr->u.syment._n._n_n._n_offset =
-		  ((uintptr_t)
-		   copy_name (abfd,
-			      aux->u.auxent.x_file.x_n.x_fname,
-			      (size_t) bfd_coff_filnmlen (abfd)));
+		{
+		  len = bfd_coff_filnmlen (abfd);
+		  src = aux->u.auxent.x_file.x_n.x_fname;
+		}
+	      sym->u.syment._n._n_n._n_offset =
+		(uintptr_t) copy_name (abfd, src, len);
 	    }
 
 	  /* Normalize other strings available in C_FILE aux entries.  */
 	  if (!obj_pe (abfd))
-	    for (int numaux = 1; numaux < internal_ptr->u.syment.n_numaux; numaux++)
+	    for (int numaux = 1;
+		 numaux < sym->u.syment.n_numaux;
+		 numaux++)
 	      {
-		aux = internal_ptr + numaux + 1;
+		aux = sym + numaux + 1;
 		BFD_ASSERT (! aux->is_sym);
 
 		if (aux->u.auxent.x_file.x_n.x_n.x_zeroes == 0)
 		  {
-		    /* The string information is a long one, point into the string table.  */
+		    /* The string information is a long one, point
+		       into the string table.  */
 		    if (string_table == NULL)
 		      {
 			string_table = _bfd_coff_read_string_table (abfd);
@@ -1989,48 +1975,48 @@ coff_get_normalized_symtab (bfd *abfd)
 			  return NULL;
 		      }
 
-		    if ((bfd_size_type)(aux->u.auxent.x_file.x_n.x_n.x_offset)
+		    if ((bfd_size_type) aux->u.auxent.x_file.x_n.x_n.x_offset
 			>= obj_coff_strings_len (abfd))
 		      aux->u.auxent.x_file.x_n.x_n.x_offset =
 			(uintptr_t) _("<corrupt>");
 		    else
 		      aux->u.auxent.x_file.x_n.x_n.x_offset =
 			(uintptr_t) (string_table
-				     + (aux->u.auxent.x_file.x_n.x_n.x_offset));
+				     + aux->u.auxent.x_file.x_n.x_n.x_offset);
 		  }
 		else
 		  aux->u.auxent.x_file.x_n.x_n.x_offset =
 		    ((uintptr_t)
 		     copy_name (abfd,
 				aux->u.auxent.x_file.x_n.x_fname,
-				(size_t) bfd_coff_filnmlen (abfd)));
+				bfd_coff_filnmlen (abfd)));
 	      }
 
 	}
       else
 	{
-	  if (internal_ptr->u.syment._n._n_n._n_zeroes != 0)
+	  if (sym->u.syment._n._n_n._n_zeroes != 0)
 	    {
 	      /* This is a "short" name.  Make it long.  */
-	      size_t i;
 	      char *newstring;
 
 	      /* Find the length of this string without walking into memory
 		 that isn't ours.  */
-	      for (i = 0; i < 8; ++i)
-		if (internal_ptr->u.syment._n._n_name[i] == '\0')
+	      for (i = 0; i < SYMNMLEN; ++i)
+		if (sym->u.syment._n._n_name[i] == '\0')
 		  break;
 
-	      newstring = (char *) bfd_zalloc (abfd, (bfd_size_type) (i + 1));
+	      newstring = bfd_alloc (abfd, i + 1);
 	      if (newstring == NULL)
 		return NULL;
-	      strncpy (newstring, internal_ptr->u.syment._n._n_name, i);
-	      internal_ptr->u.syment._n._n_n._n_offset = (uintptr_t) newstring;
-	      internal_ptr->u.syment._n._n_n._n_zeroes = 0;
+	      memcpy (newstring, sym->u.syment._n._n_name, i);
+	      newstring[i] = 0;
+	      sym->u.syment._n._n_n._n_offset = (uintptr_t) newstring;
+	      sym->u.syment._n._n_n._n_zeroes = 0;
 	    }
-	  else if (internal_ptr->u.syment._n._n_n._n_offset == 0)
-	    internal_ptr->u.syment._n._n_n._n_offset = (uintptr_t) "";
-	  else if (!bfd_coff_symname_in_debug (abfd, &internal_ptr->u.syment))
+	  else if (sym->u.syment._n._n_n._n_offset == 0)
+	    sym->u.syment._n._n_n._n_offset = (uintptr_t) "";
+	  else if (!bfd_coff_symname_in_debug (abfd, &sym->u.syment))
 	    {
 	      /* Long name already.  Point symbol at the string in the
 		 table.  */
@@ -2040,43 +2026,47 @@ coff_get_normalized_symtab (bfd *abfd)
 		  if (string_table == NULL)
 		    return NULL;
 		}
-	      if (internal_ptr->u.syment._n._n_n._n_offset >= obj_coff_strings_len (abfd)
-		  || string_table + internal_ptr->u.syment._n._n_n._n_offset < string_table)
-		internal_ptr->u.syment._n._n_n._n_offset =
+	      if (sym->u.syment._n._n_n._n_offset >= obj_coff_strings_len (abfd))
+		sym->u.syment._n._n_n._n_offset =
 		  (uintptr_t) _("<corrupt>");
 	      else
-		internal_ptr->u.syment._n._n_n._n_offset =
-		  ((uintptr_t) (string_table
-				+ internal_ptr->u.syment._n._n_n._n_offset));
+		sym->u.syment._n._n_n._n_offset =
+		  (uintptr_t) (string_table
+			       + sym->u.syment._n._n_n._n_offset);
 	    }
 	  else
 	    {
 	      /* Long name in debug section.  Very similar.  */
 	      if (debug_sec_data == NULL)
-		debug_sec_data = build_debug_section (abfd, & debug_sec);
-	      if (debug_sec_data != NULL)
 		{
-		  BFD_ASSERT (debug_sec != NULL);
-		  /* PR binutils/17512: Catch out of range offsets into the debug data.  */
-		  if (internal_ptr->u.syment._n._n_n._n_offset > debug_sec->size
-		      || debug_sec_data + internal_ptr->u.syment._n._n_n._n_offset < debug_sec_data)
-		    internal_ptr->u.syment._n._n_n._n_offset =
-		      (uintptr_t) _("<corrupt>");
-		  else
-		    internal_ptr->u.syment._n._n_n._n_offset =
-		      (uintptr_t) (debug_sec_data
-				   + internal_ptr->u.syment._n._n_n._n_offset);
+		  debug_sec_data = build_debug_section (abfd, &debug_sec);
+		  if (debug_sec_data == NULL)
+		    return NULL;
 		}
+	      /* PR binutils/17512: Catch out of range offsets into
+		 the debug data.  */
+	      if (sym->u.syment._n._n_n._n_offset >= debug_sec->size)
+		sym->u.syment._n._n_n._n_offset =
+		  (uintptr_t) _("<corrupt>");
 	      else
-		internal_ptr->u.syment._n._n_n._n_offset = (uintptr_t) "";
+		sym->u.syment._n._n_n._n_offset =
+		  (uintptr_t) (debug_sec_data
+			       + sym->u.syment._n._n_n._n_offset);
 	    }
 	}
-      internal_ptr += internal_ptr->u.syment.n_numaux;
+    }
+
+  /* Free the raw symbols.  */
+  if (obj_coff_external_syms (abfd) != NULL
+      && ! obj_coff_keep_syms (abfd))
+    {
+      free (obj_coff_external_syms (abfd));
+      obj_coff_external_syms (abfd) = NULL;
     }
 
   obj_raw_syments (abfd) = internal;
   BFD_ASSERT (obj_raw_syment_count (abfd)
-	      == (unsigned int) (internal_ptr - internal));
+	      == (size_t) (internal_ptr - internal));
 
   return internal;
 }
diff --git a/bfd/coffswap.h b/bfd/coffswap.h
index 190b8f02a0b..f0677ff8857 100644
--- a/bfd/coffswap.h
+++ b/bfd/coffswap.h
@@ -402,8 +402,8 @@ coff_swap_aux_in (bfd *abfd,
 		  void * ext1,
 		  int type,
 		  int in_class,
-		  int indx,
-		  int numaux,
+		  int indx ATTRIBUTE_UNUSED,
+		  int numaux ATTRIBUTE_UNUSED,
 		  void * in1)
 {
   AUXENT *ext = (AUXENT *) ext1;
@@ -426,14 +426,7 @@ coff_swap_aux_in (bfd *abfd,
 #if FILNMLEN != E_FILNMLEN
 #error we need to cope with truncating or extending FILNMLEN
 #else
-	  if (numaux > 1 && obj_pe (abfd))
-	    {
-	      if (indx == 0)
-		memcpy (in->x_file.x_n.x_fname, ext->x_file.x_fname,
-			numaux * sizeof (AUXENT));
-	    }
-	  else
-	    memcpy (in->x_file.x_n.x_fname, ext->x_file.x_fname, FILNMLEN);
+	  memcpy (in->x_file.x_n.x_fname, ext->x_file.x_fname, FILNMLEN);
 #endif
 	}
       goto end;

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-08-28 13:43 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-28 13:43 [binutils-gdb] COFF swap_aux_in 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).