[PATCH] Do not double-read minimal symbols for PE COFF

[PATCH] Do not double-read minimal symbols for PE COFF

[Tom Tromey]

This changes coffread.c to avoid re-reading minimal symbols when
possible.  This only works when there are no COFF symbols to be read,
but at least for my mingw builds of gdb, this seems to be the case.

Tested using the AdaCore internal test suite on Windows.  I also did
some local builds to ensure that no warnings crept in.
---
 gdb/coffread.c | 132 ++++++++++++++++++++++++++++---------------------
 1 file changed, 75 insertions(+), 57 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index da871d5af11..b836d417f8f 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -525,6 +525,80 @@ find_linenos (bfd *abfd, struct bfd_section *asect, void *vpinfo)
 }
 
 
+/* A helper function for coff_symfile_read that reads minimal
+   symbols.  It may also read other forms of symbol as well.  */
+
+static void
+coff_read_minsyms (file_ptr symtab_offset, unsigned int nsyms,
+		   struct objfile *objfile)
+
+{
+  /* If minimal symbols were already read, and if we know we aren't
+     going to read any other kind of symbol here, then we can just
+     return.  */
+  if (objfile->per_bfd->minsyms_read && pe_file && nsyms == 0)
+    return;
+
+  minimal_symbol_reader reader (objfile);
+
+  if (pe_file && nsyms == 0)
+    {
+      /* We've got no debugging symbols, but it's a portable
+	 executable, so try to read the export table.  */
+      read_pe_exported_syms (reader, objfile);
+    }
+  else
+    {
+      /* Now that the executable file is positioned at symbol table,
+	 process it and define symbols accordingly.  */
+      coff_symtab_read (reader, symtab_offset, nsyms, objfile);
+    }
+
+  /* Install any minimal symbols that have been collected as the
+     current minimal symbols for this objfile.  */
+
+  reader.install ();
+
+  if (pe_file)
+    {
+      for (minimal_symbol *msym : objfile->msymbols ())
+	{
+	  const char *name = msym->linkage_name ();
+
+	  /* If the minimal symbols whose name are prefixed by "__imp_"
+	     or "_imp_", get rid of the prefix, and search the minimal
+	     symbol in OBJFILE.  Note that 'maintenance print msymbols'
+	     shows that type of these "_imp_XXXX" symbols is mst_data.  */
+	  if (msym->type () == mst_data)
+	    {
+	      const char *name1 = NULL;
+
+	      if (startswith (name, "_imp_"))
+		name1 = name + 5;
+	      else if (startswith (name, "__imp_"))
+		name1 = name + 6;
+	      if (name1 != NULL)
+		{
+		  int lead = bfd_get_symbol_leading_char (objfile->obfd);
+		  struct bound_minimal_symbol found;
+
+		  if (lead != '\0' && *name1 == lead)
+		    name1 += 1;
+
+		  found = lookup_minimal_symbol (name1, NULL, objfile);
+
+		  /* If found, there are symbols named "_imp_foo" and "foo"
+		     respectively in OBJFILE.  Set the type of symbol "foo"
+		     as 'mst_solib_trampoline'.  */
+		  if (found.minsym != NULL
+		      && found.minsym->type () == mst_text)
+		    found.minsym->set_type (mst_solib_trampoline);
+		}
+	    }
+	}
+    }
+}
+
 /* The BFD for this file -- only good while we're actively reading
    symbols into a psymtab or a symtab.  */
 
@@ -625,56 +699,7 @@ coff_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
   if (val < 0)
     error (_("\"%s\": can't get string table"), filename);
 
-  minimal_symbol_reader reader (objfile);
-
-  /* Now that the executable file is positioned at symbol table,
-     process it and define symbols accordingly.  */
-
-  coff_symtab_read (reader, (long) symtab_offset, num_symbols, objfile);
-
-  /* Install any minimal symbols that have been collected as the
-     current minimal symbols for this objfile.  */
-
-  reader.install ();
-
-  if (pe_file)
-    {
-      for (minimal_symbol *msym : objfile->msymbols ())
-	{
-	  const char *name = msym->linkage_name ();
-
-	  /* If the minimal symbols whose name are prefixed by "__imp_"
-	     or "_imp_", get rid of the prefix, and search the minimal
-	     symbol in OBJFILE.  Note that 'maintenance print msymbols'
-	     shows that type of these "_imp_XXXX" symbols is mst_data.  */
-	  if (msym->type () == mst_data)
-	    {
-	      const char *name1 = NULL;
-
-	      if (startswith (name, "_imp_"))
-		name1 = name + 5;
-	      else if (startswith (name, "__imp_"))
-		name1 = name + 6;
-	      if (name1 != NULL)
-		{
-		  int lead = bfd_get_symbol_leading_char (objfile->obfd);
-		  struct bound_minimal_symbol found;
-
-		  if (lead != '\0' && *name1 == lead)
-		    name1 += 1;
-
-		  found = lookup_minimal_symbol (name1, NULL, objfile);
-
-		  /* If found, there are symbols named "_imp_foo" and "foo"
-		     respectively in OBJFILE.  Set the type of symbol "foo"
-		     as 'mst_solib_trampoline'.  */
-		  if (found.minsym != NULL
-		      && found.minsym->type () == mst_text)
-		    found.minsym->set_type (mst_solib_trampoline);
-		}
-	    }
-	}
-    }
+  coff_read_minsyms (symtab_offset, num_symbols, objfile);
 
   if (!(objfile->flags & OBJF_READNEVER))
     bfd_map_over_sections (abfd, coff_locate_sections, (void *) info);
@@ -1160,13 +1185,6 @@ coff_symtab_read (minimal_symbol_reader &reader,
 	}
     }
 
-  if ((nsyms == 0) && (pe_file))
-    {
-      /* We've got no debugging symbols, but it's a portable
-	 executable, so try to read the export table.  */
-      read_pe_exported_syms (reader, objfile);
-    }
-
   if (get_last_source_file ())
     coff_end_compunit_symtab (objfile);
 
-- 
2.34.1

Re: [PATCH] Do not double-read minimal symbols for PE COFF

[Eli Zaretskii]

> Date: Fri, 22 Apr 2022 11:40:14 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Tom Tromey <tromey@adacore.com>
> 
> This changes coffread.c to avoid re-reading minimal symbols when
> possible.  This only works when there are no COFF symbols to be read,
> but at least for my mingw builds of gdb, this seems to be the case.

coffread.c is also used bu go32 (a.k.a. DJGPP) build of GDB.  What
does this change mean for programs compiled with COFF debug info?

Re: [PATCH] Do not double-read minimal symbols for PE COFF

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Fri, 22 Apr 2022 11:40:14 -0600
>> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Tom Tromey <tromey@adacore.com>
>> 
>> This changes coffread.c to avoid re-reading minimal symbols when
>> possible.  This only works when there are no COFF symbols to be read,
>> but at least for my mingw builds of gdb, this seems to be the case.

Eli> coffread.c is also used bu go32 (a.k.a. DJGPP) build of GDB.  What
Eli> does this change mean for programs compiled with COFF debug info?

It shouldn't change anything.  It's just an optimization.

Tom

Re: [PATCH] Do not double-read minimal symbols for PE COFF

[Tom Tromey]

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> This changes coffread.c to avoid re-reading minimal symbols when
Tom> possible.  This only works when there are no COFF symbols to be read,
Tom> but at least for my mingw builds of gdb, this seems to be the case.

Tom> Tested using the AdaCore internal test suite on Windows.  I also did
Tom> some local builds to ensure that no warnings crept in.

I'm checking this in.

Tom