libctf, include: find types of symbols by name

The existing ctf_lookup_by_symbol and ctf_arc_lookup_symbol functions
suffice to look up the types of symbols if the caller already has a
symbol number.  But the caller often doesn't have one of those and only
knows the name of the symbol: also, in object files, the caller might
not have a useful symbol number in any sense (and neither does libctf:
the 'symbol number' we use in that case literally starts at 0 for the
lexicographically first-sorted symbol in the symtypetab and counts those
symbols, so it corresponds to nothing useful).

This means that even though object files have a symtypetab (generated by
the compiler or by ld -r), the only way we can look up anything in it is
to iterate over all symbols in turn with ctf_symbol_next until we find
the one we want.

This is unhelpful and pointlessly inefficient.

So add a pair of functions to look up symbols by name in a dict and in a
whole archive: ctf_lookup_by_symbol_name and ctf_arc_lookup_symbol_name.
These are identical to the existing functions except that they take
symbol names rather than symbol numbers.

To avoid insane repetition, we do some refactoring in the process, so
that both ctf_lookup_by_symbol and ctf_arc_lookup_symbol turn into thin
wrappers around internal functions that do both lookup by symbol index
and lookup by name.  This massively reduces code duplication because
even the existing lookup-by-index stuff wants to use a name sometimes
(when looking up in indexed sections), and the new lookup-by-name stuff
has to turn it into an index sometimes (when looking up in non-indexed
sections): doing it this way lets us share most of that.

The actual name->index lookup is done by ctf_lookup_symbol_idx.  We do
not anticipate this lookup to be as heavily used as ld.so symbol lookup
by many orders of magnitude, so using the ELF symbol hashes would
probably take more time to read them than is saved by using the hashes,
and it adds a lot of complexity.  Instead, do a linear search for the
symbol name, caching all the name -> index mappings as we go, so that
future searches are likely to hit in the cache.  To avoid having to
repeat this search over and over in a CTF archive when
ctf_arc_lookup_symbol_name is used, have cached archive lookups (the
sort done by ctf_arc_lookup_symbol* and the ctf_archive_next iterator)
pick out the first dict they cache in a given archive and store it in a
new ctf_archive field, ctfi_crossdict_cache.  This can be used to store
cross-dictionary cached state that depends on things like the ELF symbol
table rather than the contents of any one dict.  ctf_lookup_symbol_idx
then caches its name->index mappings in the dictionary named in the
crossdict cache, if any, so that ctf_lookup_symbol_idx in other dicts
in the same archive benefit from the previous linear search, and the
symtab only needs to be scanned at most once.

(Note that if you call ctf_lookup_by_symbol_name in one specific dict,
and then follow it with a ctf_arc_lookup_symbol_name, the former will
not use the crossdict cache because it's only populated by the dict
opens in ctf_arc_lookup_symbol_name. This is harmless except for a small
one-off waste of memory and time: it's only a cache, after all.  We can
fix this later by using the archive caching machinery more
aggressively.)

In ctf-archive, we do similar things, turning ctf_arc_lookup_symbol into
a wrapper around a new function that does both index -> ID and name ->
ID lookups across all dicts in an archive.  We add a new
ctfi_symnamedicts cache that maps symbol names to the ctf_dict_t * that
it was found in (so that linear searches for symbols don't need to be
repeated): but we also *remove* a cache, the ctfi_syms cache that was
memoizing the actual ctf_id_t returned from every call to
ctf_arc_lookup_symbol.  This is pointless: all it saves is one call to
ctf_lookup_by_symbol, and that's basically an array lookup and nothing
more so isn't worth caching.  (Equally, given that symbol -> index
mappings are cached by ctf_lookup_by_symbol_name, those calls are nearly
free after the first call, so there's no point caching the ctf_id_t in
that case either.)

We fix up one test that was doing manual symbol lookup to use
ctf_arc_lookup_symbol instead, and enhance it to check that the caching
layer is not totally broken: we also add a new test to do lookups in a
.o file, and another to do lookups in an archive with conflicted types
and make sure that sort of multi-dict lookup is actually working.

