Re: [PATCH 0/4] Minor cleanups in coff-pe-read.c

[Tom Tromey <tromey@adacore.com>]

Pedro> The series looks fine to me, but now I'm curious on what you mean
Pedro> by "reuse minimal symbols without re-reading".

Minimal symbols are normally stored on the per-BFD object.  So, a savvy
symbol reader can simply reuse them without doing any additional work.
For less savvy symbol readers, the minimal_symbol_reader object itself
knows how to do this -- it simply discards symbols when they are being
re-read.  Only elfread.c was ever converted to do this sharing the best
way:

  /* If we already have minsyms, then we can skip some work here.
     However, if there were stabs or mdebug sections, we go ahead and
     redo all the work anyway, because the psym readers for those
     kinds of debuginfo need extra information found here.  This can
     go away once all types of symbols are in the per-BFD object.  */
  if (objfile->per_bfd->minsyms_read
      && ei->stabsect == NULL
      && ei->mdebugsect == NULL
      && ei->ctfsect == NULL)
    {
      if (symtab_create_debug)
	gdb_printf (gdb_stdlog,
		    "... minimal symbols previously read\n");
      return;
    }

The problem for other readers is that they tend not to be "pure", in the
sense that they mix minimal symbol reading with other kinds of
reading -- and the other kinds may not be shared or even shareable.

That's why the elfread.c change has so many conditions.  For coffread.c,
coff_symfile_read may create full symbols, and it wasn't immediately
obvious to me that this could easily be skipped for PE.

Making everything be shared would be great.  Then we could hoist this
check into generic code and re-using a BFD would be simple.  However,
this is the objfile splitting project, which I've attempted and failed
at several times now.  (It's still a good project but it needs a
strategy other than one giant patch series.)


Now that I look at this again, though, maybe it's easier than I thought.

  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);
    }

Maybe this can be checked earlier.  I'll look at it more.  I had thought
that coff_symtab_read installing minsyms would be a problem, but that
seems to happen in a loop that's checking nsyms.


The symbol readers are all headache-inducing.  For example just glancing
at coffread.c makes me think there are memory leaks:

		sym->type ()->set_name (xstrdup (sym->linkage_name ()));

... type names must be allocated on an obstack.  Here there's probably
just no need to copy at all.  No idea how to test this though.

They also tend to be a mess of global variables and other bad practices.

Tom