include/ChangeLog
2021-02-17  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-api.h (ctf_arc_lookup_symbol_name): New.
	(ctf_lookup_by_symbol_name): Likewise.

libctf/ChangeLog
2021-02-17  Nick Alcock  <nick.alcock@oracle.com>

	* ctf-impl.h (ctf_dict_t) <ctf_symhash>: New.
	<ctf_symhash_latest>: Likewise.
	(struct ctf_archive_internal) <ctfi_crossdict_cache>: New.
	<ctfi_symnamedicts>: New.
	<ctfi_syms>: Remove.
	(ctf_lookup_symbol_name): Remove.
	* ctf-lookup.c (ctf_lookup_symbol_name): Propagate errors from
	parent properly.  Make static.
	(ctf_lookup_symbol_idx): New, linear search for the symbol name,
	cached in the crossdict cache's ctf_symhash (if available), or
	this dict's (otherwise).
	(ctf_try_lookup_indexed): Allow the symname to be passed in.
	(ctf_lookup_by_symbol): Turn into a wrapper around...
	(ctf_lookup_by_sym_or_name): ... this, supporting name lookup too,
	using ctf_lookup_symbol_idx in non-writable dicts.  Special-case
	name lookup in dynamic dicts without reported symbols, which have
	no symtab or dynsymidx but where name lookup should still work.
	(ctf_lookup_by_symbol_name): New, another wrapper.
	* ctf-archive.c (enosym): Note that this is present in
	ctfi_symnamedicts too.
	(ctf_arc_close): Adjust for removal of ctfi_syms.  Free the
	ctfi_symnamedicts.
	(ctf_arc_flush_caches): Likewise.
	(ctf_dict_open_cached): Memoize the first cached dict in the
	crossdict cache.
	(ctf_arc_lookup_symbol): Turn into a wrapper around...
	(ctf_arc_lookup_sym_or_name): ... this.  No longer cache
	ctf_id_t lookups: just call ctf_lookup_by_symbol as needed (but
	still cache the dicts those lookups succeed in).  Add
	lookup-by-name support, with dicts of successful lookups cached in
	ctfi_symnamedicts.  Refactor the caching code a bit.
	(ctf_arc_lookup_symbol_name): New, another wrapper.
	* ctf-open.c (ctf_dict_close): Free the ctf_symhash.
	* libctf.ver (LIBCTF_1.2): New version.  Add
	ctf_lookup_by_symbol_name, ctf_arc_lookup_symbol_name.
	* testsuite/libctf-lookup/enum-symbol.c (main): Use
	ctf_arc_lookup_symbol rather than looking up the name ourselves.
	Fish it out repeatedly, to make sure that symbol caching isn't
	broken.
	(symidx_64): Remove.
	(symidx_32): Remove.
	* testsuite/libctf-lookup/enum-symbol-obj.lk: Test symbol lookup
	in an unlinked object file (indexed symtypetab sections only).
	* testsuite/libctf-writable/symtypetab-nonlinker-writeout.c
	(try_maybe_reporting): Check symbol types via
	ctf_lookup_by_symbol_name as well as ctf_symbol_next.
	* testsuite/libctf-lookup/conflicting-type-syms.*: New test of
	lookups in a multi-dict archive.

1 files changed, 134 insertions, 46 deletions

diff --git a/libctf/ctf-archive.c b/libctf/ctf-archive.c
index 193fc4d..6d9c75c 100644
--- a/libctf/ctf-archive.c
+++ b/libctf/ctf-archive.c
@@ -46,8 +46,8 @@ static int arc_mmap_writeout (int fd, void *header, size_t headersz,
 static int arc_mmap_unmap (void *header, size_t headersz, const char **errmsg);
 static void ctf_arc_import_parent (const ctf_archive_t *arc, ctf_dict_t *fp);
 
-/* Flag to indicate "symbol not present" in
-   ctf_archive_internal.ctfi_symdicts.  Never initialized.  */
+/* Flag to indicate "symbol not present" in ctf_archive_internal.ctfi_symdicts
+   and ctfi_symnamedicts.  Never initialized.  */
 static ctf_dict_t enosym;
 
 /* Write out a CTF archive to the start of the file referenced by the passed-in
@@ -529,8 +529,8 @@ ctf_arc_close (ctf_archive_t *arc)
     }
   else
     ctf_dict_close (arc->ctfi_dict);
-  free (arc->ctfi_syms);
   free (arc->ctfi_symdicts);
+  free (arc->ctfi_symnamedicts);
   ctf_dynhash_destroy (arc->ctfi_dicts);
   if (arc->ctfi_free_symsect)
     free ((void *) arc->ctfi_symsect.cts_data);
@@ -645,8 +645,9 @@ ctf_cached_dict_close (void *fp)
   ctf_dict_close ((ctf_dict_t *) fp);
 }
 
-/* Return the ctf_dict_t with the given name and cache it in the
-   archive's ctfi_dicts.  */
+/* Return the ctf_dict_t with the given name and cache it in the archive's
+   ctfi_dicts.  If this is the first cached dict, designate it the
+   crossdict_cache.  */
 static ctf_dict_t *
 ctf_dict_open_cached (ctf_archive_t *arc, const char *name, int *errp)
 {
@@ -678,6 +679,9 @@ ctf_dict_open_cached (ctf_archive_t *arc, const char *name, int *errp)
     goto oom;
   fp->ctf_refcnt++;
 
+  if (arc->ctfi_crossdict_cache == NULL)
+    arc->ctfi_crossdict_cache = fp;
+
   return fp;
 
  oom:
@@ -693,11 +697,12 @@ void
 ctf_arc_flush_caches (ctf_archive_t *wrapper)
 {
   free (wrapper->ctfi_symdicts);
-  free (wrapper->ctfi_syms);
+  free (wrapper->ctfi_symnamedicts);
   ctf_dynhash_destroy (wrapper->ctfi_dicts);
   wrapper->ctfi_symdicts = NULL;
-  wrapper->ctfi_syms = NULL;
+  wrapper->ctfi_symnamedicts = NULL;
   wrapper->ctfi_dicts = NULL;
+  wrapper->ctfi_crossdict_cache = NULL;
 }
 
 /* Return the ctf_dict_t at the given ctfa_ctfs-relative offset, or NULL if
@@ -778,31 +783,46 @@ ctf_archive_count (const ctf_archive_t *wrapper)
   return wrapper->ctfi_archive->ctfa_ndicts;
 }
 
-/* Look up a symbol in an archive.  Return the dict in the archive that the
-   symbol is found in, and (optionally) the ctf_id_t of the symbol in that dict
-   (so you don't have to look it up yourself).  The dict and mapping are both
-   cached, so repeated lookups are nearly free.
+/* Look up a symbol in an archive by name or index (if the name is set, a lookup
+   by name is done).  Return the dict in the archive that the symbol is found
+   in, and (optionally) the ctf_id_t of the symbol in that dict (so you don't
+   have to look it up yourself).  The dict is cached, so repeated lookups are
+   nearly free.
 
    As usual, you should ctf_dict_close() the returned dict once you are done
    with it.
 
    Returns NULL on error, and an error in errp (if set).  */
 
-ctf_dict_t *
-ctf_arc_lookup_symbol (ctf_archive_t *wrapper, unsigned long symidx,
-		       ctf_id_t *typep, int *errp)
+static ctf_dict_t *
+ctf_arc_lookup_sym_or_name (ctf_archive_t *wrapper, unsigned long symidx,
+			    const char *symname, ctf_id_t *typep, int *errp)
 {
   ctf_dict_t *fp;
+  void *fpkey;
   ctf_id_t type;
 
   /* The usual non-archive-transparent-wrapper special case.  */
   if (!wrapper->ctfi_is_archive)
     {
-      if ((type = ctf_lookup_by_symbol (wrapper->ctfi_dict, symidx)) == CTF_ERR)
+      if (!symname)
 	{
-	  if (errp)
-	    *errp = ctf_errno (wrapper->ctfi_dict);
-	  return NULL;
+	  if ((type = ctf_lookup_by_symbol (wrapper->ctfi_dict, symidx)) == CTF_ERR)
+	    {
+	      if (errp)
+		*errp = ctf_errno (wrapper->ctfi_dict);
+	      return NULL;
+	    }
+	}
+      else
+	{
+	  if ((type = ctf_lookup_by_symbol_name (wrapper->ctfi_dict,
+						 symname)) == CTF_ERR)
+	    {
+	      if (errp)
+		*errp = ctf_errno (wrapper->ctfi_dict);
+	      return NULL;
+	    }
 	}
       if (typep)
 	*typep = type;
@@ -820,27 +840,28 @@ ctf_arc_lookup_symbol (ctf_archive_t *wrapper, unsigned long symidx,
       return NULL;
     }
 
-  /* Make enough space for all possible symbols, if not already done.
-     We cache both the ctf_id_t and the originating dictionary of all symbols.
-     The dict links are weak, to the dictionaries cached in ctfi_dicts: their
-     refcnts are *not* bumped.  */
+  /* Make enough space for all possible symbol indexes, if not already done.  We
+     cache the originating dictionary of all symbols.  The dict links are weak,
+     to the dictionaries cached in ctfi_dicts: their refcnts are *not* bumped.
+     We also cache similar mappings for symbol names: these are ordinary
+     dynhashes, with weak links to dicts.  */
 
-  if (!wrapper->ctfi_syms)
+  if (!wrapper->ctfi_symdicts)
     {
-      if ((wrapper->ctfi_syms = calloc (wrapper->ctfi_symsect.cts_size
-					/ wrapper->ctfi_symsect.cts_entsize,
-					sizeof (ctf_id_t))) == NULL)
+      if ((wrapper->ctfi_symdicts = calloc (wrapper->ctfi_symsect.cts_size
+					    / wrapper->ctfi_symsect.cts_entsize,
+					    sizeof (ctf_dict_t *))) == NULL)
 	{
 	  if (errp)
 	    *errp = ENOMEM;
 	  return NULL;
 	}
     }
-  if (!wrapper->ctfi_symdicts)
+  if (!wrapper->ctfi_symnamedicts)
     {
-      if ((wrapper->ctfi_symdicts = calloc (wrapper->ctfi_symsect.cts_size
-					    / wrapper->ctfi_symsect.cts_entsize,
-					    sizeof (ctf_dict_t *))) == NULL)
+      if ((wrapper->ctfi_symnamedicts = ctf_dynhash_create (ctf_hash_string,
+							    ctf_hash_eq_string,
+							    free, NULL)) == NULL)
 	{
 	  if (errp)
 	    *errp = ENOMEM;
@@ -848,22 +869,38 @@ ctf_arc_lookup_symbol (ctf_archive_t *wrapper, unsigned long symidx,
 	}
     }
 
-  /* Perhaps it's cached.  */
-  if (wrapper->ctfi_symdicts[symidx] != NULL)
+  /* Perhaps the dict in which we found a previous lookup is cached.  If it's
+     supposed to be cached but we don't find it, pretend it was always not
+     found: this should never happen, but shouldn't be allowed to cause trouble
+     if it does.  */
+
+  if ((symname && ctf_dynhash_lookup_kv (wrapper->ctfi_symnamedicts,
+					 symname, NULL, &fpkey))
+      || (!symname && wrapper->ctfi_symdicts[symidx] != NULL))
     {
-      if (wrapper->ctfi_symdicts[symidx] == &enosym)
+      if (symname)
+	fp = (ctf_dict_t *) fpkey;
+      else
+	fp = wrapper->ctfi_symdicts[symidx];
+
+      if (fp == &enosym)
+	goto no_sym;
+
+      if (symname)
 	{
-	  if (errp)
-	    *errp = ECTF_NOTYPEDAT;
-	  if (typep)
-	    *typep = CTF_ERR;
-	  return NULL;
+	  if ((type = ctf_lookup_by_symbol_name (fp, symname)) == CTF_ERR)
+	    goto cache_no_sym;
+	}
+      else
+	{
+	  if ((type = ctf_lookup_by_symbol (fp, symidx)) == CTF_ERR)
+	    goto cache_no_sym;
 	}
 
       if (typep)
-	*typep = wrapper->ctfi_syms[symidx];
-      wrapper->ctfi_symdicts[symidx]->ctf_refcnt++;
-      return wrapper->ctfi_symdicts[symidx];
+	*typep = type;
+      fp->ctf_refcnt++;
+      return fp;
     }
 
   /* Not cached: find it and cache it.  We must track open errors ourselves even
@@ -882,16 +919,36 @@ ctf_arc_lookup_symbol (ctf_archive_t *wrapper, unsigned long symidx,
 
   while ((fp = ctf_archive_next (wrapper, &i, &name, 0, local_errp)) != NULL)
     {
-      if ((type = ctf_lookup_by_symbol (fp, symidx)) != CTF_ERR)
+      if (!symname)
 	{
-	  wrapper->ctfi_syms[symidx] = type;
-	  wrapper->ctfi_symdicts[symidx] = fp;
-	  ctf_next_destroy (i);
+	  if ((type = ctf_lookup_by_symbol (fp, symidx)) != CTF_ERR)
+	    wrapper->ctfi_symdicts[symidx] = fp;
+	}
+      else
+	{
+	  if ((type = ctf_lookup_by_symbol_name (fp, symname)) != CTF_ERR)
+	    {
+	      char *tmp;
+	      /* No error checking, as above.  */
+	      if ((tmp = strdup (symname)) != NULL)
+		ctf_dynhash_insert (wrapper->ctfi_symnamedicts, tmp, fp);
+	    }
+	}
 
+      if (type != CTF_ERR)
+	{
 	  if (typep)
 	    *typep = type;
+	  ctf_next_destroy (i);
 	  return fp;
 	}
+      if (ctf_errno (fp) != ECTF_NOTYPEDAT)
+	{
+	  if (errp)
+	    *errp = ctf_errno (fp);
+	  ctf_next_destroy (i);
+	  return NULL;				/* errno is set for us.  */
+	}
       ctf_dict_close (fp);
     }
   if (*local_errp != ECTF_NEXT_END)
@@ -899,11 +956,25 @@ ctf_arc_lookup_symbol (ctf_archive_t *wrapper, unsigned long symidx,
       ctf_next_destroy (i);
       return NULL;
     }
+
   /* Don't leak end-of-iteration to the caller.  */
   *local_errp = 0;
 
-  wrapper->ctfi_symdicts[symidx] = &enosym;
+ cache_no_sym:
+  if (!symname)
+    wrapper->ctfi_symdicts[symidx] = &enosym;
+  else
+    {
+      char *tmp;
+
+      /* No error checking: if caching fails, there is only a slight performance
+	 impact.  */
+      if ((tmp = strdup (symname)) != NULL)
+	if (ctf_dynhash_insert (wrapper->ctfi_symnamedicts, tmp, &enosym) < 0)
+	  free (tmp);
+    }
 
+ no_sym:
   if (errp)
     *errp = ECTF_NOTYPEDAT;
   if (typep)
@@ -911,6 +982,23 @@ ctf_arc_lookup_symbol (ctf_archive_t *wrapper, unsigned long symidx,
   return NULL;
 }
 
+/* The public API for looking up a symbol by index.  */
+ctf_dict_t *
+ctf_arc_lookup_symbol (ctf_archive_t *wrapper, unsigned long symidx,
+		       ctf_id_t *typep, int *errp)
+{
+  return ctf_arc_lookup_sym_or_name (wrapper, symidx, NULL, typep, errp);
+}
+
+/* The public API for looking up a symbol by name. */
+
+ctf_dict_t *
+ctf_arc_lookup_symbol_name (ctf_archive_t *wrapper, const char *symname,
+			    ctf_id_t *typep, int *errp)
+{
+  return ctf_arc_lookup_sym_or_name (wrapper, 0, symname, typep, errp);
+}
+
 /* Raw iteration over all CTF files in an archive.  We pass the raw data for all
    CTF files in turn to the specified callback function.  */
 static int